Conversation
…rting when an output buffer cannot be allocated The sync zlib APIs size their output buffers from the caller's input (deflateBound / libdeflate bound for compression, the gzip trailer's ISIZE or the input length for decompression, then doubling growth). All of those reservations were infallible, so an allocation the system refused aborted the process with "memory allocation of N bytes failed" instead of throwing the out-of-memory error the Zig implementation used to throw. Reserve those buffers with try_reserve(_exact) and throw a JS OOM error when a reservation fails. The zlib and libdeflate growth loops report the failure to their caller (ZlibError::OutOfMemory / AllocError). The ISIZE reservation is only a hint from untrusted data, so when it is refused the decompressor falls back to the input-sized buffer and grows from there. The zlib compressor's input-sized initial buffer is dropped since init replaces it with the deflateBound one anyway.
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Comment |
|
Updated 12:04 PM PT - Aug 15th, 2026
❌ @robobun, your commit b6c32b2 has some failures in 🧪 To try this PR locally: bunx bun-pr 39043That installs a local version of the PR into your bun-39043 --bun |
|
Status: fix pushed, ready for review.
|
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it reworks error-path cleanup across the zlib/libdeflate streaming loops and changes the ISIZE-hint fallback semantics in native code, a human look would still be worthwhile.
What was reviewed:
reserve_expand_tail(0)aftertry_reserveexposes the same spare capacity as the oldreserve_expand_tail(N)call — checkedvec_ext.rs:400, semantics are equivalent when the allocation succeeds.decompress_to_vec_grow's only caller isBunObject.rs; the two otherZlibReaderArrayList::read_allcallers (StandaloneModuleGraph.rs,create_command.rs) already treat anyErruniformly, so the newOutOfMemoryvariant is handled.- The new compressor OOM branch's
end()+state = Errorsequence matches the existingMemErrorarm;deflateEndon a null state is a defined no-op, so the subsequentDropis harmless. ZlibCompressorArrayList::initstill pointsnext_out/avail_outat the reserved buffer after the caller's pre-allocation was removed.
Extended reasoning...
Overview
The PR converts every user-input-sized output-buffer reservation in Bun.gzipSync/deflateSync/gunzipSync/inflateSync from infallible (Vec::with_capacity/reserve/reserve_exact, which abort on allocator failure) to fallible (try_reserve/try_reserve_exact), surfacing refusals as JS RangeError: Out of memory instead of a process abort. It touches src/runtime/api/BunObject.rs (both sync entry points, both library backends), src/zlib/lib.rs (the growth step in both array-list drivers plus the deflateBound reservation in init), and src/libdeflate_sys/libdeflate.rs (the doubling loop in decompress_to_vec_grow, whose signature now returns Result<_, AllocError>). The gzip ISIZE hint reservation now falls back to the input-sized buffer when refused rather than throwing, which is a deliberate semantic choice. A new ASAN-gated subprocess test in zstd.test.ts exercises all twelve reservation sites deterministically via max_allocation_size_mb.
Security risks
This is itself a robustness/DoS fix: an untrusted gzip trailer could previously pick any allocation size up to 256 MiB and abort the process if the host lacked headroom. I verified the ISIZE fallback logic still bounds the up-front reservation and that the growth loops still respect max_output_size (the decompression-bomb guard) — the remaining_budget clamp in ZlibReaderArrayList::read_all is unchanged. No new user-controlled sizes reach an infallible allocation. No auth/crypto/permissions surface.
Level of scrutiny
High. This is native code adjacent to unsafe blocks (reserve_expand_tail, set_len) and adds new error paths that must correctly release zlib/libdeflate state. Per the repo's review guidance, native memory-safety error paths are the most-blocked category. I traced each new early return: the compressor's init OOM path drops zlib_reader (whose Drop runs deflateEnd); the read_all OOM branches mirror the existing MemError arms; the libdeflate OOM path in BunObject.rs drops list and lets decompressor's RAII Drop run on return. The try_reserve + reserve_expand_tail(0) split is behaviourally identical to the prior reserve_expand_tail(N) on the success path since try_reserve guarantees cap >= len + N and reserve_expand_tail(0) exposes cap - len.
Other factors
The signature change to decompress_to_vec_grow has exactly one caller (verified by grep). ZlibCompressorArrayList is only used by BunObject.rs, and the two other ZlibReaderArrayList callers already collapse any Err from read_all, so the additional OutOfMemory return site is transparent to them (and read_all could already return that variant from zlib's own Z_MEM_ERROR). The removed pre-allocation before ZlibCompressorArrayList::init is safe because init immediately reserves deflateBound and repoints next_out/avail_out afterward. The test follows repo conventions (spreads bunEnv, drains pipes concurrently, asserts a combined object, ASAN-gated with a stated reason). Given the number of new native error paths and the small ISIZE-fallback design decision, I'm deferring rather than shadow-approving so a maintainer can confirm the fallback semantics and the state/end() ordering match intent.
|
On the two points left for a human:
|
|
#39417 now contains this change (the |
|
Closing: this change landed on main as part of #39417 (8bc4d2a). The src/zlib/lib.rs and src/libdeflate_sys/libdeflate.rs hunks from this PR are on main as-is, and the gzipSync / deflateSync / gunzipSync / inflateSync paths in src/runtime/api/BunObject.rs now throw out of memory for the initial reservation, the growth steps, and the compression bound (for libdeflate the bound reservation moved into compress_to_vec, which returns an error the caller throws). Verified by running this PR's test block unmodified against a debug (ASAN) build of main: all 12 cases pass, including the gzip trailer fallback cases. |
Problem
Bun.gzipSync,Bun.deflateSync,Bun.gunzipSyncandBun.inflateSyncabort the whole process when the output buffer they allocate cannot be obtained:memory allocation of 314572800 bytes failed, thenpanic(main thread): abort() calledand a crash report (exit 134). Before the port to Rust these threw a JS out-of-memory error (try ...initCapacity(...)in the Zig version).Vec::with_capacity/reserve_exact/reserve), and all of them are sized by the caller's data:src/runtime/api/BunObject.rsgunzip_or_inflate_sync: the gzip trailer's ISIZE field (any value below 256 MiB is honored, so 8 bytes of untrusted input pick the size), else the input length.src/runtime/api/BunObject.rsgzip_or_deflate_sync: the input length, then libdeflate's compression bound.src/zlib/lib.rsZlibCompressorArrayList::init:deflateBound(input);ZlibReaderArrayList::read_all/ZlibCompressorArrayList::read_all: the growth step taken whenever zlib fills the buffer.src/libdeflate_sys/libdeflate.rsdecompress_to_vec_grow: the doubling step onInsufficientSpace.ulimit -v 1048576, thenBun.gunzipSync/gzipSync/inflateSyncof a 300 MiB buffer (with eitherlibrary) aborts instead of throwing. ForgunzipSyncthe requested size can also come from the trailer of a gzip a few hundred bytes long (anything up to 256 MiB), so whether untrusted gzip data takes the process down depends only on the host's headroom.Fix
try_reserve/try_reserve_exact. InBunObject.rsa refusal becomesglobal.throw_out_of_memory()(the sameRangeError: Out of memoryJSC throws for its own failed allocations, and what the zstd async path and strings: don't abort on UTF-8 to UTF-16 output buffer allocation failure #33014 / TextEncoderStream: throw out of memory instead of aborting when a chunk's output buffer cannot be allocated #38941 already do for the same class of bug). The growth loops report the refusal to their caller instead:ZlibError::OutOfMemoryfrom the zlib array lists (initerrors already reachthrow_error, which maps that variant to the OOM error;read_allerrors now distinguish it from zlib data errors), andResult<_, AllocError>fromdecompress_to_vec_grow, whose only caller is this file.ZlibCompressorArrayList::initimmediately re-reserves it todeflateBound, so thatdeflateBoundreservation is now the single (fallible) allocation. The final buffer is unchanged since the result is shrunk to fit before it is handed to JSC.try_reservegrows aVecexactly likereservedoes), and a refused allocation previously aborted, so no existing result is altered. The small fixed-size allocations (the boxed zlib stream, libdeflate's own state) keep the existing crash-on-OOM policy; this change is about the buffers whose size the caller's data controls.test/js/bun/util/zstd.test.ts(new blockgzip/deflate sync APIs throw when an output buffer cannot be allocated): one child under ASAN's per-allocation cap runs twelve cases, one per refused reservation (both libraries: the compression bounds, the input-sized decompression buffer, the zlib and libdeflate growth steps, a truthful ISIZE that falls back and then fails while growing, and a claimed size after a small member that falls back and decodes), checks each throwsRangeError: Out of memory(or decodes, for the last pair) and that the process still round-trips afterwards. Without the fix the child aborts at the first case (exit 134); with it the test passes.zstd.test.ts,test/js/node/zlib/zlib.test.js(round trips both libraries),test/js/web/fetch/fetch-gzip.test.ts, the gzip regression tests undertest/regression/issue/, an ad-hoc script round-tripping 1008 combinations of sizes (0 to 3 MiB, including stored level 0 where the output exceeds the input), levels and library pairs on the debug build, andcargo clippyonbun_zlib,bun_libdeflate_sysandbun_runtime.Background
Vec::with_capacity,reserveandreserve_exactcall Rust's allocation error handler when the allocator returns null, which printsmemory allocation of N bytes failedand aborts; thetry_variants returnErrinstead and leave theVecas it was. Both families grow aVecby the same policy, so swapping them changes nothing when the allocation succeeds.deflateBound(zlib) andlibdeflate_*_compress_boundgive the most output a compressor can produce for an input of a given size, slightly more than the input itself. Both backends here compress in one shot into a buffer of that size.gunzipSyncreads the last 4 bytes of whatever it is given as a guess for the output size. Both backends here decode one member and ignore whatever follows it (node also ignores trailing bytes that do not start another member, which is what the test appends), so those 4 bytes need not describe the data that gets decoded.ZlibReaderArrayList/ZlibCompressorArrayList(src/zlib/lib.rs) drive zlib over a borrowedVec<u8>: whenever zlib reports the buffer full (avail_out == 0) they grow theVecand point zlib at the new spare capacity.reserve_expand_tail(0)is the existing helper for exposing already-reserved capacity to C. libdeflate has no streaming decode, sodecompress_to_vec_growinstead retries from scratch with a buffer twice as large each time.max_allocation_size_mbwithallocator_may_return_null=1: under the debug/ASAN build every Rust allocation goes through ASAN's malloc, which refuses anything above the cap, while theBuffers the script creates are allocated by JSC's own allocator and are unaffected. That makes the failing allocation deterministic and cheap, so the block isdescribe.skipIf(!isASAN). The 12 MiB payloads are produced withnode:zlib, which streams through chunk-sized buffers and so works under the cap.