Skip to content

Blob: convert a type that is not a string in new Blob, new File and slice - #47740

Open
robobun wants to merge 5 commits into
mainfrom
robobun/9dce8142/blob-type-domstring
Open

Blob: convert a type that is not a string in new Blob, new File and slice#47740
robobun wants to merge 5 commits into
mainfrom
robobun/9dce8142/blob-type-domstring

Conversation

@robobun

@robobun robobun commented Nov 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Problem

  • new Blob(parts, { type }), new File(bits, name, { type }) and blob.slice(start, end, type) drop a type that is not a string, with no error. The File API declares each a DOMString.
  • The three bodies tested content_type.is_string() first (src/runtime/webcore/Blob.rs:1806, :2161, :5320). No user reported it.

Fix

  • The first commit is the fix: ToString converts every type but undefined.
  • The constructors then share set_type_from_options, which reads type with fast_get.
  • The AOT table of built-ins now says that these calls run operand code, in the lines of AOT: hangs, wrong answers, failed builds; bun test --aot follows #44833's mocks #47605.
  • Verified: test/js/web/fetch/blob.test.ts (29 of 46 new tests fail on main), two AOT programs, 18 more suites. Self-reviewed: 27 concerns raised, 24 addressed (Notes).

Background

  • WebIDL takes only undefined as absent: null gives "null", a Symbol throws. lastModified in the same options converts already (File: normalize lastModified option (NaN->0, null is present) #33922).
  • That table tells bun build --compile --aot what a built-in runs. With the old rows it removes an unused conversion.
  • Considered one conversion function on BlobContentType: a type inside a JsResult cost 43 cycles per typed slice.

Downsides

  • A maintainer must decide this: type: null and type: false now give "null" and "false", as in Node.js, and fetch sends Content-Type: null. Before, Bun sent no header. headers.get("content-type") gives null.
  • A Symbol, or a toString that throws, now throws. Not fixed: slice of an empty Blob returns before it reads an operand and drops every type, a string too (Blob.slice: type the slice like new Blob(parts, { type }) #47335).
  • Cost: a string type runs main's code (blob.slice() 421 to 415 instructions). Binary text: 140,286,972 to 140,286,716 bytes.
Notes

Where this comes from. No user reported it. A comparison of Bun with Node.js found it. It is the same in Bun 1.3.14, 1.4.4 and main.

Node.js v26.3.0 main 3f3a9ff this PR
new Blob([], { type: new MIMEType("text/made-up") }).type "text/made-up" "" "text/made-up"
new Blob([], { type: 1 }).type "1" "" "1"
new File([], "n", { type: ["text/made-up"] }).type "text/made-up" "" "text/made-up"
new Blob(["ab"]).slice(0, 1, { toString: () => "text/made-up" }).type "text/made-up" "" "text/made-up"
new Blob([], { type: null }).type "null" "" "null"
new Blob([], { type: false }).type "false" "" "false"
new Blob([], { type: Symbol() }) TypeError "" TypeError
new Blob([], { type: undefined }).type "" "" ""

Visible changes.

  1. A type or contentType that is not a string and not undefined is converted with ToString. A number, a boolean, a BigInt and null give their text. An object gives what its Symbol.toPrimitive, toString or valueOf gives. A Symbol, and an object with no way to a primitive, throw a TypeError. An exception from toString propagates and no Blob is made. The conversion runs once, inside the call. The text is then checked and lowercased as a string is.
  2. The converted type replaces a type that Bun took from somewhere else. On main, new Blob([typedBlob], { type: 1 }) kept the type of the part, and typedBlob.slice(0, 1, 1) kept a registered type of the receiver. A value whose text is empty ([], { toString: () => "" }) now does what "" does.

The question for a maintainer. null and false are the values that existing code can pass: headers.get("content-type") gives null for an absent header, and a lookup of a MIME type can give null or false when it finds nothing.

type given to new Blob main this PR and Node.js v26.3.0
null .type is "", a fetch upload has no Content-Type "null", Content-Type: null
false "", no Content-Type "false", Content-Type: false
null, for a File in a FormData body the part has Content-Type: application/octet-stream the part has Content-Type: null
  • For it: the File API, Node.js and browsers. web-platform-tests asserts "null" for slice (null type Blob slice). lastModified: null in the same options is a present value since File: normalize lastModified option (NaN->0, null is present) #33922.
  • Against it: a program that sent no header now sends a wrong one.
  • If the answer is no for the constructors: JSValue::get_stringish_allow_empty is the getter in the tree that takes null and false as absent and converts the rest. It reads by name, so it cannot serve slice.

Reference.

  • File API: dictionary BlobPropertyBag { DOMString type = ""; ... } and Blob slice(optional [Clamp] long long start, optional [Clamp] long long end, optional DOMString contentType). WebIDL converts a DOMString with ToString and takes only undefined as absent.
  • web-platform-tests, with a stand-in for testharness.js (main to this PR): FileAPI/file/File-constructor.any.js 42 to 43 of 49 (Property bag propagates exceptions). FileAPI/blob/Blob-slice.any.js stays at 143 of 150: null type Blob slice has a receiver of size 0 and needs Blob.slice: type the slice like new Blob(parts, { type }) #47335 too. FileAPI/blob/Blob-constructor.any.js stays at 56 of 73: options properties should be accessed in lexicographic order now sees type converted and still fails, because Bun does not read endings.
  • Node.js converts with a template literal, once, in the constructor and in slice of lib/internal/blob.js.
  • A script that logs every getter, toString and valueOf call at the three entry points prints 27 lines. 20 of them differ from Node.js v26.3.0 on main, and 18 on this PR. The 18 are all items of "Not changed" below: the receiver of size 0 (3), Bun's forms of slice (5), the types that Bun takes from a part or from the receiver (5), start and end (1), endings (1), the order in new File (1), and type on Object.prototype, which Bun does not read (2).

Order. new Blob converts its parts, then reads type, then converts it. A part that throws leaves type unread. new File reads and converts type before it reads lastModified. slice converts only the operand that is its contentType: in slice("a/b", 1, object) the string is the contentType and the object is not read. slice reads the size of the receiver before it converts, as it does on main for a String object.

The table of built-ins. bun build --compile --aot reads it (src/codegen/builtins/host/web-classes.builtins.ts).

Measurements. Release builds (bun run build:release) of main 3f3a9ff and of this PR, linux-x64. perf, valgrind, strace and bloaty are not installed, so the counts come from gdb and from perf_event_open.

Instructions / mallocs / frees per host call. A gdb script single-steps 7 calls from the entry of the host function to its return, after 200 calls, and counts the entries of the allocator. The table gives the mode of the 7. small is new Blob([JSON.stringify("hello world ")]), typed is new Blob(["abcdef"], { type: "text/plain" }), file is new File(["abc"], "n.txt"), empty is new Blob([]), o is { toString() { return "x/custom" } }.

Call main this PR
Calls that gave the right answer on main
small.slice() 421 / 1 / 0 415 / 1 / 0
small.slice(0, 5) 443 / 1 / 0 435 / 1 / 0
small.slice(0, 5, "") 807 / 2 / 0 792 / 2 / 0
small.slice(0, 5, "x/custom") 1306 / 3 / 1 1290 / 3 / 1
small.slice(0, 5, "text/html") 1408 / 1 / 0 1392 / 1 / 0
small.slice("x/custom") 1279 / 3 / 1 1267 / 3 / 1
small.slice(0, 5, new String("x/custom")) 1765 / 3 / 1 1751 / 3 / 1
typed.slice() 436 / 1 / 0 430 / 1 / 0
file.slice(0, 2) 456 / 1 / 0 448 / 1 / 0
new Blob(["abc"]) 776 / 3 / 0 774 / 3 / 0
new Blob(["abc"], {}) 1271 / 4 / 1 953 / 3 / 0
new Blob(["abc"], { type: "" }) 1735 / 5 / 2 1375 / 4 / 1
new Blob(["abc"], { type: "x/custom" }) 2156 / 6 / 2 1796 / 5 / 1
new Blob(["abc"], { type: "text/html" }) 2257 / 4 / 1 1897 / 3 / 0
new Blob(["abc"], { type: new String("x/custom") }) 2664 / 6 / 2 2309 / 5 / 1
new File(["abc"], "n", { lastModified: 1 }) 1979 / 5 / 2 1664 / 4 / 1
new File(["abc"], "n", { type: "x/custom", lastModified: 1 }) 2864 / 7 / 3 2507 / 6 / 2
new File(["abc"], "n", { type: "text/html", lastModified: 1 }) 2965 / 5 / 2 2608 / 4 / 1
A receiver of size 0
empty.slice() 179 / 1 / 0 179 / 1 / 0
empty.slice(0, 0) 182 / 1 / 0 182 / 1 / 0
empty.slice(0, 0, "") 183 / 1 / 0 183 / 1 / 0
empty.slice(0, 0, "x/custom") 183 / 1 / 0 183 / 1 / 0
empty.slice(0, 0, "text/html") 183 / 1 / 0 183 / 1 / 0
Calls that now convert
small.slice(0, 5, o) 450 / 1 / 0 1981 / 3 / 1
small.slice(0, 5, 1) 444 / 1 / 0 1009 / 3 / 1
small.slice(0, 5, null) 444 / 1 / 0 1057 / 3 / 1
new Blob(["abc"], { type: o }) 1332 / 4 / 1 2539 / 5 / 1
new Blob(["abc"], { type: 1 }) 1321 / 4 / 1 1490 / 5 / 1
new Blob(["abc"], { type: null }) 1321 / 4 / 1 1554 / 5 / 1
new File(["abc"], "n", { type: o, lastModified: 1 }) 2040 / 5 / 2 3250 / 6 / 2
  • slice is main's code with another guard: the test for undefined is shorter than the test for a string.
  • The rows of a constructor with options lose one malloc and one free. That is the fast_get commit: options.get(global, "type") made and freed a StringImpl at every call to look the name up.
  • new File without lastModified reads the clock, and that read does not take the same number of steps twice under gdb. So the File rows give lastModified.

Cycles per call in user mode (perf_event_open: the counts of a loop of 1.6 million calls minus those of 400 thousand, one pinned core, BUN_JSC_useConcurrentJIT=0, BUN_JSC_useConcurrentGC=0, BUN_JSC_numberOfGCMarkers=1). Median of 15 runs, with the three binaries taken in turn. The second run of main shows the noise.

Call main this PR main again
small.slice() 166 167 165
small.slice(0, 5) 168 167 170
small.slice(0, 5, "") 310 311 311
small.slice(0, 5, "x/custom") 391 395 391
small.slice(0, 5, "text/html") 396 395 402
small.slice(0, 5, "image/png") 430 437 428
small.slice("x/custom") 388 390 390
typed.slice() 177 178 177
empty.slice() 109 105 110
empty.slice(0, 0, "") 117 113 118
new Blob(["hello world"]) 276 278 278
new Blob(["abc"], {}) 401 307 405
new Blob(["abc"], { type: "x/custom" }) 704 565 708
new Blob(["abc"], { type: "text/html" }) 658 559 677
new File(["abc"], "n", { lastModified: 1 }) 568 480 571
new File(["abc"], "n", { type: "text/html", lastModified: 1 }) 837 738 834
small.slice(0, 5, o) 170 541 170
new Blob(["abc"], { type: o }) 379 692 377

Code bytes (llvm-nm -S): BlobExt::slice 1195 to 1114, BlobExt::constructor 1072 to 673, JSDOMFile__construct 1436 to 1111, the new set_type_from_options 488, BlobContentType::from_script 304 on both. Together 4007 to 3690. Release binary text (size): 140,286,972 to 140,286,716 bytes.

What the self-review cut. The first version of this change was larger.

  • It had one conversion function on BlobContentType that returned JsResult<Option<BlobContentType>>. It ran fewer instructions than main and was slower: slice(0, 5, "x/custom") went from 390 to 433 cycles. The type is 40 bytes, and the error of a JsResult is one byte inside them, so the compiler copied the type with 16-byte loads at odd offsets, and no 8-byte store is forwarded to such a load. The review then cut the conversion to the three guards on main's text. BlobContentType and is_valid_blob_type are not touched.
  • It converted the operands of slice for a receiver of size 0 too. That is the change of Blob.slice: type the slice like new Blob(parts, { type }) #47335, and it gave slice a second rule for the mark that a caller gave the type. It is out.
  • It made an empty type from script the static empty type and gave back a registered type with no copy. Both are faster (slice(0, 5, "") 310 to 229 cycles, slice(0, 5, "text/html") 397 to 372) and neither is this fix. They are out.
  • Of the 27 concerns, 3 are not done. The review asked to leave the table hunk to AOT: hangs, wrong answers, failed builds; bun test --aot follows #44833's mocks #47605: it is here, in the lines of AOT: hangs, wrong answers, failed builds; bun test --aot follows #44833's mocks #47605, because the differential test fails without it. It asked to vendor the three files of web-platform-tests: Blob.slice(): convert start/end as WebIDL [Clamp] long long (round half to even) #41961 vendors Blob-slice.any.js with a driver, and a second copy would collide. It asked for a maintainer's answer on null before this PR opens: the question is the first Downside.

Not changed.

  • A receiver of size 0: slice returns before it reads an operand (Blob.rs:1765). The slice has no type, a string included, and no operand code runs: new Blob([]).slice(0, 0, Symbol()) does not throw. Blob.slice: type the slice like new Blob(parts, { type }) #47335 moves that return below the contentType block. A conversion before the return would still give no type, so new Blob([]).slice(0, 0, type) is not among the receivers of the new tests: it needs both PRs.
  • start and end of slice that are not numbers count as absent. WebIDL converts them with ToNumber. test/js/web/fetch/blob.test.ts:61 pins the present rule: blob.slice(Symbol(), "-123") does not throw, and "-123" is a contentType there. Blob.slice(): convert start/end as WebIDL [Clamp] long long (round half to even) #41961 leaves that question to a maintainer.
  • Bun's own forms slice(type) and slice(start, type) are chosen by a string, or a String object, in the place of start or end. An object there is not a type.
  • options that is not an object (new Blob([], 1)) is ignored (Blob.rs:2158, :5315). WebIDL throws a TypeError: 4 subtests of Blob-constructor.any.js and 4 of File-constructor.any.js.
  • endings is not read (Blob constructor endings option doen't work. #11796). new File converts name before the bits.
  • The type that Bun gives when no valid type is given: the type of a single Blob part (Blob: new Blob([blob]) and new File([blob], name) do not take the type of the part #46173), and a registered type of the receiver of a slice.
  • A slice typed with a registered type, blob.slice(0, 1, "image/png"), does not count as typed by the caller, so a fetch upload of it has no Content-Type (Blob.slice: mark the slice as typed when it is given a contentType #46516). A converted new MIMEType("image/png") is the same.
  • new Blob([], { type: "application/x-www-form-urlencoded;charset=UTF-8" }).type keeps its upper case, on main and here. Node.js lowercases it.
  • Bun's own options. Bun.file(path, { type }) ignores a type that is not a string. Bun.file("s3://...", { type }), S3Client and client.file throw ERR_INVALID_ARG_TYPE for one, and so does Bun.file(path).write(data, { type }). Bun.write(path, data, { type }) does not read type. These are not members of the File API, and no specification gives them a conversion. The schema of Bun.file declares its rule (src/runtime/webcore/Blob.bind.ts:53): "Anything but a string is ignored, and so is """. Its type replaces the type that Bun takes from the path. One rule for Bun's own options is a separate decision.
  • The other members of the comparison: keyUsages of Web Crypto, and redirect, mode and cache of a Request.

Open PRs on the same lines. Trial merges of this head.

Earlier attempt. #33023 converted the contentType of slice with ToString, and start and end with ToNumber. A cleanup of stale pull requests closed it with no review. This PR takes its contentType half.

Suites. Debug build of this head, with BUN_JSC_validateExceptionChecks=1: test/js/web/fetch/blob.test.ts 206 pass. These 18 files together: 754 pass, 2 skipped, 5 todo, 0 fail: bun-file, bun-file-read, file-type, bun-serve-static, bun-serve-file, blob-cow, blob-write, blob-array-fast-path, blob-file-name-ownership, structured-clone-blob-file, FormData, FormData-multipart-serialization, test/js/web/fetch/response.test.ts, body-clone, client-fetch, fetch-args, worker_blob, test/js/deno/fetch/blob.test.ts. Release build: bun test --aot test/js/web/fetch/blob.test.ts 204 pass, 2 skipped. The 73 programs under test/aot/parallel that name Blob or File: 73 pass. test/internal/builtins-table/ and the table tests under test/aot/builtins/ (34 files): 234 pass, 1 skipped. test/integration/bun-types/bun-types.test.ts: 30 pass. test/internal/source-lints/: 195 pass.

[human-review] gate passed · iteration 0 · 6 files touched

fails on main (without fix) 
ASAN without fix: 29 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/web/fetch/blob.test.ts
bun test v1.4.4 (367d939d9)

test/js/web/fetch/blob.test.ts:
(pass) blob: imports have sourcemapped stacktraces [11.31ms]
(pass) Blob.slice [19.72ms]
(pass) Bun.file().slice [29.28ms]
(pass) new Blob [2.69ms]
(pass) type is the text/plain that the script gave [4.28ms]
(pass) type is the text/html that the script gave [2.00ms]
(pass) type is the text/css that the script gave [1.48ms]
(pass) type is the application/json that the script gave [1.69ms]
(pass) type is the application/javascript that the script gave [1.74ms]
(pass) type is the image/png that the script gave [1.43ms]
(pass) a clone has the type of its Blob, which got "Application/X-THING; A=B" from a header [4.90ms]
(pass) a clone has the type of its Blob, which got "TEXT/PLAIN" from a header [1.26ms]
(pass) a clone has the type of its Blob, which got "Text/Plain; Charset=UTF-8" from a header [0.87ms]
(pass) a clone has the type of its Blob, which got "text/plain" from a header [0.82ms]
(pass) a Content-Type header made from a Blob still names the
... (truncated)

release without fix: 2 skipped
bun test v1.4.4-canary.1 (f0454d946)

test/js/web/fetch/blob.test.ts:
(pass) blob: imports have sourcemapped stacktraces [3.83ms]
(pass) Blob.slice [0.37ms]
(pass) Bun.file().slice [10.00ms]
(pass) new Blob [0.12ms]
(pass) type is the text/plain that the script gave [0.78ms]
(pass) type is the text/html that the script gave [0.04ms]
(pass) type is the text/css that the script gave [0.01ms]
(pass) type is the application/json that the script gave [0.01ms]
(pass) type is the application/javascript that the script gave [0.01ms]
(pass) type is the image/png that the script gave [0.01ms]
(pass) a clone has the type of its Blob, which got "Application/X-THING; A=B" from a header [3.93ms]
(pass) a clone has the type of its Blob, which got "TEXT/PLAIN" from a header [0.02ms]
(pass) a clone has the type of its Blob, which got "Text/Plain; Charset=UTF-8" from a header [0.01ms]
(pass) a clone has the type of its Blob, which got "text/plain" from a header
(pass) a Content-Type header made from a Blob still names the charset of text [0.21ms]
(pass) a type that is not a string > new Blob(parts, { type }) > is converted with ToString [0.54ms]
(pass) a type that is not a string > n
... (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/pr_gate.xml" test/js/web/fetch/blob.test.ts
bun test v1.4.4 (367d939d9)

test/js/web/fetch/blob.test.ts:
(pass) blob: imports have sourcemapped stacktraces [14.09ms]
(pass) Blob.slice [20.63ms]
(pass) Bun.file().slice [31.42ms]
(pass) new Blob [3.23ms]
(pass) type is the text/plain that the script gave [5.06ms]
(pass) type is the text/html that the script gave [2.08ms]
(pass) type is the text/css that the script gave [1.48ms]
(pass) type is the application/json that the script gave [1.46ms]
(pass) type is the application/javascript that the script gave [1.78ms]
(pass) type is the image/png that the script gave [1.35ms]
(pass) a clone has the type of its Blob, which got "Application/X-THING; A=B" from a header [5.17ms]
(pass) a clone has the type of its Blob, which got "TEXT/PLAIN" from a header [1.00ms]
(pass) a clone has the type of its Blob, which got "Text/Plain; Charset=UTF-8" from a header [0.81ms]
(pass) a clone has the type of its Blob, which got "text/plain" from a header [0.75ms]
(pass) a Content-Type header made from a Blob still names the
... (truncated)

release with fix: 2 skipped
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 793ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/326] gen generated_host_exports.rs
generated_host_exports.rs: 119 exports (lazy=4, generic=115, rust=0); 288 extern-C blocks audited
[2/325] gen the table of built-ins
builtins table: operands without uses: none
builtins table: 2101 host entries have a result that is not described; a program in which a value may be anything holds what any host class needs: 0 modules, 15 functions
[3/13] gen builtin_sources.bin
[4/12] rustc bun_executable 
[5/12] rustc bun_runtime 
[6/12] check the generated files each crate waits for
[7/12] link bun-profile
ld.lld: warning: Linking two modules of different target triples: 'obj/src/simdutf_sys/bun-simdutf.cpp.o' is 'x86_64-pc-linux-gnu' whereas 'rust-target/x86_64-unknown-linux-gnu/deps/libbun_lint_react-46440581f8dc23da.rlib(bun_lint_react-46440581f8dc23da.bun_lint_react.6093be95695f9829-cgu.0.rcgu.o at 568468)' is 'x86_64-unknown-linux-gnu'


ld.lld: warning: Linking two modules of different target triples: 'obj/src/simdutf_sys/bun-simdutf.cpp.o' is 'x86_64-pc-linux-gn
... (truncated)
diff hotspot 
packages/bun-types/globals.d.ts                    |   2 +-
 src/codegen/builtins/host/web-classes.builtins.ts  |   7 +-
 src/runtime/webcore/Blob.rs                        |  51 ++---
 ...ing-object-whose-tostring-nothing-else-calls.js |  21 ++
 .../a-blob-converts-a-type-that-is-not-a-string.js |  48 ++++
 test/js/web/fetch/blob.test.ts                     | 249 +++++++++++++++++++++
 6 files changed, 344 insertions(+), 34 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file 
file                                                      reads  edits  tests
packages/bun-types/globals.d.ts                               0      0     43
src/codegen/builtins/host/web-classes.builtins.ts             3      0     44
src/runtime/webcore/Blob.rs                                   2      2     42
…ts-a-string-object-whose-tostring-nothing-else-calls.js      0      0     45
…-classes/a-blob-converts-a-type-that-is-not-a-string.js      0      1     48
test/js/web/fetch/blob.test.ts                                2      1     41

robobun and others added 5 commits November 2, 2026 10:17
@robobun
Blob: convert a type that is not a string in new Blob, new File and s…
fd9e901 
…lice

The File API declares the `type` of the options of `new Blob()` and
`new File()`, and the contentType of `slice()`, as a DOMString:
`undefined` is absent, and ToString converts any other value. The three
bodies first tested `is_string()`, which is true for a string and for a
String object, and dropped any other value with no error.

The two constructors now convert every `type` that is present. `slice`
converts every contentType that is not `undefined`, after it picks Bun's
`slice(type)` and `slice(start, type)` forms.
@robobun
Blob: new Blob and new File share the step that reads the type option
b79fb01 
The two constructors had the same block. No behaviour changes.
@robobun
Blob: read the type option by its pre-made name
b8aa5b8 
`options.get(global, "type")` makes and frees a StringImpl at every call
to look the name up. `fast_get` does the same lookup with the
`BuiltinName` that exists for `type`.
@robobun @Jarred-Sumner
AOT: the table of built-ins says that a Blob converts its type
ab1e0c1 
The rows of `new Blob()` and of `Blob.prototype.slice` said that the
`type` member and the operands of `slice` are only inspected. A
conversion to a string runs the code of an object. That was already so
on main for a String object with a `toString` of its own: a compiled
program dropped such a conversion in a call whose result nothing uses.

The two declarations are the lines that #47605 has for them, so the two
changes merge in either order.

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
@robobun
bun-types: say what the type of a BlobPropertyBag is
f0454d9 
The comment said that `type` is not yet implemented.
@robobun
robobun requested a review from alii as a code owner November 2, 2026 10:57
@github-actions github-actions Bot added the claude label Nov 2, 2026
@robobun

robobun commented Nov 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author
Updated 7:29 AM PT - Nov 2nd, 2026

❌ @robobun, your commit f0454d9 has 40 failures in Build #136872 (All Failures):

🧪   To try this PR locally:

bunx bun-pr 47740

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

bun-47740 --bun

@robobun

robobun commented Nov 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Status: ready for review. One question for a maintainer is the first item under Downsides ({ type: null } gives the type "null").

How I reproduced it. On main 3f3a9ff (release and debug builds), against Node.js v26.3.0:

import { MIMEType } from "node:util";
const type = new MIMEType("text/html");
console.log(new Blob(["<p>"], { type }).type); // Bun: ""    Node.js: "text/html"
console.log(new File(["<p>"], "n", { type }).type); // Bun: ""    Node.js: "text/html"
console.log(new Blob(["<p>"]).slice(0, 1, type).type); // Bun: ""    Node.js: "text/html"
console.log(new Response(new Blob(["<p>"], { type })).headers.get("content-type")); // Bun: null

test/js/web/fetch/blob.test.ts: 29 of the 46 new tests fail on main, and all 46 pass on this branch.

CI. Build #136872 is red in 40 test files: 39 under test/aot/ and test/bundler/compile-aot-size.test.ts. The same 40 files are red in the final build of the last merged PR (#136399 of #47622), so they are red without this change. No test that this PR adds or changes is among them. compile-aot-size.test.ts maps 25,953,252 bytes here and 25,961,444 bytes there, against a limit of 25,880,000. The build is finished: all 188 jobs ran, the ASAN lane included.

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 271b65e and f0454d9.

Files selected for processing: 6
  • packages/bun-types/globals.d.ts
  • src/codegen/builtins/host/web-classes.builtins.ts
  • src/runtime/webcore/Blob.rs
  • test/aot/parallel/host-classes/a-blob-converts-a-string-object-whose-tostring-nothing-else-calls.js
  • test/aot/parallel/host-classes/a-blob-converts-a-type-that-is-not-a-string.js
  • test/js/web/fetch/blob.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

Blob, File, and Blob.slice now convert supplied type values to strings. The changes add tests for conversion behavior, argument handling, errors, and propagation to responses and uploads.

Changes

Blob type conversion

Layer / File(s) Summary
Convert Blob and File type values
packages/bun-types/globals.d.ts, src/codegen/builtins/host/web-classes.builtins.ts, src/runtime/webcore/Blob.rs		 Blob and File type options and Blob.slice content types use string conversion. Blob and File constructors share type-setting logic that accepts content types only when every byte is in the specified printable ASCII range. The Blob type comment now describes its MIME type.
Validate conversion and argument behavior
test/aot/parallel/host-classes/a-blob-converts-*, test/js/web/fetch/blob.test.ts		 Tests cover conversion counts, getter reads, thrown errors, conversion order, inherited types, and which slice argument supplies the content type.
Verify MIME type propagation
test/js/web/fetch/blob.test.ts		 Tests cover MIMEType conversion in BuildArtifact.slice and converted types in Response headers, served responses, and uploads.

Suggested reviewers: jarred-sumner

Merge Risk: Low · up to f0454

Type conversion works for nonempty slices, but empty Blob slices still skip conversion and its errors. This is a bounded compatibility gap that should be fixed or explicitly accepted before merging.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the main change: converting non-string type values in Blob, File, and slice operations.
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.
Description check Passed The description is complete and directly explains the problem, implementation, behavior changes, scope, known limitations, and verification results. It does not use the exact template headings, but it…
Full details: Description check

Explanation

The description is complete and directly explains the problem, implementation, behavior changes, scope, known limitations, and verification results. It does not use the exact template headings, but it clearly covers both required topics: what the PR does and how the code was verified.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

coderabbitai[bot]
coderabbitai Bot reviewed Nov 2, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment •
edited
Loading

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.

  • Fix comments: Fix CodeRabbit comments on this PR

Comment thread test/js/web/fetch/blob.test.ts Resolved
claude[bot]
claude Bot reviewed Nov 2, 2026
View reviewed changes

@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.

Beyond the inline findings, I also checked the switch from options.get(global_this, "type") to fast_get(..., BuiltinName::Type) in the new set_type_from_options helper: it still walks the prototype chain via getIfPropertyExists and maps undefined to absent, so the WebIDL "only undefined is missing" rule holds and behavior is otherwise unchanged. The ? after to_utf8 runs before any Blob field is touched and no raw pointer is held across the user toString call, and the AOT table marking start/end as converted is consistent with native code re-routing a String object in those positions to contentType.

Extended reasoning...

The change makes Blob/File type and slice contentType ToString-convert non-string values in src/runtime/webcore/Blob.rs, updates the AOT built-ins table and a .d.ts JSDoc, and adds tests; it touches no auth, crypto or injection surface. Four inline findings were posted, and the ruled-out note covers the fast_get semantics, exception propagation and the AOT row that were examined separately.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

  • 🟡 src/runtime/webcore/Blob.rs — Callers of Bun.file(path, { type }) with a MIMEType object, number or array still get a Blob whose type is the extension-derived default, while the same value in new Blob, new File and slice is now converted. Blob.rs:5406-5409 maps every non-string type to BlobType::Ignored and drops it. After this PR a program that builds a type object once and passes it to both new File and Bun.file sees two different types. Fix: convert the type member of Bun.file options with the same ToString rule (or document and test that Bun.file intentionally differs).

    Why this was flagged

    Input: Bun.file("a.bin", { type: new MIMEType("text/made-up") }) or { type: 1 }, entered through the file host function at Blob.rs:5369. Blob.bind.ts:54 declares BlobType with an ignored: b.any arm, and Blob.rs:5406-5409 turns that arm into None, so the type falls back to the path-derived MIME type. On the base this is the same, so it is pre-existing. What changes with this PR is that the three sibling type sites (Blob.rs:1807, Blob.rs:5268 via both constructors) now ToString every present value, so a user who moves the same options object between new File and Bun.file now gets "text/made-up" from one and "application/octet-stream" from the other. The PR text scopes the change to the File API, but REVIEW.md asks for every sibling site sharing the pattern to be fixed or explicitly excluded with a stated reason in the PR; the Blob.bind.ts:53 comment is the only statement of intent. Remedy: ToString-convert Bun.file's type too, or state the exclusion and add a test pinning the divergence.

    Verification: A program passes a non-string type to Bun.file(path, { type }). src/runtime/webcore/Blob.rs:5404-5410 maps BlobType::Ignored(_) => None, so the type stays the extension-derived default. This code is untouched by the diff, so Bun.file alone behaves exactly as on the base. The new test matrix at test/js/web/fetch/blob.test.ts:145-153 does not cover Bun.file(path, { type }).

  • 🟣 src/runtime/webcore/Blob.rs — pre-existing: callers slicing an empty Blob get no conversion of contentType, unlike every other entry point this PR converts. Blob.rs:1765-1771 returns before the new !content_type.is_undefined() branch at Blob.rs:1807 runs, so new Blob([]).slice(0, 0, Symbol()) does not throw, a toString is never called, and a type getter side effect never happens, while a one-byte receiver throws or converts. The AOT row now declares slice as operandCodeOnly with contentType converted, which this path does not do. Fix: convert contentType (and run its code) before the size-0 early return, or at least convert on that path, so all slice receivers behave the same; the WPT case null type Blob slice needs this too.

    Why this was flagged

    Caller invokes blob.slice(start, end, contentType) on a receiver whose self.size.get() is 0, for example new Blob([]).slice(0, 0, { toString() { throw err } }) or new Blob([]).slice(0, 0, Symbol()). At src/runtime/webcore/Blob.rs:1765-1771 the method returns an empty Blob immediately, never reaching the new conversion at Blob.rs:1807-1814, so no user code runs and no TypeError is thrown, whereas new Blob(["a"]).slice(0, 0, Symbol()) throws "Cannot convert a symbol to a string". The base branch has the same early return, so the inconsistency is pre-existing; the PR description says the web-platform-test null type Blob slice still fails for this reason and defers it to #47335. The new table row in src/codegen/builtins/host/web-classes.builtins.ts:455-460 declares the operand code runs, which is not true on this path (safe for the optimizer, but the observable behaviour differs from the constructors).

    Verification: slice() on a Blob/File of size 0 with a non-undefined contentType. src/runtime/webcore/Blob.rs:1765-1771: the size-0 early return runs before the conversion branch at 1807. So new Blob([]).slice(0, 0, Symbol()) returns an empty Blob with no TypeError, while a one-byte receiver throws. The base branch fails identically; the new tests in test/js/web/fetch/blob.test.ts only use non-empty receivers.

  • 🟣 src/runtime/webcore/Blob.rs — Callers passing a non-number start or end to slice (an object with valueOf, a BigInt, a numeric string) still get a silently wrong range, and this PR now tells the AOT compiler those operands are converted. Blob.rs:1787 and Blob.rs:1796 read start/end only via is_number(), so slice({valueOf:()=>1}, 2) slices from 0 and slice(1n) returns the whole blob, while the File API declares [Clamp] long long (ToNumber). Fix: convert start and end with a coercing to_number()? before clamping, matching the converted row the PR adds at web-classes.builtins.ts:458, and test object, BigInt and string-number inputs.

    Why this was flagged

    Input: blob.slice(obj, 2) where obj is { valueOf() { return 1 } }, or blob.slice(1n, 3n), from any user code calling Blob.prototype.slice. Blob.rs:1787 if start.is_number() and Blob.rs:1796 if end.is_number() skip any non-number without conversion, so relative_start stays 0 and relative_end stays size. The base branch has identical lines (the diff hunk at 1803 leaves 1787-1803 untouched), so this is pre-existing, but the PR changes the sibling guard at Blob.rs:1807 to spec ToString for contentType and rewrites the same function's AOT row at web-classes.builtins.ts:458 to start: converted, end: converted, deleting the comment 'An operand of another kind than a number or a string counts as absent'. The table now describes a conversion native code does not perform. Node.js and browsers slice from 1 for both inputs; Bun returns bytes 0..2 or the whole blob. The whole-class rule in REVIEW.md asks for every operand of the same WebIDL signature to be fixed together. Remedy: ToNumber-convert start/end (to_number(global)? then truncate/clamp) in slice and keep the table row honest.

    Verification: pre-existing. Triggering condition: any user code passes a non-number, non-string value as start or end to Blob.prototype.slice. Mechanism verified at src/runtime/webcore/Blob.rs:1787 if start.is_number() { and :1796 if end.is_number() { — any value that is not a JS number is skipped with no ToNumber conversion. The base branch fails identically by the same route.

Comment thread src/runtime/webcore/Blob.rs
Comment on lines +5267 to +5268
if let Some(content_type) = options.fast_get(global_this, jsc::BuiltinName::Type)? {
let content_type_str = content_type.to_utf8(global_this)?;

@claude claude Bot Nov 2, 2026

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.

🔴 Programs that pass a MIME lookup result as type now upload Content-Type: null or Content-Type: false where the base sent no header. set_type_from_options at Blob.rs:5268 calls to_utf8 on every present value, so null and false become the text "null" / "false" and pass is_valid_blob_type. This follows from the stated DOMString purpose and the author flags it as a maintainer decision, but it is a silent wire-level break for servers that reject unknown media types. Fix: a maintainer must accept it explicitly; if the spec behaviour stays, say in the type JSDoc at packages/bun-types/globals.d.ts:1569 and the release notes that null/false become "null"/"false" and are sent as the Content-Type. [also at: src/runtime/webcore/Blob.rs:1807 - Programs that slice with a null or false contentType now get a sub-Blob typed "null" or "false" and upload Content-Type: null, where the base inherited or dropped the type.]

Why this was flagged

A program does new File([buf], name, { type: mimeTypes.lookup(name) }) or new Blob([body], { type: res.headers.get("content-type") }) and the lookup misses, giving false or null. On the base, jsdom_blob_construct/jsdom_file_construct tested content_type.is_string() first and ignored the value, so .type was "" and a fetch upload carried no Content-Type. After the change, set_type_from_options at src/runtime/webcore/Blob.rs:5268 runs to_utf8 on the value, "null"/"false" is all printable ASCII so is_valid_blob_type at Blob.rs:74 accepts it, and Blob.rs:5271-5272 store it as the type. A receiving server that validates Content-Type now gets a 415-class failure that it did not get before. No guard in the diff prevents it; the PR description lists it under Downsides as a question for a maintainer, and the .d.ts hunk at packages/bun-types/globals.d.ts:1569 does not mention it.

Verification: set_type_from_options at src/runtime/webcore/Blob.rs:5262-5276 calls content_type.to_utf8(global_this)? (giving "null"/"false") and is_valid_blob_type at Blob.rs:74-76 only checks printable ASCII, so these pass and content_type_was_set is set. Base code (removed hunks at old lines 2159-2170 and 5318-5329) guarded with content_type.is_string(), leaving .type === "".

@robobun robobun Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agreed: a maintainer must accept this. It is the first item under Downsides, and the Notes have the table of what changes on the wire.

The facts for that decision:

  • WebIDL takes only undefined as absent for a DOMString. Node.js v26.3.0 gives "null" and "false" at the three sites. web-platform-tests asserts "null" for slice (null type Blob slice in FileAPI/blob/Blob-slice.any.js).
  • The cost is the one that you name. A program that passed the result of a failed lookup sent no header. It now sends Content-Type: null.

The code and the JSDoc stay as they are until a maintainer answers, because the two answers need different text. If the conversion stays, I add a sentence on null and false to the JSDoc of BlobPropertyBag.type. If it does not, the constructors keep null and false absent.

This thread stays open for that answer.

@robobun

robobun commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

Replies to the three findings outside the diff in this review. None of them changes the code of this PR.

Bun.file(path, { type }). Excluded on purpose. Bun.file is Bun's own API, and its schema declares another rule (src/runtime/webcore/Blob.bind.ts:53): "Anything but a string is ignored, and so is """. There type replaces the type that Bun takes from the path, and a value that gives none keeps that type:

main, and this PR
Bun.file("a.bin", { type: new MIMEType("text/made-up") }).type "application/octet-stream", and toString does not run
Bun.file("a.txt", { type: "" }).type "text/plain;charset=utf-8"
Bun.file("s3://b/k", { type: 1 }) TypeError ERR_INVALID_ARG_TYPE

So Bun's own options already have two rules, and neither is ToString. Which rule they share is a separate decision. The Notes say this under "Not changed".

A receiver of size 0. The answer is in this thread. The return drops a string too, so a conversion before it still gives no type. #47335 moves the return below the contentType block. The PR body names the case under Downsides.

start and end. Not in this PR, for two reasons.

On the table row: converted for start and end is an upper bound, and the row needs it. A String object in one of those places is the contentType, and native code converts it. test/aot/parallel/host-classes/a-blob-converts-a-string-object-whose-tostring-nothing-else-calls.js fails with the old row.

@coderabbitai coderabbitai Bot mentioned this pull request Nov 6, 2026
Open
@robobun robobun mentioned this pull request Nov 10, 2026
Open

This branch has not been deployed

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

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments
@claude claude[bot] claude[bot] left review comments
@alii alii Awaiting requested review from alii alii is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@Jarred-Sumner Jarred-Sumner

Labels

claude

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@robobun @Jarred-Sumner