Move name and lastModified from Blob.prototype to File.prototype - #32434
Move name and lastModified from Blob.prototype to File.prototype#32434robobun wants to merge 6 commits into
name and lastModified from Blob.prototype to File.prototype#32434Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplaces the previous ChangesJSDOMFile prototype/constructor split and Blob property migration
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 3:03 PM PT - Aug 16th, 2026
✅ @robobun, your commit ac2df5c8956c87bd3087a66c930146204aa92d98 passed in 🧪 To try this PR locally: bunx bun-pr 32434That installs a local version of the PR into your bun-32434 --bun |
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Ready for review at One substantive addition since the last green run: Blob Deferred as follow-ups per review discussion: the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/jsc/bindings/JSDOMFile.cpp (2)
177-181:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRelease the ThrowScope on the throwing tail call.
throwTypeErrorleaves a pending exception; return throughRELEASE_AND_RETURNso exception-scope validation sees the intentional throw.Proposed fix
auto scope = DECLARE_THROW_SCOPE(lexicalGlobalObject->vm()); throwTypeError(lexicalGlobalObject, scope, "Class constructor File cannot be invoked without 'new'"_s); - return {}; + RELEASE_AND_RETURN(scope, JSValue::encode(jsUndefined()));As per coding guidelines, "
**/*.cpp: Exception checks required after every call that can throw or run user code" and throwing tail calls should use the checked propagation path.🤖 Prompt for 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. In `@src/jsc/bindings/JSDOMFile.cpp` around lines 177 - 181, In the static call method of JSDOMFile.cpp, the throwTypeError function call leaves a pending exception but the function returns with a simple return statement. Replace the return {}; with RELEASE_AND_RETURN(scope, JSValue()); to properly release the ThrowScope and use the checked propagation path for the throwing tail call, ensuring exception-scope validation sees the intentional throw as required by the coding guidelines.Source: Coding guidelines
149-175:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCheck for constructor exceptions before wrapping
ptr.
JSDOMFile__constructparses/coerces constructor arguments and can throw or run JS. The returned native pointer is wrapped withoutRETURN_IF_EXCEPTION, which can trip JSC exception validation or return a value with a pending exception.Proposed fix
auto* globalObject = defaultGlobalObject(lexicalGlobalObject); auto& vm = JSC::getVM(globalObject); + auto throwScope = DECLARE_THROW_SCOPE(vm); JSObject* newTarget = asObject(callFrame->newTarget()); auto* constructor = globalObject->JSDOMFileConstructor(); Structure* structure = globalObject->JSDOMFileStructure(); if (constructor != newTarget) { - auto scope = DECLARE_THROW_SCOPE(vm); - auto* functionGlobalObject = defaultGlobalObject( // ShadowRealm functions belong to a different global object. getFunctionRealm(lexicalGlobalObject, newTarget)); - RETURN_IF_EXCEPTION(scope, {}); + RETURN_IF_EXCEPTION(throwScope, {}); structure = InternalFunction::createSubclassStructure(lexicalGlobalObject, newTarget, functionGlobalObject->JSDOMFileStructure()); - RETURN_IF_EXCEPTION(scope, {}); + RETURN_IF_EXCEPTION(throwScope, {}); } void* ptr = JSDOMFile__construct(lexicalGlobalObject, callFrame); + RETURN_IF_EXCEPTION(throwScope, {}); if (!ptr) [[unlikely]] { - return JSValue::encode(JSC::jsUndefined()); + RELEASE_AND_RETURN(throwScope, JSValue::encode(JSC::jsUndefined())); } - return JSValue::encode( - WebCore::JSBlob::create(vm, globalObject, structure, ptr)); + RELEASE_AND_RETURN(throwScope, JSValue::encode( + WebCore::JSBlob::create(vm, globalObject, structure, ptr)));As per coding guidelines, "
**/*.cpp: Exception checks required after every call that can throw or run user code; use RETURN_IF_EXCEPTION or JSError propagation before using results".🤖 Prompt for 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. In `@src/jsc/bindings/JSDOMFile.cpp` around lines 149 - 175, The construct method in JSDOMFile.cpp calls JSDOMFile__construct which can throw or run user code, but the returned ptr value is wrapped in WebCore::JSBlob::create without checking for exceptions. Add a RETURN_IF_EXCEPTION check after the JSDOMFile__construct call and before the JSBlob::create wrapping, using the existing scope variable that was declared in the constructor subclass path. This ensures that if an exception occurs during construction, the method returns early rather than attempting to wrap a potentially invalid pointer.Source: Coding guidelines
🤖 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/jsc/bindings/JSDOMFile.cpp`:
- Around line 15-17: The File.name property currently has a writable setter
(JSDOMFile__setName) which allows mutation of file metadata, while lastModified
is readonly. To align with the W3C File API specification, remove the
JSDOMFile__setName function declaration and its associated installation at line
69, then apply the ReadOnly attribute to the name property descriptor to match
the immutable pattern used for lastModified.
- Around line 50-51: The JSDOMFile__setName function can throw exceptions during
value coercion or storage, but the code does not validate whether an exception
occurred before using the returned bool result. After the JSDOMFile__setName
call and before the RELEASE_AND_RETURN statement, add an exception check using
RETURN_IF_EXCEPTION or appropriate JSError propagation to ensure any pending
exceptions are properly handled before the result is returned.
In `@src/runtime/webcore/Blob.rs`:
- Around line 2014-2016: The slice() method currently only clears is_jsdom_file
but retains the file-backed store from dupe(), allowing is_bun_file() to promote
it back to a File when to_js() is called later. To fix this and ensure
Bun.file().slice() returns a plain Blob as per the File API spec, either route
the slice result through a plain JSBlob wrapper that strips the file-backed
store entirely, or add an explicit state flag (like is_plain_blob_from_slice)
that prevents the is_bun_file() check in the to_js path from promoting sliced
blobs back to File objects.
---
Outside diff comments:
In `@src/jsc/bindings/JSDOMFile.cpp`:
- Around line 177-181: In the static call method of JSDOMFile.cpp, the
throwTypeError function call leaves a pending exception but the function returns
with a simple return statement. Replace the return {}; with
RELEASE_AND_RETURN(scope, JSValue()); to properly release the ThrowScope and use
the checked propagation path for the throwing tail call, ensuring
exception-scope validation sees the intentional throw as required by the coding
guidelines.
- Around line 149-175: The construct method in JSDOMFile.cpp calls
JSDOMFile__construct which can throw or run user code, but the returned ptr
value is wrapped in WebCore::JSBlob::create without checking for exceptions. Add
a RETURN_IF_EXCEPTION check after the JSDOMFile__construct call and before the
JSBlob::create wrapping, using the existing scope variable that was declared in
the constructor subclass path. This ensures that if an exception occurs during
construction, the method returns early rather than attempting to wrap a
potentially invalid pointer.
🪄 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: 76e87ba3-fff3-4c49-96c2-0bd053015954
📒 Files selected for processing (11)
src/jsc/bindings/JSDOMFile.cppsrc/jsc/bindings/JSDOMFile.hsrc/jsc/bindings/JSS3File.cppsrc/jsc/bindings/ZigGlobalObject.cppsrc/jsc/bindings/ZigGlobalObject.hsrc/jsc/bindings/ZigGlobalObject.lut.txtsrc/jsc/bindings/blob.cppsrc/runtime/webcore/Blob.rssrc/runtime/webcore/response.classes.tstest/js/bun/util/inspect.test.jstest/js/web/fetch/blob.test.ts
💤 Files with no reviewable changes (1)
- test/js/bun/util/inspect.test.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 4338-4340: Remove the is_bun_file() check from the condition on
line 4338 that determines whether to deserialize as a File. Instead, ensure that
Bun.file() payloads are explicitly marked with a File indicator during
serialization before wrapping. Update the deserialization logic to only promote
to File based on this explicit marker (likely through is_jsdom_file or a similar
explicit flag), not through the is_bun_file() method check. This prevents slices
of Bun.file() blobs (which clear is_jsdom_file but retain the file store) from
being incorrectly promoted back to File, preserving the slice invariant where a
slice of a Blob remains a Blob.
🪄 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: 2de364e9-4cc5-440a-9e10-82c3ec69d4e4
📒 Files selected for processing (2)
src/runtime/webcore/Blob.rstest/js/web/fetch/blob.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/jsc/bindings/DOMFormData.cpp`:
- Around line 160-163: The `set` method that takes a name, blob, and filename
parameters is not applying the same filename sanitization that the `append`
method uses. Instead of directly passing the filename to `blob->setFileName()`,
apply the `replaceUnpairedSurrogatesWithReplacementCharacter()` function to the
filename (handling both null and non-null cases) before calling setFileName,
ensuring consistent behavior between the `set` and `append` methods for filename
sanitization.
🪄 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: e0777294-c33a-4acf-9612-11879b781f6e
📒 Files selected for processing (3)
src/jsc/bindings/DOMFormData.cppsrc/jsc/bindings/blob.cppsrc/runtime/api/Archive.rs
|
Do you need to make it so |
|
Yes, done in f329ef6. if self.is_s3() { return s3_file::to_js_unchecked(...); }
if self.is_jsdom_file.get() { return dom_file_to_js_unchecked(...); }
js::to_js_unchecked(...)To make that hold everywhere, I did not fold One knock-on: now that |
3541162 to
5fb5f36
Compare
|
Rebased onto The only conflict was - File GlobalObject::m_JSDOMFileConstructor CellProperty
crypto GlobalObject::m_cryptoObject CellProperty
...
Crypto GlobalObject::m_JSCrypto ClassStructure
+ File GlobalObject::m_JSDOMFileClassStructure ClassStructure
Re-verified on the rebased tree:
|
5fb5f36 to
6cdb0e7
Compare
|
Rebased onto main (511caaa) and re-verified. Every case from the File/Blob prototype report now matches Node: Added explicit assertions for |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Triage update: #30328 is the PR landing the File prototype split (#32430 and #31656 were closed in its favor), and #32425 was closed as the narrower alternative to this one. This PR stays open for the part #30328 does not do, removing |
889434c to
fc7cd7d
Compare
There was a problem hiding this comment.
I reviewed the latest push (55c31cf switching the is_bun_file() inspect arm to !name.is_empty(), plus autofix) and this pass found nothing new — that commit resolves the last inline nit about structuredClone(Bun.file(fd)) printing a spurious name: "".
I'm not auto-approving because the open design question in the description (whether Bun.file() / Bun.stdin etc. should inherit from File.prototype and become instanceof File, vs. a separate BunFile prototype) and the overlap with #30328 are maintainer calls, and the change spans JSC structure/prototype plumbing across 19 files.
What was reviewed
show_namenow keys on!name.is_empty()and compares against the header'sget_file_name(), so both the fd-clone and thenew File([Bun.file(p)], "custom.txt")cases render correctly (both covered ininspect.test.js).LazyClassStructurewiring:m_JSDOMFileClassStructureis inFOR_EACH_GLOBALOBJECT_GC_MEMBERso it's visited;init.setConstructorsetsFile.prototype.constructorbefore any global lookup (covered by the "constructor is set before the File global is touched" test).is_jsdom_filereset on every path that yields a plain Blob from a File source:slice(),new Blob([file]),body.blob()(both arms), and the v1-only structured-clone fallback.
Extended reasoning...
Overview
This PR splits File.prototype from Blob.prototype and moves the name/lastModified accessors onto the new prototype. It touches 19 files: hand-written JSC C++ (JSDOMFile.cpp gains a JSDOMFilePrototype/JSDOMFileConstructor pair and a LazyClassStructure initializer; ZigGlobalObject.{h,cpp} and the .lut.txt swap the File global from a CellProperty to a ClassStructure; blob.cpp and DOMFormData.cpp route File-flavoured wrappers through the new structure; JSS3File.cpp re-parents the S3 prototype), Rust runtime (Blob.rs gains dom_file_to_js_unchecked plus three new host ABI exports and threads is_jsdom_file through to_js, slice(), the Blob constructor's single-part dupe path, structured-clone deserialize and construct_bun_file; Body.rs, BunObject.rs, standalone_graph_jsc.rs, ObjectURLRegistry.rs follow), the response.classes.ts codegen input, and five test files.
Since my last inline comment (03:42Z), 55c31cf changed the is_bun_file() arm of show_name from name.tag() != Tag::Dead to !name.is_empty(), and added a regression assertion (Bun.inspect(structuredClone(Bun.file(123))) equals the un-cloned output). That was the fix I suggested, and it also composes correctly with the earlier new File([Bun.file(p)], "custom.txt") case because get_file_name() returns the store path, not blob.name, so the equality check only suppresses the line when they genuinely match.
Security risks
None identified. No untrusted input parsing was added; the FormData change only substitutes emptyString() for a null WTF::String before the existing surrogate-replacement call. The new BUN__createJSDOMFileUnsafely mirrors the generated Blob__create (same JSBlob::create + reportExtraMemoryAllocated), taking ownership of the passed pointer the same way. The m_name WriteBarrier remains on JSBlob via the values: ["stream", "name"] list, so the cached JS name string is still visited by visitChildrenImpl.
Level of scrutiny
High. This is a user-visible API-shape change (removing two accessors from Blob.prototype, changing Bun.file() instanceof File from false to true, re-parenting S3File.prototype) implemented across hand-written JSC prototype/structure code, a LazyClassStructure on ZigGlobalObject, generated-class inputs, and Rust FFI. The PR description explicitly flags a design decision for maintainers — whether Bun.file()/stdio/embedded files should sit on File.prototype (this branch) or a dedicated BunFile prototype — and notes that the prototype-split plumbing overlaps #30328 and will need reworking on top of it. Those are exactly the kind of choices a human maintainer should sign off on.
Other factors
The three earlier deferred nits (WebIDL brand check accepting a plain Blob receiver on the extracted File.prototype.name getter; FormData defaulting a nameless Blob entry to "" instead of the spec's "blob"; both explicitly acknowledged as follow-ups) remain open but are strict improvements over pre-PR behavior and don't block. Test coverage for the new shape is thorough (prototype chain, Symbol.toStringTag, slice()/body.blob()/new Blob([file])/structuredClone staying Blob, embedded-file File-ness in bundler_compile.test.ts, the #14257 subclass-assignment scenario, and the lazy-init constructor test). The earlier robobun note about a merge conflict in Blob.rs may still apply.
### Problem - mordant's `reimplemented_helper` finding baselined for `src/jsc/webcore_types.rs`: `Blob::is_bun_file` and `Blob::needs_to_read_file` had the same signature and the same body (the store is `Some` and its data is `Store::File`), so a change to one would have silently missed the other. - The duplication is inherited, not a porting mistake: `Blob.zig` already had `isBunFile` and `needsToReadFile` with the same body, and neither side has changed since the port (#30412). Nothing relies on them differing. ### Fix - Delete `is_bun_file` and keep `needs_to_read_file`, which is what the ~40 other call sites (and the `blob_needs_to_read_file` hook that `bun_sql_jsc` goes through) already use. - Point the two `is_bun_file` callers at it: `Response.rs` (`getCompleteWebRequestOrResponseBodyValueAsArrayBuffer` returns `undefined` for a file-backed body) and `CryptoHasher.rs` (the synchronous hashers reject `Bun.file()` input). Same predicate, so no behavior change. - Remove the `reimplemented_helper:src/jsc/webcore_types.rs` entry from `mordant-baseline.toml`, and the two stale comments in `webcore/Blob.rs` that named the deleted method. The entry is removed by hand rather than by regenerating the file: a full `bun run rust:mordant:baseline` on Linux also drops two unrelated entries (`always_unwrapped_option` in `PackageInstall.rs`, `narrowed_two_ways` in `node_crypto_binding.rs`) that this PR has no business touching. - No new test: there is no observable behavior to pin (the two predicates were byte-for-byte the same), so no test can distinguish before from after. The `Bun.file()` rejection at the `CryptoHasher` call site is already covered by `bun-cryptohasher.test.ts` ("Bun.file in CryptoHasher is not supported yet"), and the mordant run below is the check for the finding itself. Same shape as #39124, #39116 and #39135 from this batch. - Verified: - `bun run rust:mordant` (full workspace) on this branch: clean, no `target/mordant/over-baseline.txt`. With main's `webcore_types.rs` restored on top of the trimmed baseline it reports `reimplemented_helper ... over the mordant baseline (0 recorded for src/jsc/webcore_types.rs)` and `1 finding(s) over the baseline in bun_jsc`, so the entry was this site and the lint no longer fires. - The `mordant` workflow also runs on this PR since the baseline changed; with the entry removed it fails if the finding is still reported. - `bun bd test test/js/bun/util/bun-cryptohasher.test.ts` (covers the `CryptoHasher` call site: `Bun.file()` input still throws): 402 pass. - `bun bd test` on `test/js/web/fetch/{blob,blob-write,body,blob-file-name-ownership,response}.test.ts`, `test/js/web/structured-clone-blob-file.test.ts` and `test/js/bun/io/bun-write.test.js` (`--timeout 60000`, since several of these spawn a debug+ASAN child and exceed the 5s default): everything passes except `blob.test.ts` "Bun.file(path).slice(start, end) streams only the slice", which fails identically on a clean checkout of main (88a6398) and is being handled separately; it is a `FileReader` streaming bug unrelated to this predicate. - Two open PRs add `is_bun_file()` calls in `Blob.rs` (#32434, #33659); whichever lands after this one needs those calls renamed to `needs_to_read_file()`. That is a compile error, not a silent change. ### Background - `Blob` (`src/jsc/webcore_types.rs`) is a view (offset + size) onto a refcounted `Store`, whose `data` is one of `Bytes` (in memory), `File` (a path or fd; what `Bun.file()` creates) or `S3`. `needs_to_read_file()` asks whether the blob is `File`-backed, i.e. whether its bytes have to be read off disk before anything in memory can look at them; synchronous consumers use it to bail out. - mordant is the advisory Rust lint pack the `rust-lints` workflow runs (`bun run rust:mordant`). `mordant-baseline.toml` holds the per-(lint, file) counts of findings that predate the job, so CI only fails when a count goes up; once a baselined finding is fixed its entry is deleted. Co-authored-by: Alistair Smith <hi@alistair.sh>
`"name" in new Blob()` returned true because `File.prototype` and `Blob.prototype` were the same object: `File` was an InternalFunction with a patched `Symbol.hasInstance`, not a real class with its own prototype. Give `File` a real `JSDOMFilePrototype` / `JSDOMFileConstructor` pair registered through `LazyClassStructure`, so `Object.getPrototypeOf(File.prototype) === Blob.prototype` and `name` / `lastModified` live only on `File.prototype`. `is_jsdom_file` is now the single source of truth for which prototype a Blob's JS wrapper gets: `Blob::to_js()` routes to the File structure when it is set (alongside the existing `is_s3()` branch), so call sites no longer pick the structure themselves. `Bun.file()`, `Bun.stdin/stdout/ stderr`, embedded assets and archive entries set the flag at creation; `slice()`, `new Blob()` and `Body.blob()` clear it. Structured-clone deserialize honors the serialized flag for every payload version that carries it. Side effects: - `File.prototype.name` returns "" instead of undefined when there is no backing path (`Bun.file(fd)`, `new File([], "")`). File.name is a USVString, so this matches Node and browsers and makes `structuredClone(Bun.file(fd)).name` round-trip. - `Bun.inspect` no longer prints a redundant `name:` line for a FileRef header that already shows the path; a File constructed over a BunFile with its own name still shows it. Fixes #20700 Fixes #14257 Fixes #14102
3c80700 to
cf3ed26
Compare
|
Rebased onto Two small conflicts, both in
Re-verified on the rebased tree: |
… File Bun.file() now serializes is_jsdom_file=1, so a 0 on a file-backed store means slice()/body.blob(). Payloads written before this change wrote 0 for Bun.file() itself; without a version bump they would deserialize as plain Blobs and lose .name/.lastModified. Promote file-backed payloads older than v5 and keep the byte authoritative from v5 on.
The slice is a plain Blob at runtime (no name/lastModified), matching File.prototype.slice() in the File API.
There was a problem hiding this comment.
I reviewed this PR and the bug-hunting pass found no new issues — every finding from the prior ~17 review rounds has been addressed. Given the scope (new File.prototype + structure on ZigGlobalObject, serialization format bump to v5, Bun.file()/Bun.stdin/S3File now inheriting File.prototype) and the open design question the description explicitly flags for maintainers — whether Bun.file() instanceof File should become true or whether a separate BunFile prototype is preferable — a human sign-off is still worthwhile.
What was reviewed: prototype-chain wiring (JSDOMFilePrototype/LazyClassStructure, File.prototype.constructor set via init.setConstructor, S3File chained through File); every File-producing path routes through is_jsdom_file → to_js() (Bun.file, stdio, embedded files, structuredClone, FormData, resolveObjectURL, Archive); every Blob-producing path clears the flag (slice(), new Blob([file]), Body.blob()); m_name WriteBarrier kept on JSBlob via values: and used by the hand-written getter; BUN__createJSDOMFileUnsafely reports extra memory like Blob__create; the v5 gate keeps pre-v5 file-backed payloads File-typed while v5 slices stay Blob.
Extended reasoning...
Overview
This PR splits File.prototype from Blob.prototype and moves the name/lastModified accessors onto File.prototype. It touches 21 files: a new JSDOMFilePrototype + JSDOMFileConstructor + LazyClassStructure on ZigGlobalObject (JSDOMFile.cpp/h, ZigGlobalObject.cpp/h/.lut.txt), the generated-class definition (response.classes.ts drops name/lastModified from Blob's proto but keeps name in values: for the WriteBarrier cache), blob.cpp/DOMFormData.cpp (toJS now routes on null-vs-set fileName), JSS3File.cpp (S3File.prototype now chains through File.prototype), Rust-side plumbing in Blob.rs/Body.rs/BunObject.rs/standalone_graph_jsc.rs/ObjectURLRegistry.rs to set/clear is_jsdom_file at every producer, a serialization format bump (v4→v5) with a back-compat gate, and bun.d.ts (BunFile.slice() now returns Blob). Six test files are updated/extended.
Security risks
None identified. No auth/crypto/permissions surface. The structured-clone deserializer change is a version-gated read of an already-trusted byte; no new untrusted-length arithmetic. The new C++ getters do dynamicDowncast<JSBlob> before dereferencing wrapped() and use RETURN_IF_EXCEPTION.
Level of scrutiny
High. This is user-facing Web API surface (File/Blob prototype identity, instanceof, Symbol.toStringTag), it changes what Bun.file() is an instance of (a documented Bun API), it bumps a persisted serialization format, and it adds a new cached structure to ZigGlobalObject. It has been through many review rounds with a dozen-plus resolved findings (slice/Body.blob/new Blob([file]) File-leak, WebSocket blob promotion, FormData entry wrapping, resolveObjectURL, embeddedFiles/compiled-asset flag consistency, reportExtraMemoryAllocated, DOMAttribute brand check, inspect show_name edge cases, structured-clone version gating, .d.ts drift), all now addressed with tests.
Other factors
The description explicitly leaves a design decision to maintainers: routing Bun.file()/stdio/embedded/S3File through File.prototype (so Bun.file(p) instanceof File === true, a behavior change) versus a separate BunFile prototype. It also notes overlap with #30328 (the prototype-split PR), with the plumbing here to be reworked on top of that once it lands. Jarred asked a design question on the thread earlier. Those are exactly the kinds of calls a human should make; the code itself looks correct for the approach taken.
2 participants
Repro
Per the W3C File API,
nameandlastModifiedbelong onFile.prototype, notBlob.prototype. Node, Deno, Firefox and Chrome all returnfalsefor"name" in new Blob().Cause
Filedid not have its own prototype.JSDOMFile.cpppointedFile.prototypedirectly at theJSBlobPrototypeobject and usedJSBlobStructure()fornew File():Because there was no separate
File.prototype, thenameandlastModifiedaccessors were placed onBlob.prototypeinresponse.classes.ts.Fix
Add
JSDOMFilePrototype([[Prototype]]=Blob.prototype) carrying thename/lastModifiedaccessors plusSymbol.toStringTag = "File", and a cachedm_JSDOMFileStructureonZigGlobalObjectthat points instances at it. The File constructor's own[[Prototype]]is now the Blob constructor andFile.prototype.constructor === File. TheSymbol.hasInstanceoverride onFileis removed since the prototype chain now makes defaultinstanceofwork.All File-producing paths route through the new structure:
new File()and subclasses,Bun.file(), structuredClone of Files, FormData file entries (WebCore::toJS/toJSNewlyCreated).S3File.prototypenow chains throughFile.prototypesoname/lastModifiedremain reachable there. Instances remainWebCore::JSBlobcells; only the structure/prototype differs.The
m_namecachedWriteBarrierstays onJSBlob(via thevalueslist in the class definition) so the Rust-side name cache continues to work.Verification
blob: can set lastModified propertyis the exact scenario from #14257 (assigninglastModifiedon aBloband in aBlobsubclass constructor now creates an own data property, as in Node;File.prototype.lastModifiedstays read-only).Existing tests in
blob.test.ts,globals.test.js,FormData.test.ts,structured-clone.test.ts,blob-file-name-ownership.test.tspass.Fixes #20700
Fixes #14257
Relationship to #30328
#30328 is the PR landing the prototype split itself (
File.prototypedistinct from and inheritingBlob.prototype,Symbol.toStringTag,constructor, ordinaryinstanceof; it resolves #14102 and #25422). #32430 and #31656 were closed in its favor. #30328 keepsnameandlastModifiedonBlob.prototype(while also defining them onFile.prototype), so this PR stays open for removing them fromBlob.prototype(#20700, #14257). The prototype plumbing here duplicates #30328 and will be dropped once that lands; what remains is theresponse.classes.tschange plus the question below. #32425 (a setter onBlob.prototype.lastModified) was closed as the narrower alternative to this.Point for maintainers: once the accessors leave
Blob.prototype, something has to keep the documentedBun.file().name/.lastModifiedworking. This PR does that by havingBun.file(),Bun.stdin/stdout/stderr, embedded files andS3Fileinherit fromFile.prototype, which also makesBun.file(path) instanceof Filetrue (it is false today, and #30328 leaves it false). An alternative would be a separate BunFile prototype carrying those two accessors.no test proof · iteration 13 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/bundler/bundler_compile.test.ts test/js/bun/util/inspect.test.js test/js/web/fetch/blob.test.ts test/js/web/html/FormData-multipart-serialization.test.ts test/js/web/structured-clone-blob-file.test.ts