Skip to content

Blob: stringify null/undefined parts, accept iterable blobParts, coerce slice() arguments#33023

Open
robobun wants to merge 2 commits into
mainfrom
claude/farm/ed8e89b7/blob-webidl-coercion
Open

Blob: stringify null/undefined parts, accept iterable blobParts, coerce slice() arguments#33023
robobun wants to merge 2 commits into
mainfrom
claude/farm/ed8e89b7/blob-webidl-coercion

Conversation

@robobun

@robobun robobun commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

null and undefined entries in a blobParts array are silently dropped, so the same code produces different bytes in Bun than in Node, Deno and browsers. The File API takes a WebIDL sequence<BlobPart> whose non-Blob/BufferSource members are converted with ToString, so null contributes the four bytes "null":

new Blob([null, undefined, 1, {}, true]).size
// Bun:  20  ("1[object Object]true")
// Node, Chrome, Firefox, Safari: 33  ("nullundefined1[object Object]true")

new Blob([null]).size // Bun: 4, because the single-element fast path already stringifies

Blob bytes usually end up on the wire (multipart uploads, Response(blob), Bun.write), so a stray null in a parts array yields a payload whose length and hash silently differ from every other runtime. Related conversions in the same constructor and in slice() were also off:

new Blob(null)                      // Bun: empty blob   spec/Node: TypeError
new Blob(new Set(["a", "b"]))       // Bun: TypeError    spec/Node: "ab" (any iterable works)
blob.slice(null, null).size         // Bun: whole blob   spec/Node: 0 (ToNumber(null) is 0)
blob.slice({ valueOf: () => 2 }, 4) // Bun: whole blob   spec/Node: bytes [2, 4)
blob.slice(1, 3, 123).type          // Bun: ""           spec/Node: "123"

Cause: Blob::from_js_without_defer_gc skips is_undefined_or_null() items in the multi-part loop, only walks real JSArrays, and treats a null argument as "no parts". Blob::get_slice only honors start/end values that are already numbers and a contentType that is already a string, instead of applying the WebIDL conversions ([Clamp] long long via ToNumber, DOMString via ToString).

Changes:

  • The part loop no longer skips null/undefined; they flow into the existing ToString arm. Array holes behave the same (new Blob([, "x"]) is "undefinedx", as in Node).
  • new Blob(null) and new File(null, name) throw a TypeError. new Blob() / new Blob(undefined) still produce an empty Blob (the argument is optional).
  • A non-array iterable object (Set, Map, generator, custom iterator, Proxy of an array) is materialized into a JSArray via the JS iteration protocol (new JSValue::to_array_from_iterable in src/jsc/JSValue.rs) and then joined like an array. The array is what keeps generator-produced values alive for the join, since they have no other GC root.
  • slice() runs ToNumber on provided, non-undefined start/end values (null is 0, objects use valueOf, symbols throw, NaN is 0) and ToString on a provided, non-string contentType.

Deliberate Bun extensions that are kept, now with comments saying so:

  • new Blob(typedArray) still copies the bytes. The spec iterates a top-level typed array as a sequence of numbers (new Blob(new Uint8Array([65,66])) is "6566" in Node), which is never what the caller wants.
  • The documented slice(contentType) / slice(begin, contentType) overloads on Blob/BunFile/S3File (a string in the start/end position) are unchanged, so blob.slice("1", "3") still resolves to the contentType overload rather than ToNumber.
  • A detached ArrayBuffer part still contributes 0 bytes: that is what the current WebIDL "get a copy of the bytes held by the buffer source" specifies and what browsers do (Node throws).

Two existing assertions encoded the old coercion and were updated in the same spirit as the change: blob.slice(Symbol(), "-123") and blob.slice(Object.create(null), "-123") now throw TypeError (ToNumber would throw in any other runtime), and the sparse-array fast-path test expects holes to stringify.

How did you verify your code works?

