Skip to content

webcore: File.prototype.slice() returns a plain Blob, not a File - #33604

Open
robobun wants to merge 5 commits into
mainfrom
farm/14cd5d82/file-slice-returns-blob
Open

webcore: File.prototype.slice() returns a plain Blob, not a File#33604
robobun wants to merge 5 commits into
mainfrom
farm/14cd5d82/file-slice-returns-blob

Conversation

@robobun

@robobun robobun commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Repro

const f = new File(["0123456789"], "secret-report.pdf", { type: "a/b", lastModified: 1234 });
const s = f.slice(2, 5);
s instanceof File   // bun: true    node/spec: false
s.name              // bun: "secret-report.pdf"   node: undefined
s.lastModified      // bun: 1234    node: undefined

Per the File API slice algorithm, slice() on a File constructs a new plain Blob, with no name, no lastModified, and not a File. Bun returned a File carrying the parent's identity. x instanceof File is the standard branch form/upload handlers and FormData encoders use to decide whether to emit a filename=, so every byte range sliced from a File took that branch and leaked the parent's name onto arbitrary derived fragments.

The same propagation happened for new Blob([file]), which also answered true to instanceof File.

Cause

get_slice_from and the Blob constructor both build the result via dupe(), which copies is_jsdom_file, name and last_modified from the source. instanceof File is implemented as is_jsdom_file.get(), so the slice answered as a File.

The name additionally lives on the shared Bytes store as stored_name (set by the File constructor), and the .name getter fell through to it regardless of the receiver's File brand, so even after clearing is_jsdom_file the slice still reported the parent's name.

Fix

In get_slice_from and the Blob constructor, clear is_jsdom_file, last_modified and name on the duped result so it is a plain Blob.

In get_name_string, skip the Bytes.stored_name fallback when the receiver is not a DOM File. This keeps Bun.file(path).name and S3 .name (non-Bytes stores) working while preventing the shared store's DOM File name from leaking onto plain Blobs that reference it.

structuredClone of a File is unaffected: it goes through Blob__dupe, which still copies is_jsdom_file.

Verification

$ USE_SYSTEM_BUN=1 bun test test/js/web/fetch/blob.test.ts -t "File.prototype.slice"
(fail) File.prototype.slice() returns a Blob, not a File
$ bun bd test test/js/web/fetch/blob.test.ts
27 pass, 0 fail

Related: #32430 and #32434 restructure the File prototype chain more broadly; this PR is the minimal behavioral fix for slice() and new Blob([file]).


[review] gate passed · iteration 8 · 3 files touched

fails on main (without fix)
ASAN without fix: 1 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/fetch/blob.test.ts
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (33ed42b59)

test/js/web/fetch/blob.test.ts:
(pass) blob: imports have sourcemapped stacktraces [19.81ms]
(pass) Blob.slice [44.55ms]
(pass) Bun.file().slice [50.28ms]
(pass) new Blob [4.01ms]
(pass) new Blob stringifies non-Blob object parts in order [11.88ms]
(pass) blob: can be fetched [12.61ms]
(pass) blob: URL has Content-Type [10.66ms]
(pass) blob: can be imported [14.98ms]
(pass) blob: can reliable get type from fetch #10072 [258.25ms]
(pass) new Blob(new Uint8Array()) is supported [5.00ms]
(pass) new File(new Uint8Array()) is supported [4.59ms]
(pass) new File('123', '123') is NOT supported [3.05ms]
(pass) new File() lastModified option > lastModified: NaN -> 0 [3.24ms]
(pass) new File() lastModified option > lastModified: "not 
... (truncated)

release without fix: 7 failed, 2 skipped
bun test v1.4.0-canary.1 (1498d7b77)

