Repository navigation
Conversation
Per https://xhr.spec.whatwg.org/#create-an-entry, when FormData.append or FormData.set receives a Blob that is not a File and no filename argument, the entry becomes a File named "blob". Bun was leaving the name empty, so multipart serialization emitted filename="" and the retrieved entry's .name was undefined. Also switch Blob__setAsFile to store the FormData-assigned filename on the per-Blob name field instead of the shared store's stored_name, so it can be overridden by a subsequent set()/append() with an explicit filename and does not leak into the user's original Blob. Fixes #14725
Walkthrough
ChangesBlob filename defaulting to "blob"
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Updated 5:34 PM PT - Jun 16th, 2026
❌ @robobun, your commit 7499288 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 32431That installs a local version of the PR into your bun-32431 --bun |
|
Found 3 issues this PR may fix:
🤖 Generated with Claude Code |
|
CI is blocked on infrastructure: linux build agents fail with |
Files retrieved from FormData carry their filename in the per-Blob name field (set by Blob__setAsFile), not the store's stored_name. Re-appending such a File to a FormData without a filename argument was returning empty because Blob__getFileNameString only looked at the store. Check the per-Blob name first, matching get_name_string.
When the parts array is a single Blob, Blob::get returns a dupe that inherits the source's per-Blob name. The Bytes arm previously only wrote the constructor's filename into stored_name, so the inherited name won in get_name_string/Blob__getFileNameString. Set blob.name unconditionally so the new File's .name always reflects the constructor argument.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/webcore/Blob.rs (1)
5652-5657:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid writing constructor names into a shared byte store.
Line 5657 still mutates
bytes.stored_nameeven though this path can be adupe()over a sharedStoreRefas noted above. That can leaknew File([blob], "x.txt")back into the original Blob, causing a laterFormData.append("k", blob)to derive"x.txt"fromget_file_name()instead of defaulting to"blob". Keep the constructor name onblob.name, or only populatestored_namewhen the byte store is uniquely owned.Proposed fix
blob.name.set(name_value_str.dupe_ref()); if let Some(store_) = blob.store.get() { - match store_.data_mut() { - store::Data::Bytes(bytes) => { + if store_.has_one_ref() { + if let store::Data::Bytes(bytes) = store_.data_mut() { // `get::<_, true>` on a single-Blob sequence returns // `dupe()` (a shared StoreRef), so this `Bytes` may already // carry an owned `stored_name` from the source blob; the // assignment drops (frees) the previous `Box<[u8]>`. bytes.stored_name = name_value_str.to_owned_slice().into_boxed_slice(); } - store::Data::S3(_) | store::Data::File(_) => {} } } else if !name_value_str.is_empty() {As per coding guidelines, “Store state on the object whose lifetime matches it.”
🤖 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/runtime/webcore/Blob.rs` around lines 5652 - 5657, The mutation of bytes.stored_name on line 5657 in the store::Data::Bytes branch can affect a shared StoreRef when bytes is obtained via dupe(), leaking the constructor name into the original Blob. To fix this, either avoid mutating bytes.stored_name when it could be shared and instead store the constructor name on the blob object itself whose lifetime matches the name's usage, or add a check to only populate bytes.stored_name when the byte store is uniquely owned (not a shared reference). Follow the principle that state should be stored on the object whose lifetime matches it.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.
Outside diff comments:
In `@src/runtime/webcore/Blob.rs`:
- Around line 5652-5657: The mutation of bytes.stored_name on line 5657 in the
store::Data::Bytes branch can affect a shared StoreRef when bytes is obtained
via dupe(), leaking the constructor name into the original Blob. To fix this,
either avoid mutating bytes.stored_name when it could be shared and instead
store the constructor name on the blob object itself whose lifetime matches the
name's usage, or add a check to only populate bytes.stored_name when the byte
store is uniquely owned (not a shared reference). Follow the principle that
state should be stored on the object whose lifetime matches it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2c096985-afa1-4f49-8dea-5aa1025bdd2b
📒 Files selected for processing (3)
src/runtime/webcore/Blob.rstest/js/web/html/FormData-multipart-serialization.test.tstest/js/web/html/FormData.test.ts
|
Re the CodeRabbit outside-diff note on |
There was a problem hiding this comment.
Both regressions I flagged are addressed and I have nothing further on this revision; deferring to a human since this reworks where Blob filenames live (per-Blob name vs shared store stored_name) across several call sites and CI hasn't yet run against the final diff.
Extended reasoning...
Overview
This PR changes FormData's default filename for plain-Blob entries from "" to "blob" per the XHR spec, and in doing so reworks how Blob filenames are stored: Blob__setAsFile now writes to the per-Blob name field instead of the shared store's stored_name, Blob__getFileNameString now consults name before the store and falls back to "blob", and jsdom_file_construct_ now sets blob.name unconditionally so the File constructor's name argument wins over anything inherited via dupe(). Tests in FormData.test.ts and FormData-multipart-serialization.test.ts are updated and extended with re-append and new File([f], name) round-trip cases.
Security risks
None apparent. The change affects which filename string is emitted in multipart bodies and returned from .name; it does not touch parsing of untrusted input, escaping, or any auth/permission path.
Level of scrutiny
Moderate-to-high. The diff is small but sits in core src/runtime/webcore/Blob.rs and shifts the source of truth for Blob filenames between two storage locations that are read by several distinct paths (get_name_string, get_file_name, Blob__getFileNameString, the File constructor's single-Blob fast path, FormData serialization). I flagged two separate regressions on earlier revisions of this PR — re-appending a FormData-retrieved File losing its name, and new File([f], newName) keeping the inherited name — both now fixed and covered by tests. That it took two iterations to converge is itself a signal that the per-Blob-name vs shared-stored_name interaction is subtle enough to warrant a human pass.
Other factors
CI failed on infrastructure both times (build agents could not be provisioned), so no automated tests have run against the current head; the author reports the FormData suite passes locally. The author also notes the pre-existing shared-store stored_name leak in jsdom_file_construct_ is left in place (kept for get_file_name() callers like loader detection / auto Content-Disposition) but is no longer observable via .name/FormData — that trade-off seems reasonable but is the kind of judgment call a maintainer should sign off on.
|
Heads up: #32915 touches the same area from a different angle (the |
|
A FormData differential pass flagged the same
Two things worth folding in while this function is open: 1. The no-filename arm still hands C++ a borrowed view of the store's bytes, and that is a reachable use-after-free on main.
const f = new File(["x"], "aaaa.txt");
const fd = new FormData();
fd.append("k", f);
new File([f], "bbbb.txt"); // frees the allocation m_fileName points into
console.log(await new Response(fd).text());On 1.4.0 the part's filename comes out as heap garbage that changes every run, e.g. #[unsafe(no_mangle)]
pub extern "C" fn Blob__getFileNameString(this: &Blob) -> BunString {
if let Some(name) = this.get_name_string() {
return name;
}
if this.is_jsdom_file.get() {
// `new File(bits, "")`: keep the explicit empty name.
return BunString::empty();
}
BunString::static_(b"blob")
}With that, the snippet above prints 2. pub extern "C" fn Blob__setAsFile(this: &mut Blob, path_str: &mut BunString) {
if !this.is_jsdom_file.replace(true) {
// Wrapping a plain `Blob` creates a new `File`: stamp `lastModified`.
this.last_modified
.set(bun_core::time::milli_timestamp() as f64);
}
this.name.set(path_str.dupe_ref());
}A real I also confirmed the Other cases verified against Node that are cheap to assert: |
|
Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-06-16, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. The linked issue (#14725) stays open. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main. |
Fixes #14725.
Fixes #22762.
Fixes #21788.
Repro
Before:
Node, Chrome, Firefox, Safari:
And
formData.get("fname").namewasundefinedinstead of"blob".Cause
XHR spec "create an entry" step 3 says that when the value is a
Blobthat is not aFile, it becomes aFilewhose name is"blob".Blob__getFileNameString(called fromFormData.append/setwhen no filename argument is given) returned an empty string for plain Blobs, so both the multipartfilenameattribute and the retrieved entry's.namewere empty.Additionally,
Blob__setAsFilestored the FormData-assigned filename on the Blob store'sstored_name, which is shared with the user's original Blob. With a non-empty default this would have leaked the name into the original Blob, and thestored_name.is_empty()guard prevented overriding the name via a laterset(name, blob, filename).Fix
Blob__getFileNameStringnow returns"blob"when the value is a plain Blob (not a File) with no existing name.Blob__setAsFilenow stores the filename on the per-Blobnamefield instead of the shared store, so it applies to the FormData entry (and the File returned fromget()) without affecting the original Blob, and can be overridden by a laterset/appendwith an explicit filename.This also makes
formData.set("k", Bun.file(path), "custom.bin")return.name === "custom.bin"fromget(), which previously returned the path.Verification