Repository navigation
Conversation
`source_from_js` chose the Blob source from the length of the in-memory view, then from the presence of a store. A zero-byte in-memory Blob has neither, so it reached the argument throw that is for a value that is not a Blob. A zero-length slice of a byte store has a store, so it became `Source::Blob`, and the synchronous body path refused it as an fd/S3 file. The Blob arm now asks `in_memory_view()` when the view is empty. Zero bytes in memory are `Source::Owned`. Only a file- or S3-backed Blob is `Source::Blob`. `in_memory_view()` is `#[inline]`, like the store accessors it wraps, so the new call site adds no call. `pin_for_task` reported `len == 0` as a detached buffer. The borrow helper already answers 0 for a detached view or buffer, so an empty buffer now goes to the decoder and the terminal rejects with ERR_IMAGE_UNKNOWN_FORMAT. `js_thread_bytes` gave `metadata()` a detached buffer as zero bytes. It now returns `None` for one, as its doc says, so `metadata()` rejects with ERR_INVALID_STATE like the other terminals.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 4 minutes. View limit details
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 |
|
Updated 9:18 PM PT - Oct 19th, 2026
✅ @robobun, your commit 261fc5ba74e5791fc2fb674897d19edadb4d23ee passed in 🧪 To try this PR locally: bunx bun-pr 46117That installs a local version of the PR into your bun-46117 --bun |
|
Status Reproduced on a release build of main (91e7e3b) and on 1.4.2: await new Bun.Image(new Uint8Array(0)).png().bytes(); // rejects ERR_INVALID_STATE: Image: source ArrayBuffer was detached
new Blob([]).image(); // throws TypeError: Image() input must be a path string, data: URL, ArrayBuffer, TypedArray or Blob
PR: #46117 |
![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 that dropping the len == 0 arms in pin_for_task cannot leak a pin: the kind-2 Pin(v) on an empty buffer is released by PipelineJs::_pin on the job path and by _pin scope exit on every return of encode_for_body. bun_core::ffi::slice accepts a null pointer at length 0, so an empty FastTypedArray with a null vector is safe, and in_memory_view() is None only for file/fd/S3 stores, so those still take Source::Blob.
Extended reasoning...
The change touches src/runtime/image/Image.rs (Blob classification in source_from_js, detached filter in js_thread_bytes, and the pin_for_task borrow match), adds #[inline] and a doc tweak in src/runtime/webcore/Blob.rs, and rewrites the zero-byte tests in test/js/bun/image/image.test.ts. It touches unsafe ArrayBuffer pinning and borrowed-slice code but no auth, crypto, or injection surface. The inline findings concern the store-moved-out Bun.file scenario (#38503) that the replaced test now relies on, which is a maintainer call, so a human look is still warranted.
|
#38503 is closed. Its array body part is now #46732, a draft on top of #46719: The row |
Problem
new Bun.Image(await req.bytes()).webp().blob()on an empty POST rejectsImage: source ArrayBuffer was detached.new Blob([]).image()throwsTypeError: Image() input must be a path string, data: URL, ArrayBuffer, TypedArray or Blob.source_from_js(src/runtime/image/Image.rs:420) rejects a Blob with no store.pin_for_task(:739) readslen == 0as detached.Fix
in_memory_view(). Zero bytes in memory becomeSource::Owned, which already rejects withERR_IMAGE_UNKNOWN_FORMAT.pin_for_taskdrops itslen == 0test, andjs_thread_bytesreturnsNonefor a detached buffer. The borrow helper tells detached from empty.test/js/bun/image/image.test.ts, 20 tests fail on main. Also ranimage-adversarial,image-kernels,bindgen.Background
Source::Ownedcopies the bytes.Source::Blobholds the JS Blob and reads it later.in_memory_view()(WebSocket: real closing handshake, one failure path,wsrewritten on native transports (consolidates the open WebSocket PRs) #45223) returnsNoneonly for a file or S3 store.Downsides
ERR_IMAGE_UNKNOWN_FORMATwhere handlers sawERR_INVALID_STATE.new Response(new Blob([]).image())threw a codedTypeError. It now throws an uncodedError, like other body errors (Bun.Image: set error.code on sync body-encode errors #40381).source_from_js91 to 93 instructions (in-memory Blob), 66 to 70 (file Blob).metadata()on a buffer: 1 more branch.Notes
Reach. No user reported this. It came from a review comment on #45509. A search of the tracker for each message finds 0 issues. A
Bun.servehandler can reach it with an empty POST or an empty file part of a form.Repro (main):
A handler that answers 400 for
error.code === "ERR_IMAGE_UNKNOWN_FORMAT"answers 500 on main fornew Bun.Image(await req.bytes())on an empty POST, and for(await req.formData()).get("f").image()on an empty file part. It answers 400 here. This is for awaited terminals only:new Response(image)throws an error with nocodeon main and here.The contract.
packages/bun-types/bun.d.ts:9434definesERR_IMAGE_UNKNOWN_FORMATas "input bytes didn't match any sniffer" andERR_INVALID_STATEas "the input ArrayBuffer was transferred between construction and the terminal call".docs/runtime/image.mdx:16says the constructor accepts a Blob.What a store-less Blob is.
Blob::init(src/jsc/webcore_types.rs:290) gives zero bytes no store.new Blob([]),new File([], name),structuredClone(new Blob([])), an emptyFormDatafile part,await res.blob()of an empty body, a slice of a zero-size Blob and a zero-byte in-memoryBun.build()output all reach the throw.new Bun.Image(output)takes aBuildArtifactthrough the same arm, and so doesoutput.image()of #45509.Changes for input that works today.
Source::Ownedfrom construction. TheImageholds noStrongon the Blob, andmetadata()is settled at return. As a body it reports the format, not an fd/S3 file.metadata()already did.metadata()on a detached buffer rejects withERR_INVALID_STATE. It reported the format. The other terminals already said "detached".Measurements. Release builds of main 91e7e3b and of this branch, linux x64. Debug builds for the breakpoint count.
source_from_js, non-empty in-memory Blob: 91 vs 93 instructions in-symbol, 15 conditional branches and 3 calls on both (gdbnexti, 16 warm-ups then 3 calls, 2 runs). File Blob: 66 vs 70, 10 branches and 2 calls on both.Uint8Array: 64 vs 64.ImagePrototype__doMetadata, PNG in aUint8Array: 152 vs 150 instructions, 14 vs 15 conditional branches, 8 calls on both. In-memory Blob source: 131 vs 131.nm -S):source_from_js1842 to 1893 B,ImagePrototype__doMetadata1463 to 1455 B,pin_for_task494 to 494 B,encode_for_body1791 to 1791 B. The out-of-line copy ofJSC__JSValue__unpinArrayBuffer(108 B) is gone: it has one call site fewer..textis 81,665,765 B on both (size -A).heapStats().protectedObjectTypeCounts.Blob): zero-length slice 1000 to 0, empty Blob throws to 0,Bun.file1000 to 1000.Bun.peek.statusat return:metadata()on an empty Blob throws torejected, on a zero-length slicependingtorejected, on 10 shapes of empty bufferrejectedtorejected.png().bytes()on an empty bufferrejectedtopending.metadata()cells already said it. Detached-buffer cells changed: 4 of 40, themetadata()cells.new ArrayBuffer(0): 0. Same for 1000 bodies and 1000 views.read_bytes_to_handler(Blob.rs:629): 1 on main, 1 here (gdb breakpoint,image.test.ts).image.test.tson the release build of main: 20 tests fail. They are 19 of the 20 new cases and the changed S3 test. The other new case is a guard: a value that is not a Blob still throwsERR_INVALID_ARG_TYPE. The same 19 fail on 1.4.2.in_memory_view()is now#[inline]. Without the attribute the new call site was the only out-of-line call: 15 more instructions, 3 more branches and a call for every file Blob. The store accessors it wraps are#[inline]already.The in-memory arm of
read_bytes_to_handler, and #38503.BlobReadChainis the only caller, and it now starts only for a file or S3 Blob. The arm still runs for a file Blob whose store an array body moved out after construction:new Response([file])(Blob.rs:3084). The Image then reads zero bytes for a valid file, andnew Response(image)refuses it with the fd/S3 message. The S3 test now reaches the arm through that move, in place of the zero-length slice. #38503 removes the move. If #38503 lands, that test row must flip. Then no test reaches the arm, and it can go, with the store-less branch ofencode_for_body. If it does not land, the row keeps the arm tested.Not changed.
new Response(image),new Request(url, { body: image })and afetchbody throw with nocode. Bun.Image: set error.code on sync body-encode errors #40381 adds the codes.new Response(Bun.file(p).slice(a, b).image())ignores the window of the slice. Bun.Image: as a body, keep the window of a Bun.file() slice that starts past byte 0 #46081 fixes it.image.write(new Blob([]))andBun.write(new Blob([]), data)still sayCannot write to a detached Blob(validate_writable_blob). Bun.Image: reject a byte-backed Blob passed to write(dest) instead of panicking #39187 pinsImage#writeto the error ofBun.write.Bun.Imageand has its own PRs:Bun.Archiveand S3 writes (s3, Bun.Archive: use the bytes of an in-memory Blob slice, not its whole store #44779, Bun.Archive: read Bun.file() input from disk instead of writing empty bytes #41193),formData()(formData(): read a file- or S3-backed Blob body before parsing it #45161), redis (redis: reject file-backed Blob arguments instead of sending the empty string #36836), the shell (shell: fix inverted file-blob check for builtin> ${Response(...)}redirect #35318). For example,new Bun.Archive(new Blob([]))still builds a 10,240-byte archive.sharp. sharp throws
Input Buffer is emptyat construction (lovell/sharp#2687, after lovell/sharp#2681). It can: its buffer is in memory. ABun.Imagesource can be a file or an S3 object that is read at the terminal, and main already constructs from an emptyUint8Array.Suites.
bun bd test test/js/bun/image/image.test.ts: 123 pass, 5 skip (system-backend tests).image-kernels.test.ts: 37 pass.test/internal/bindgen.test.ts: 124 pass.websocket-server.test.ts -t Blob: 3 pass.image-adversarial.test.ts: 97 pass, 1 fail. The failure isdecode/encode cycles plateau, which takes longer than the 5 s default on a local debug build. It fails the same way on main.Why the new tests are in
image.test.ts.image-adversarial.test.tsstates that it does not check which error is thrown. These cases check which error an awaited terminal reports.Overlap. This branch merges clean with #45314 and #46081 in either order (
git merge-tree). It conflicts with #42185 in thepin_for_taskarms: keep the arms without thelen == 0test. Thebuffer:cases fail if the test comes back.Self-review, not done.
Image: input is empty). It changes the message for every empty source, files included. It is a separate decision.Source::Blob(Box<Blob>)), so that a later store move cannot empty the source. It changes the result for non-empty file Blobs, so it is a change of its own.Bun.ImagePRs land. The order is for a maintainer to choose. This branch merges clean with the two it overlaps most.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/image/image.test.ts