test/js/web/fetch/blob.test.ts:
(pass) blob: imports have sourcemapped stacktraces [0.61ms]
(pass) Blob.slice [0.50ms]
(pass) Bun.file().slice [0.84ms]
(pass) new Blob [0.07ms]
(pass) new Blob stringifies non-Blob object parts in order [0.18ms]
(pass) blob: can be fetched [0.23ms]
(pass) blob: URL has Content-Type [0.17ms]
(pass) blob: can be imported [0.27ms]
(pass) blob: can reliable get type from fetch #10072 [205.39ms]
(pass) new Blob(new Uint8Array()) is supported [0.11ms]
(pass) new File(new Uint8Array()) is supported [0.06ms]
(pass) new File('123', '123') is NOT supported [0.05ms]
217 |     [true, 1],
218 |     ["123", 123],
219 |     [1234, 1234],
220 |     [-1, -1],
221 |   ] as const)("lastModified: %p -> %p", (input, expected) => {
222 |     expect(lm({ lastModified: input })).toBe(expected);
                                              ^
error: expect(received).toBe(expected)

Expected: 0
Received: NaN

      at <anonymous> (/workspace/bun/test/js/web/fetch/blob.test.ts:222:41)
(fail) new File() lastModified option > lastModified: NaN -> 0 [0.20ms]
217 |     [true, 1],
218 |     ["123", 123],
219 |     [1234, 1234],
... (truncated)
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/fetch/blob.test.ts
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (33ed42b59)

test/js/web/fetch/blob.test.ts:
(pass) blob: imports have sourcemapped stacktraces [20.71ms]
(pass) Blob.slice [44.72ms]
(pass) Bun.file().slice [47.69ms]
(pass) new Blob [4.13ms]
(pass) new Blob stringifies non-Blob object parts in order [11.62ms]
(pass) blob: can be fetched [12.85ms]
(pass) blob: URL has Content-Type [10.68ms]
(pass) blob: can be imported [15.23ms]
(pass) blob: can reliable get type from fetch #10072 [253.89ms]
(pass) new Blob(new Uint8Array()) is supported [5.96ms]
(pass) new File(new Uint8Array()) is supported [4.48ms]
(pass) new File('123', '123') is NOT supported [3.02ms]
(pass) new File() lastModified option > lastModified: NaN -> 0 [3.24ms]
(pass) new File() lastModified option > lastModified: "not 
... (truncated)

release with fix: 2 skipped
$ bun scripts/build.ts --profile=release
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     33ed42b593
  features     (none)

22 deps, 105 codegen, 1168 objects in 823ms

ninja: Entering directory `/workspace/bun/build/release'
[1/1231] install /workspace/bun
bun install v1.4.0-canary.1 (1498d7b77)

Checked 124 installs across 170 packages (no changes) [9.00ms]
[2/1231] gen ErrorCode+*.h
[3/1231] install /workspace/bun/packages/bun-error
bun install v1.4.0-canary.1 (1498d7b77)

Checked 1 install across 2 packages (no changes) [4.00ms]
[4/1231] install /workspace/bun/src/node-fallbacks
bun install v1.4.0-canary.1 (1498d7b77)

Checked 129 installs across 147 packages (no changes) [6.00ms]
[5/1231] gen bindgenv2
[6/1231] fetch tinycc
[tinycc] up to date
[7/1230] fetch picohttpparser
[picohttpparser] up to date
[8/1230] fetch z
... (truncated)
diff hotspot
src/jsc/webcore_types.rs       |  9 +++++
 src/runtime/webcore/Blob.rs    | 26 ++++++++++++++
 test/js/web/fetch/blob.test.ts | 79 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 114 insertions(+)

gate history · 1 passed · 0 rejected · iteration 8

evidence per changed file
file                            reads  edits  tests
src/jsc/webcore_types.rs            3      1      0
src/runtime/webcore/Blob.rs        19     11      0
test/js/web/fetch/blob.test.ts      5      7      0

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Blob creation paths now clear File identity fields, while name retrieval suppresses stored names for plain byte-backed Blobs. Tests cover slicing, Blob wrapping, multipart naming, and preservation of the original File.

Changes

Blob File Identity Stripping

Layer / File(s) Summary
Strip File identity in slice and constructor paths
src/runtime/webcore/Blob.rs
get_slice_from and the Blob constructor reset is_jsdom_file, last_modified, and name so slices and new Blob(parts) do not retain File metadata.
Suppress name exposure for non-File Blobs
src/jsc/webcore_types.rs, src/runtime/webcore/Blob.rs
hides_bytes_stored_name, get_name_string, and Blob__getFileNameString suppress stored names for non-is_jsdom_file Blobs backed by store::Data::Bytes(_).
Test coverage for slice identity behavior
test/js/web/fetch/blob.test.ts
A new test verifies plain Blob results, metadata clearing, Blob wrapping, multipart filename behavior, and preservation of the original File and its clone.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main behavior change in the PR.
Description check ✅ Passed The description covers the change, root cause, fix, and verification, though it uses custom headings instead of the template.

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the claude label Jul 7, 2026
@robobun

robobun commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 7:16 PM PT - Jul 10th, 2026

@robobun, your commit 33ed42b has 3 failures in Build #71690 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 33604

That installs a local version of the PR into your bun-33604 executable, so you can run:

bun-33604 --bun

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Found 1 issue this PR may fix:

  1. Blob has name #20700 - Blob incorrectly exposes a name property; this PR's .name getter guard prevents plain Blobs from leaking the parent File's name via Bytes.stored_name

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #20700

🤖 Generated with Claude Code

@robobun

robobun commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

This does not fix #20700. That issue is about the .name accessor living on Blob.prototype at all ("name" in new Blob() is still true after this change); #32434 is the PR that moves the accessor to File.prototype. This PR only stops a File's name/brand from leaking onto the Blob returned by slice() and new Blob([file]).

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. Blob/File: do not inherit part metadata on the single-Blob-part fast path #33603 - Also fixes new Blob([file]) being instanceof File and File metadata leaking through dupe(), using a dupe_without_metadata() approach to clear is_jsdom_file on the single-Blob-part fast path

🤖 Generated with Claude Code

@robobun

robobun commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

Not a duplicate of #33603. That PR fixes the new Blob([part]) / new File([part], ...) single-part fast path inheriting the part's metadata; it does not touch get_slice_from, so file.slice() instanceof File (the bug this PR targets) stays true there.

Overlap: both end up fixing new Blob([file]) instanceof File, via different layers (#33603 inside Blob::get, this PR after it in the Blob constructor). The get_slice_from change and the get_name_string gating here are unique to this PR and required for file.slice().name to be undefined, since the slice shares the source's Bytes store where stored_name lives.

Comment thread src/runtime/webcore/Blob.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/runtime/webcore/Blob.rs`:
- Around line 2190-2196: `get_name_string` and `Blob__getFileNameString`
duplicate the same file-identity suppression logic, so extract that predicate
into a shared private `Blob` helper such as `suppresses_stored_name` and have
both call sites use it. Keep the helper encapsulating the `is_jsdom_file` and
`store.data` check so any future refinement only needs to be updated in one
place.

In `@test/js/web/fetch/blob.test.ts`:
- Line 222: The assertion in the blob slicing test is too weak because it only
checks that lastModified is not the old value instead of verifying the
normalized Blob behavior. Update the expectation in the blob slice test around
sliced so it asserts the actual value of lastModified is undefined, matching the
fact that the result is a plain Blob and not a File. Keep the check anchored to
the sliced blob object and its lastModified property.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f1e3f85a-10dc-491c-ad40-6feeb76c3474

📥 Commits

Reviewing files that changed from the base of the PR and between 3f67971 and a18ce67.

📒 Files selected for processing (2)
  • src/runtime/webcore/Blob.rs
  • test/js/web/fetch/blob.test.ts

Comment thread src/runtime/webcore/Blob.rs Outdated
Comment thread test/js/web/fetch/blob.test.ts
@robobun

robobun commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

CI status: the diff is green on all 280 lanes that ran. test/js/web/fetch/blob.test.ts and FormData.test.ts pass everywhere.

The two red jobs are both darwin 26 aarch64 - test-bun failing with buildkite-agent artifact download timed out after 120s before any tests run (the runner can't fetch the built binary). Same infra failure on both build 69642 and build 69669. The other annotated failures are retried flakes on unrelated lanes (http2, spawn, bun-install, repl, postgres, napi, next-pages), none touching Blob/File/FormData.

Ready for review.

@robobun
robobun force-pushed the farm/14cd5d82/file-slice-returns-blob branch from ac72fd5 to 2aaa634 Compare July 7, 2026 23:11
Comment thread src/runtime/webcore/Blob.rs Outdated
Comment thread src/runtime/webcore/Blob.rs
Comment thread src/runtime/webcore/Blob.rs
@robobun

robobun commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

CI on 05cd74f (build 70174): blob.test.ts and the rest of the diff pass on all 279 lanes that ran. The three red jobs are all service/infra failures unrelated to this change:

  • darwin 26 aarch64: buildkite-agent artifact download timed out after 120s before any tests run (same failure on the last four builds of this PR)
  • alpine 3.23 aarch64: test/regression/issue/26030.test.ts MySQL docker container application not healthy after 1m0s
  • windows 2019 x64: postgres-binary-array-bounds.test.ts ERR_POSTGRES_CONNECTION_REFUSED

All review threads resolved. Ready for review.

robobun added 5 commits July 10, 2026 23:51
Per the File API spec, Blob.prototype.slice() constructs a new plain Blob
regardless of the receiver's subclass. Bun was duping the receiver and
carrying over is_jsdom_file, name and last_modified, so slicing a File
yielded a File that answered true to instanceof File and inherited the
parent's name and lastModified. The same propagation happened for
new Blob([file]).

Clear the File brand and identity fields on the dupe produced by slice()
and by the Blob constructor, and gate the .name getter so it does not
surface the shared Bytes store's stored_name on a non-File Blob.
@robobun
robobun force-pushed the farm/14cd5d82/file-slice-returns-blob branch from 05cd74f to 33ed42b Compare July 10, 2026 23:58

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new issues found; the earlier regression I flagged (File-store slice unmasking the disk path) is fixed and covered by the new test. Deferring to a human because this is a user-visible Web API behavior change with acknowledged collateral (Bun.embeddedFiles FormData filename now empty) and it overlaps with #33603/#32434 at the same layer — worth a maintainer deciding how those compose.

Checked: get_slice_from / Blob constructor clear is_jsdom_file/last_modified and gate name clearing on Bytes-only stores; ordering after is_jsdom_file.set(false) makes the predicate reduce to the store-type check as intended.
Checked: empty-File .slice() early-return path (get_slice:2055) returns a fresh init_empty blob, so the new code is bypassed there — matches the test's toBeUndefined().
Checked: get_name_string / Blob__getFileNameString both route through the shared hides_bytes_stored_name() helper; structuredClone path via Blob__dupe still copies is_jsdom_file.
Remaining pre-existing sibling consumers (Any::get_file_name for Content-Disposition, Blob__setAsFile for FormData read-back) were noted inline and deferred to #32430/#32434.

Extended reasoning...

Overview

Three files: src/jsc/webcore_types.rs (adds hides_bytes_stored_name() helper), src/runtime/webcore/Blob.rs (clears File identity in get_slice_from and the Blob constructor; gates the .name getter and Blob__getFileNameString FFI on the new helper), and test/js/web/fetch/blob.test.ts (~80-line test covering slice, wrap, FormData encode, structuredClone, and the Bun.file()-backed File regression case).

Security risks

None introduced. The change is a spec-compliance fix that reduces metadata leakage (parent File name no longer propagates to derived slices). One regression I flagged earlier — where clearing blob.name on a File/S3-backed DOM File slice unmasked the on-disk path via get_file_name() fallthrough — has been fixed by gating the clear on hides_bytes_stored_name() (Bytes-store only), with a regression test.

Level of scrutiny

Medium-high. Blob.rs is core webcore runtime code with many downstream consumers (FormData encode/decode, Bun.serve Content-Disposition, structuredClone, embedded-file assets, S3). The stored_name field on the shared Bytes store has at least four independent readers, and the new predicate interacts differently with each of the three store variants. During review this PR already went through one 🔴 regression fix and two acknowledged-as-follow-up behavior notes, which is a signal that the interaction surface is non-trivial.

Other factors

  • Overlap with open PRs: #33603 fixes new Blob([file]) instanceof File via a different layer (dupe_without_metadata() inside Blob::get), and #32430/#32434 restructure the File prototype chain more broadly. A maintainer should decide sequencing/composition.
  • Acknowledged collateral: fd.append(name, Bun.embeddedFiles[i]) in --compile'd binaries now emits filename="" instead of the internal /$bunfs/root/... path. Author agreed a this.name short-circuit in Blob__getFileNameString would be strictly better but deferred it; a human should confirm that trade-off is acceptable to ship.
  • CI: green on all lanes that ran; failures are unrelated infra (artifact download timeout, MySQL/Postgres container health).
  • All four of my prior inline threads are resolved; the bug-hunting pass this run found nothing new.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant