Repository navigation
Conversation
`import()`, `require()` and `new Worker()` of a `blob:` URL made from an S3File loaded an empty module. No request went to S3 and nothing threw. The `blob:` arm of the module loader took every Blob that is not a file for bytes in memory, and an S3-backed Blob has none. For a `Bun.file(fd)` URL the loader opened `blob:<uuid>` in the working directory. The arm now takes the bytes from `in_memory_view()`. An S3-backed Blob, or a file-backed one with no path, is a TypeError with the code `ERR_INVALID_ARG_TYPE`: `import()` rejects, `require()` throws and a Worker gets an `error` event. The message says how to read the Blob first. Found by a read of the code, not by a user report. The same class at other consumers is #46673.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Loader error handling src/runtime/error.rs, src/runtime/jsc_hooks.rs |
The public Error enum adds BlobNotInMemory. Blob loading reports this error when bytes are unavailable, and Bun__transpileFile maps it to ERR_INVALID_ARG_TYPE. Other loader errors retain the existing module-not-found response. |
Module loader coverage test/js/web/fetch/blob.test.ts |
Tests check that module-loading paths reject S3-backed and file-descriptor-backed Blobs without requesting their contents. They also cover imports after reading the bytes, empty in-memory Blobs, and path-backed files. |
Worker loading coverage test/js/web/workers/worker_blob.test.ts |
Tests check error events for S3-backed worker.mjs and worker.css entry points, and check that worker dynamic imports reject with ERR_INVALID_ARG_TYPE. The tests verify that these loads do not request the Blob. |
Possibly related PRs
- oven-sh/bun#46673: Adds in-memory byte checks for file- and S3-backed Blobs in
Bun.build. - oven-sh/bun#46842: Changes the blob-backed module loader in
src/runtime/jsc_hooks.rs. - oven-sh/bun#38364: Rejects synchronous FormData serialization when S3-backed Blob bytes are unavailable.
Suggested reviewers: dylan-conway, alii
Priority:
Merge Risk: 
64689
-
Unavailable Blob bytes now produce an explicit loading error. No merge-blocking risk is identified.
Pre-merge checks: 
-
Check name Status Explanation Title check The title names the main change: a module load from a blob: URL backed by an S3File or Bun.file(fd) is refused. The changeset adds BlobNotInMemory, a TypeError with ERR_INVALID_ARG_TYPE in the blob lo… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The description is complete and relevant. It explains the problem, the fix, expected behavior, limitations, and verification results. It does not use the template headings "What does this PR do?" and … Full details: Title check
-
Explanation
The title names the main change: a module load from a blob: URL backed by an S3File or Bun.file(fd) is refused. The changeset adds BlobNotInMemory, a TypeError with ERR_INVALID_ARG_TYPE in the blob loader arm, and tests for those refusals. The title is specific and matches the primary change.
Full details: Description check
-
Explanation
The description is complete and relevant. It explains the problem, the fix, expected behavior, limitations, and verification results. It does not use the template headings "What does this PR do?" and "How did you verify your code works?", but it provides the required information under equivalent sections.
-
- 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.
Comment @coderabbitai help to get the list of available commands.
Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
|
Updated 1:40 AM PT - Oct 27th, 2026
❌ @robobun, your commit 6468950 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 46721That installs a local version of the PR into your bun-46721 --bun |
|
Status Reproduced on main let gets = 0;
const srv = Bun.serve({ port: 0, hostname: "127.0.0.1", fetch() { gets++; return new Response("export default 42;\n"); } });
const s3 = new Bun.S3Client({ endpoint: srv.url.href, accessKeyId: "k", secretAccessKey: "s", bucket: "b" });
const url = URL.createObjectURL(s3.file("mod.mjs"));
console.log((await import(url)).default, gets); // undefined 0With this PR (#46721) the CI. No failure on the current head comes from this change.
The PR waits for a maintainer. The decision to make is the design: the loader refuses these Blobs and does not read them. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I also checked the new Err(BlobNotInMemory) early return in Bun__transpileFile, which fires while blob_to_deinit is Some but before the _blob_guard scopeguard is armed: the dupe is non-heap (ref_count: 0), and RefPtr<Store> and bun_core::String both implement Drop, so the store ref and name are released by the plain drop — no leak on that path. The copy of the blob: arm in src/bundler/options.rs still calls shared_view(), but its only consumer is the error-printer source lookup, where an empty source and a refusal land on the same fallback.
Extended reasoning...
The change narrows the blob: arm of the module loader in src/runtime/jsc_hooks.rs to in-memory Blobs, adds an Error::BlobNotInMemory variant, and surfaces it as an ERR_INVALID_ARG_TYPE TypeError, with tests in blob.test.ts and worker_blob.test.ts against a loopback Bun.serve stand-in; it touches no auth, crypto, or injection surface. Two confirmed findings are posted inline, so a human should look; this note only records the ownership re-audit of the new early return and the unchanged bundler sibling.
|
Merged main (8a0e49e) into this branch.
|
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline nit, I also checked the two new early return Err(BlobNotInMemory) paths in get_loader_and_virtual_source for a leaked Blob dupe — blob_to_deinit is a scopeguard in Bun__transpileFile (src/runtime/jsc_hooks.rs:4130), so the dupe is released on both. The new Error variant's only exhaustive consumer (name()) is updated, Bun__transpileFile is the sole caller of this arm, and in_memory_view() still yields an empty slice for a store-less new Blob([]), so that case keeps loading.
Extended reasoning...
The change refuses module loads from blob: URLs whose Blob is S3- or fd-backed in the runtime loader arm, adds an Error variant, and ships tests against a loopback Bun.serve S3 stand-in; it touches no auth, crypto or injection surface. One inline nit on test resource cleanup was reported; the new error paths release the duped Blob via the existing scopeguard, and the bundler copy of the arm in src/bundler/options.rs is intentionally unchanged with only the error printer as caller.
…hen a test fails Register the cleanup before the first await with a DisposableStack. Revoke the object URL of the Worker script too, and terminate the Worker of the entry point test.
There was a problem hiding this comment.
I re-reviewed the current head and found no bugs; the DisposableStack cleanup in the Worker tests resolves the earlier nit. Because this changes what a blob: module load of an S3File / Bun.file(fd) does for users (a refusal rather than a read, with the documented new throws for { type: "file" } and .css keys), a maintainer should still sign off on that design choice.
What was reviewed:
- The two new early returns in
get_loader_and_virtual_source: the duped Blob is already inblob_to_deiniton both, so the caller's scopeguard releases it on the error path. in_memory_view()is the existing helper (None for file-backed or S3 stores), so under!needs_to_read_file()the None arm is exactly the S3 case; the fd case is caught by thestore_path() == Nonearm.Error::BlobNotInMemoryis added to both the enum and the name table; the only other match on this error is the new arm inBun__transpileFile, and thebun_bundlertwin is excluded with a stated reason.- Tests assert the exact TypeError name/code/message and zero GETs at the stand-in, release the server/URLs/Worker before the assertions, and the workaround path is exercised.
Extended reasoning...
The change adds a BlobNotInMemory variant to the runtime Error enum and two early returns in the blob: arm of the module loader in src/runtime/jsc_hooks.rs, plus a cold helper that turns it into an ERR_INVALID_ARG_TYPE TypeError; the rest is test coverage in test/js/web/fetch/blob.test.ts and test/js/web/workers/worker_blob.test.ts. It touches no auth, crypto, or injection surface; the sensitive part is a hot module-load path, and the PR reports no new conditional branch for ordinary loads. The source diff is small and the error path is resource-safe, but it is a deliberate user-visible behavior change (refuse instead of read) with trade-offs the author lists, which is a product decision rather than a correctness question, so a human sign-off is still worthwhile. Only the test-cleanup commit landed since the last review, and it addresses the one open nit.
Related to #46673
Problem
import(),require()andnew Worker()of ablob:URL made from anS3Fileload an empty module: no request, no error.blob:arm ofget_loader_and_virtual_source(src/runtime/jsc_hooks.rs:3979) takes every Blob that is not a file for bytes in memory. An S3 store has none. No user reported it: Refuse an S3-backed Blob at spawn stdin, in the shell, inBun.buildfiles and in redis #46673 hands this site off.Bun.file(fd)URL the loader opensblob:<uuid>in the working directory.Fix
in_memory_view()(Nonefor a file or S3 store). An S3 store, or a file with no path, is aTypeError(ERR_INVALID_ARG_TYPE) that says how to read the Blob first.import()rejects,require()throws, aWorkergets anerrorevent. No request is sent.test/js/web/fetch/blob.test.ts(22 new, 20 fail on main),test/js/web/workers/worker_blob.test.ts(3 new, all fail).Background
URL.createObjectURL(blob)registers a copy of the Blob. The loader finds it by the URL's UUID.require()cannot wait, and debug builds panic when a Worker releases an S3 store.Downsides
import()rejects wherefetch()of the same URL reads the object.{ type: "file" }or a.csskey, ands3file.slice(0, 0).new Blob([text, s3file])(Blob: read file-backed parts in multi-part new Blob([...]) #33600), a slice ofBun.file(path), the arm's copy inbun_bundler. Ordinaryrequire(): 387 to 389 instructions, no new branch.Notes
Origin. Found by a read of the code after the class sweep in #46673, which lists this site as handed off. No user reported it. #43931 described the class and was closed as not a user report.
Repro. A loopback
Bun.servestands in for S3 and counts requests. Linux x64.blob:URL ofS3File,Bun.file("s3://..."), a slice,new File([s3file], name):import(), animportstatement,require(),import.meta.require(),createRequire(),require.extensions[".js"](),Bun.ModuleGraph#import(),node:vmwith the default loader{},""for a.txtkey, a parse error for a.jsonkey), 0 requestsTypeError(ERR_INVALID_ARG_TYPE), 0 requestsS3File:new Worker(url)erroreventS3File:ShadowRealm#importValue()(checked by hand)TypeErrorof the caller's realm that wraps the textS3Filewith{ type: "file" }, or with a.css,.html,.wasm,.shor.sqlitekey{}, a DatabaseTypeErrors3file.slice(0, 0)TypeErrorBun.file(fd)BuildMessage: ENOENT reading "blob:<uuid>". With a file of that name in the working directory, that file runsTypeErrorBun.file(path), an in-memoryBloborFile,new Blob([])S3File:fetch(url)thentext()Error text.
Cannot load a module from "blob:<uuid>": the Blob is S3-backed or file-descriptor-backed, so its bytes are not in memory. Read it first: URL.createObjectURL(new File([await blob.bytes()], blob.name ?? "", { type: blob.type })). The File keeps the name, because the loader comes from the extension of the name when there is one and from the type when there is none. A test runs that expression for.mjs,.json,.txtand.tomlkeys and for threeBun.file(fd)shapes.Why a throw and not a read.
require()cannot wait for a GET.await (await fetch(url)).text()and then releases the last reference to the store panics:ThreadLock is locked by thread A, not thread B(RefCount<S3Credentials>::deref). A Worker entry point is always loaded on a second thread, and a read there would hold the store across the GET. The refusal still dupes the Blob and drops it on the loading thread, as main does. It holds it for one synchronous call, and main holds it until the transpile ends. webcore: make object URL blob stores safe to share across workers #31487 changes those credentials to anArc.wsrewritten on native transports (consolidates the open WebSocket PRs) #45223, Add b.bytes and define thirty-five Bun.* functions with the bindgen #45590.onLoadhooks run before this arm.Why not keep the loaders that never read the text. The first shape of this change did: it decided on the loader and let
file,css,html,sqliteand others load as on main. Release builds of main and of that shape measured +14 instructions and +2 conditional branches inBun__transpileFileon eachrequire()of an ordinary file. The results it kept never came from the Blob. The review asked for a refusal on the kind of store alone. This is that shape: an ordinaryrequire()runs 389 instructions there against 387, with no new branch or call.The copy of the arm in
bun_bundler(src/bundler/options.rs:512) is not changed. Its one caller is the error printer (VirtualMachine::fetch_without_on_load_plugins), which looks up source lines for a stack frame. An empty source and an error give the same fallback there, and no module can load from one of these URLs now.Still wrong after this change.
new File(["// banner\n", s3file], "m.mjs")andnew Blob([s3file, s3file])load a module without the S3 part: the Blob constructor drops the part when the Blob is made (Blob: read file-backed parts in multi-part new Blob([...]) #33600, reported for files as Blob constructor ignores File parts if more than one part is given #25851).Bun.file(path).slice(a, b)loads the whole file. The window is not applied.S3Fileafternew Response([s3file])took its store loads an empty module: the Blob is empty by then.Bun.buildfiles and in redis #46673 and the PRs that it lists.Measurements. Release builds (
--profile=release) of main6292c6539dand of main with this PR'ssrc/diff, Linux x64.valgrind,perfandstraceare not installed here.gdbsingle-steppedBun__transpileFilefrom its first instruction to its return with calls stepped over, so the count is the function's own instructions, the inlinedget_loader_and_virtual_sourceincluded.Bun__transpileFilerequire()of a file (501 calls)importof a file (501 calls)import()of an in-memoryblob:URL (200 calls)import()of aBun.file(path)blob:URL (200 calls)Bun__transpileFilecall forrequire()and +3 forimport(gdb single-step, 1,002-module graph ofbench/module-loader/create.js, main 6292c65 vs PR). Every call of a kind gave the same count. The difference is register moves and block order: a trace of one call shows +5mov, -2xor, and forrequire()onejmpless. No conditional branch and no call is added.blob:import: +10 instructions, +1 conditional branch (the store test ofin_memory_view()), +0 allocator calls per load.Bun.file(path)blob:import: +7 instructions, +0 allocator calls, +0openat/read/closeper load. Allocator calls and syscalls were counted with gdb breakpoints onmi_malloc,mi_heap_malloc,mi_reallocand their variants and withcatch syscall, callees included: 31 allocator calls per in-memory load and 31 per file load on both builds, and 1openat, 2read, 1closeper file load on both..text: +512 bytes,.rodata+0 (llvm-size -A).Bun__transpileFile6,059 -> 6,254 bytes. The newblob_not_in_memory_erroris 122 bytes,#[cold]and not inlined. The copy inbun_bundleris unchanged, and so are the 13VmLoaderCtxhooks. The runtimeget_loader_and_virtual_sourcehas no symbol in either binary: it is inlined intoBun__transpileFile.GET.TypeError,ERR_INVALID_ARG_TYPEand the exact message. The two Worker entry rows assert the first line of theerrorevent. The expression from the message loads the right module for 7 of 7 shapes.ErrorCode.tsrows: 325 -> 325.bun_runtime::Errorvariants: +1. Host functions: +0. Existing lines changed in the two test files: thebun:testimport line of each.Tests.
test/js/web/fetch/blob.test.ts: 22 new tests. 20 fail on a release build of main6292c6539dand on the released 1.4.3. The other two pin what must not change: the expression from the error message loads the right module for 7 shapes, and an empty Blob and aBun.file(path)still load.test/js/web/workers/worker_blob.test.ts: 3 new tests, all fail on main.blob.test.ts171 pass,worker_blob.test.ts10 pass,disallow-code-generation-from-strings.test.ts34 pass and 4 skip. That file ran with a long timeout: on a debug build, 14 to 16 of its tests exceed the default 5 s, on main too.Self-review. A review of the first shape raised 32 concerns and asked for this re-cut. Done: the refusal is on the kind of store alone and asks the shared
in_memory_view(). The loader table, theforce_loaderparameter and thebun_bundlerhook change are gone. The tests can see a stray request, and cover the zero-length slice and the one-partFile. The printed expression fits aBun.file(fd). The docs paragraph is gone. Not done, 5 concerns:Bun.buildfiles and in redis #46673 (3 concerns). That PR hands this site off and shares no file with this one. This is one commit and can be picked into it.Bun.file(fd)load can run a file from the working directory, so its refusal stays.docs/runtime/code-generation-from-strings.mdx:89,Bun.file()toBun.file(path)(1). docs: say that=strictrefuses text and Markdown imports fromdata:andblob:URLs #46020 edits that page.