New tests in test/js/web/fetch/blob.test.ts cover the part stringification, new Blob(null) rejection, iterables (including a generator interleaved with Bun.gc(true) and an iterator that throws mid-way), and the slice() coercions. All of them fail on current main and pass with this change:

bun bd test test/js/web/fetch/blob.test.ts test/js/web/fetch/blob-array-fast-path.test.ts
41 pass, 0 fail

Also ran the surrounding suites with the debug (ASAN) build: blob-cow, blob-write, blob-oom, blob-file-name-ownership, deno blob, structured-clone-blob-file, worker_blob, websocket-blob, FormData, response, body, bun-write. No regressions.

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 77ef251f-0edf-4cbf-a025-b9378672efa0

📥 Commits

Reviewing files that changed from the base of the PR and between 332f744 and eb5c074.

📒 Files selected for processing (4)
  • src/jsc/JSValue.rs
  • src/runtime/webcore/Blob.rs
  • test/js/web/fetch/blob-array-fast-path.test.ts
  • test/js/web/fetch/blob.test.ts

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

@robobun

robobun commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 9:31 AM PT - Jul 8th, 2026

@robobun, your commit eb5c074 has 1 failures in Build #70492 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 33023

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

bun-33023 --bun

Comment thread src/runtime/webcore/Blob.rs
Comment thread src/runtime/webcore/Blob.rs
Comment thread src/runtime/webcore/Blob.rs
@robobun

robobun commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased onto current main (eb5c074) and force-pushed. The conflict was with #33599, which introduced the BlobContentType enum; the slice() contentType coercion here now produces a BlobContentType instead of the old raw-pointer/content_type_was_allocated pair, and the moved empty-Blob fast path reads the is_owned() flag from it. All 43 blob tests pass locally (7 of them fail on the released binary).

CI on build 70492: 282 test jobs passed across Linux (glibc, musl, ASAN, baseline), Windows, and macOS. The two red jobs are both :alpine: 3.23 (aarch64 and x64-baseline), and on each the only failing file is test/regression/issue/26030.test.ts: the mysql_plain Docker service container misses its 60s health-check deadline (Failed to start service mysql_plain (via coordinator): application not healthy after 1m0s) so the MySQL transaction test never runs. Unrelated to this Blob diff. Earlier pre-rebase runs (66537, 66582) had the same shape: 282+ green with only macOS agent and Docker-service infra in the red.

I have already retriggered once in this PR and will not push further empty commits; the two red jobs can be retried from the Buildkite UI if a green check is wanted before merging.

robobun added 2 commits July 8, 2026 12:37
…) args

null and undefined entries in a blobParts array were silently dropped, so
the same code produced different bytes in Bun than in Node and browsers
(the File API converts non-Blob/BufferSource parts with ToString). The
single-element fast path already stringified them, so Bun disagreed with
itself too.

- Stop skipping null/undefined in the multi-part loop; they stringify to
  "null"/"undefined" like every other non-Blob/BufferSource part, and
  array holes behave the same.
- new Blob(null) and new File(null, name) throw a TypeError; undefined
  still means the optional argument was omitted.
- Accept any iterable object (Set, Map, generators) as blobParts by
  materializing it into a JSArray with the JS iteration protocol
  (JSValue::to_array_from_iterable); the array keeps generator-produced
  values rooted.
- Blob.slice(): provided, non-undefined start/end go through ToNumber
  (null, booleans, objects with valueOf; symbols throw) and a non-string
  contentType goes through ToString.

The documented Bun overloads slice(contentType) / slice(begin, contentType)
and new Blob(typedArray) are unchanged.
WebIDL converts arguments before the operation body, so ToNumber on
start/end (including its side effects and exceptions) and ToString on
contentType must run even when the source Blob is empty, and a provided
contentType applies to the returned empty Blob.
@robobun robobun force-pushed the claude/farm/ed8e89b7/blob-webidl-coercion branch from 318956a to eb5c074 Compare July 8, 2026 12:43
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