Throw instead of aborting when child output does not fit in a Buffer

[robobun]

Problem

• More than 2^32 bytes of captured child output kill the process instead of throwing. await Bun.$`head -c 4294967297 /dev/zero`.quiet() dies with panic(main thread): abort() called (SIGABRT). Bun.spawnSync({ cmd: ["head", "-c", "4294967297", "/dev/zero"] }) dies with panic: int cast: TryFromIntError(PosOverflow). So does an output of exactly 2^32 bytes, which is a valid Buffer length.
• bun:ffi toBuffer(ptr, 0, 2 ** 32 + 1) reaches the same abort with no memory at all.
• Cause 1: JSBuffer__bufferFromPointerAndLengthAndDeinit (src/jsc/bindings/JSBuffer.cpp:383 on main) and JSBuffer__fromMmap (JSBuffer.cpp:2588) hand their bytes to ArrayBuffer::createFromBytes without a length check. JSC RELEASE_ASSERTs there above MAX_ARRAY_BUFFER_SIZE (vendor/WebKit/Source/JavaScriptCore/runtime/ArrayBuffer.cpp:150).
• Cause 2: the spawnSync output arms (subprocess/Readable.rs:294, SubprocessPipeReader.rs:333) went through ArrayBuffer::from_owned_bytes / from_bytes, which convert the length through u32 with expect. That panic fires at 2^32 and above, before the bytes reach JSC.
• Cause 3: once the hand-off throws, two error paths that nothing had reached before misbehave. spawn_maybe_sync (js_bun_spawn_bindings.rs:2041) returns without finalize, which leaks the subprocess. The shell (interpreter.rs:1227, added in One termination signal, one fold: Err(Thrown) always means an exception is pending, and each event-loop dispatcher takes it in one place #37275) rejects with the JSC::Exception cell instead of the thrown value, and the JS reject callback in builtins/shell.ts expects (code, stdout, stderr), so it throws a TypeError and the promise never settles. In a debug build the child dies on an assertion in JSCell::toStringSlowCase.

Fix

• Both entry points in JSBuffer.cpp release the bytes (the deallocator, or munmap) and throw RangeError: Out of memory, the error new ArrayBuffer(2 ** 32 + 1) throws, for a length above MAX_ARRAY_BUFFER_SIZE. The JSBuffer.cpp and JSBuffer.h hunks are byte for byte the ones in Bun.file().arrayBuffer(): stop panicking at 4 GiB, throw above the ArrayBuffer limit #39558 (same blobs), which adds the check to the ArrayBuffer and typed array entry points at the same time. The two PRs merge in either order. The rest of this PR is what that check makes reachable.
• The check belongs in the entry points because they are the last step before the assert, and every producer of a Buffer from native bytes uses them: spawnSync, Bun.$, bun:ffi, bun:sqlite serialize, zlib, Bun.Archive, Bun.Image. A length of exactly 2^32 still passes. JSC accepts it, and toBuffer(ptr, 0, 2 ** 32) already works today.
• The bytes are released before the throw because the caller gave them up when it called. The length == 0 branch of the same function already does this. The repro's RSS drops to about 340 MiB after the catch.
• The two spawnSync output arms call JSValue::create_buffer_from_box. It makes the same C++ call as before with the same deallocator, without the u32 hop. The u32 conversions themselves stay as they are: Bun.file().arrayBuffer(): stop panicking at 4 GiB, throw above the ArrayBuffer limit #39558 removes them for the Bun.file() path, and this PR does not depend on that.
• spawn_maybe_sync builds stdout, stderr and the resource usage object first, runs finalize, and propagates an error after that. The pre-existing return for an exception that is already pending after the wait (a termination) runs finalize too. finalize releases the other stream's buffer and the abort signal reference, and it does not run JS, so a pending exception does not affect it.
• to_js_buffer_from_memfd and to_js_buffer_from_fd return Err when they throw. Before, they returned Ok(JSValue::ZERO) with the exception pending, and spawnSync would have stored that empty value in the result object. Nothing reaches this path from JS since spawnSync: drain piped stdio to EOF after the direct child exits #33832 (spawnSync no longer uses a memfd for stdout), so it has no test of its own.
• The shell interpreter takes the error with take_error, which unwraps the JSC::Exception, and the JS reject callback forwards the value to the promise. reject has exactly one caller, this path. Exit codes still go through resolve, which builds the ShellError.
• Verified with test/js/bun/ffi/ffi.test.js ("toBuffer at the Buffer length limit"): on main the child aborts, with the fix it throws a RangeError and a view of exactly 2^32 bytes is still created. This case costs no memory and runs on every platform. It only checks the error class, so it holds with ffi: toArrayBuffer/toBuffer throw RangeError instead of aborting on a huge byteLength #33353 too.
• Verified with test/js/bun/spawn/spawnSync.test.ts ("spawnSync output at the Buffer length limit"): 2^32 + 1 bytes throw the RangeError, 2^32 bytes come back as a Buffer. On main both cases die with the int cast panic. About 11 s per case in a debug build, 8.3 GiB peak RSS in the child, so the block skips below 16 GiB of RAM and is POSIX only (head -c). The glibc CI test machines have 8 GB and skip it, the ASAN machines have 64 GB and run it.
• Verified with test/js/bun/shell/shelloutput.test.ts ("stdout at the Buffer length limit"): the promise rejects with the RangeError. On main the child aborts. With the entry point check alone the child dies on the toStringSlowCase assertion. Same size gate.
• bun bd test passes on spawnSync.test.ts, spawn.test.ts, spawn-maxbuf.test.ts, spawnsync-isolated-event-loop.test.ts, spawnsync-no-microtask-drain.test.ts, ffi.test.js, bunshell.test.ts, shelloutput.test.ts, throw.test.ts and test/internal/source-lints. The repros are clean under BUN_JSC_validateExceptionChecks=1. cargo check for x86_64-pc-windows-msvc and clippy are clean.
• Related open PRs: Bun.file().arrayBuffer(): stop panicking at 4 GiB, throw above the ArrayBuffer limit #39558 (see above), ffi: toArrayBuffer/toBuffer throw RangeError instead of aborting on a huge byteLength #33353 (a check in bun:ffi in front of the entry point), Reject instead of aborting when arrayBuffer() or bytes() reads a body above 4 GiB #37243 (a check in Buffer.from(string) in front of it). They apply on top of this change.

Background

A Buffer is a Uint8Array over a JSC ArrayBuffer. JSC stores the byte length as size_t but caps it at MAX_ARRAY_BUFFER_SIZE (2^32 on 64-bit, PageCount.h). Bun exposes the cap as require("buffer").kMaxLength. JSC's allocating constructors return null above the cap, and the callers turn that into RangeError: Out of memory. The adopting constructor, createFromBytes, takes bytes that already exist and asserts instead, so the check has to happen before the hand-off.

Adopting means JSC takes ownership of a byte range that native code allocated, and frees it through a deallocator passed along with the bytes when the object is collected. spawnSync and the shell use this to return their captured output without a copy. bun:ffi toBuffer uses it with a deallocator that frees nothing, which is why it can describe any length without owning that much memory.

In Rust, JsResult is Result<JSValue, JsError>, and Err(Thrown) means an exception is pending on the VM. from_js_host_call turns a C++ call that returns an empty value with an exception pending into that Err. take_exception removes the pending exception and returns JSC's Exception cell, the wrapper that carries the thrown value and its stack. take_error returns the thrown value itself, which is what a JS callback has to receive. The promise helpers unwrap the cell themselves, which is why the other take_exception callers are fine.

Earlier version of this PR

The first push threw RangeError with code ERR_BUFFER_TOO_LARGE from the two entry points, with its own helper. #39558 added its check to the same two functions in the meantime, with RangeError: Out of memory for all four entry points. This PR now carries that PR's hunks unchanged instead, and the tests expect that error.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d2173b6e-e8ad-4bab-8170-5fffb73020f8

📥 Commits

Reviewing files that changed from the base of the PR and between a8bae27 and cff9d8c.

📒 Files selected for processing (4)

• src/js/builtins/shell.ts
• src/jsc/array_buffer.rs
• src/runtime/api/bun/js_bun_spawn_bindings.rs
• src/runtime/shell/interpreter.rs

💤 Files with no reviewable changes (1)

• src/runtime/shell/interpreter.rs

Included review availability: Your plan includes up to 5 reviews per rolling hour; 0 remain after this review.

---

Walkthrough

The PR enforces the maximum JavaScript buffer size, propagates buffer and subprocess conversion errors, preserves subprocess cleanup, and forwards shell interpreter errors without wrapping them in ShellError.

Changes

Buffer and error propagation

Layer / File(s)	Summary
Buffer limit and native cleanup src/jsc/bindings/JSBuffer.h, src/jsc/bindings/JSBuffer.cpp, test/js/bun/ffi/ffi.test.js	No-copy and mmap-backed buffers reject oversized allocations, release adopted memory, and preserve mapping length for cleanup. FFI tests cover the 2^32 boundary.
Fallible subprocess buffer conversion src/jsc/array_buffer.rs, src/runtime/api/bun/subprocess/Readable.rs, src/runtime/api/bun/subprocess/SubprocessPipeReader.rs, src/runtime/api/bun/js_bun_spawn_bindings.rs, test/js/bun/spawn/spawnSync.test.ts	Buffer conversion returns JavaScript errors instead of zero-value sentinels. Subprocess readers create buffers from owned byte slices. spawnSync finalizes subprocesses before propagating conversion failures.
Shell interpreter error forwarding src/js/builtins/shell.ts, src/runtime/shell/interpreter.rs, test/js/bun/shell/shelloutput.test.ts	Shell rejection callbacks accept unknown errors. Interpreter failures pass the thrown value directly to promise rejection. The shell test covers oversized output and RangeError handling.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: child output exceeding the Buffer limit now throws instead of aborting.
Description check	✅ Passed	The description explains the problem, implementation, scope, and verification results, although it uses different headings from the repository template.

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status

Ready for review. CI is green on cff9d8c.

Reproduced on the current release build (1.4.0-canary (8326d1bd3)), Linux x64:

• await Bun.$`head -c 4294967297 /dev/zero`.quiet(): panic(main thread): abort() called, SIGABRT.
• Bun.spawnSync({ cmd: ["head", "-c", "4294967297", "/dev/zero"] }): panic: int cast: TryFromIntError(PosOverflow). The same with 4294967296.
• toBuffer(ptr, 0, 2 ** 32 + 1) from bun:ffi: SIGABRT.

With this branch all three throw or reject with RangeError: Out of memory, the bytes are released, and an output of exactly 2^32 bytes comes back as a Buffer.

The JSBuffer.cpp and JSBuffer.h hunks are identical to the ones in #39558, so the two PRs merge in either order. Everything else here (spawnSync arms and error paths, memfd error propagation, shell rejection path, tests) is not in #39558.

Tests: test/js/bun/ffi/ffi.test.js (no memory needed, runs everywhere), test/js/bun/spawn/spawnSync.test.ts and test/js/bun/shell/shelloutput.test.ts (4 GiB cases, POSIX, skipped below 16 GiB of RAM). All three fail on main and pass here.

[claude]

I reviewed this PR and didn't find any bugs. Because it touches memory-safety-critical native paths across several subsystems — the DECLARE_TOP_EXCEPTION_SCOPE → DECLARE_THROW_SCOPE swap and deallocator-before-throw in JSBuffer.cpp, the finalize-before-propagate reordering in spawn_maybe_sync, and the shell reject callback contract change between interpreter.rs and builtins/shell.ts — a human look would still be worthwhile.

What was reviewed:

• JSBuffer__bufferFromPointerAndLengthAndDeinit / JSBuffer__fromMmap: the length guard runs before createFromBytes, releases the caller's bytes (deallocator / munmap) exactly once, and the throw scope change is required for Bun::throwError; the existing RETURN_IF_EXCEPTION tail still applies.
• create_buffer_from_box passes usize straight through with the same MarkedArrayBuffer_deallocator, so the u32 hop is gone and the too-large path frees the box via the C++ deallocator branch.
• spawn_maybe_sync: output is fully computed before finalize, and the destructured JSValues are GC-owned so they survive the subprocess teardown; the error path now runs finalize where it previously leaked.
• Shell: reject_get_cached has a single call site and take_error unwraps the JSC::Exception cell so the JS callback receives the thrown value; exit-code rejection still routes through resolve → ShellError.

Extended reasoning...

Overview

The PR replaces process-killing aborts/panics with a catchable ERR_BUFFER_TOO_LARGE RangeError when native bytes handed to the Buffer adoption path exceed kMaxLength (2^32). It touches:

• src/jsc/bindings/JSBuffer.cpp: adds a length guard + throwBufferTooLarge helper to both adopting entry points, swapping a top scope for a throw scope in one of them, and factoring the mmap unmap into a lambda so it can run on the reject path.
• src/jsc/array_buffer.rs: to_js_buffer_from_fd / to_js_buffer_from_memfd now return JsResult and propagate Err(Thrown) instead of Ok(JSValue::ZERO); the mmap arm is wrapped in from_js_host_call.
• src/runtime/api/bun/subprocess/{Readable,SubprocessPipeReader}.rs: switch to JSValue::create_buffer_from_box, which routes through the same C++ entry point without the u32::try_from(...).expect panic.
• src/runtime/api/bun/js_bun_spawn_bindings.rs: builds stdout/stderr/resource-usage first, runs SubprocessT::finalize unconditionally, then propagates any error — fixing a leak of the subprocess and the sibling stream's buffer on the throw path.
• src/runtime/shell/interpreter.rs + src/js/builtins/shell.ts: the interpreter now passes the unwrapped thrown value to reject, and the JS reject callback forwards it directly to the promise instead of trying to build a ShellError from three args it never receives.
• Three test files add subprocess-isolated coverage for the ffi, spawnSync, and shell paths at and just above the limit.

Security risks

None identified. The change turns a hard abort into a recoverable RangeError; the bytes are still released on the reject path (deallocator or munmap), so there is no new leak or use-after-free surface. No auth, crypto, or trust-boundary parsing is touched.

Level of scrutiny

High. This is native, memory-owning code at the JSC boundary: an exception-scope kind change, deallocator-before-throw ordering, a lambda whose capture must survive into a createSharedTask, a finalize/propagate reordering around a raw Box::from_raw, and a Rust↔JS-builtin callback contract change. Each is small but sits squarely in the categories the repo's review guide flags as most-blocked (paired acquisition/release, exception checks, error paths that must complete the operation). The PR description is unusually thorough and I found the changes internally consistent, but this is exactly the kind of diff a maintainer should sign off on.

Other factors

• Verified create_buffer_from_box (JSValue.rs:608) hands usize length + MarkedArrayBuffer_deallocator straight to JSBuffer__bufferFromPointerAndLengthAndDeinit, so the too-large branch there frees the box and the at-limit case (2^32) still passes.
• Verified reject_get_cached has exactly one call site, so the JS reject arity change has no other producer to update.
• WTF::move on the unmap lambda matches existing usage in the bindings (BunProcess.h, BunString.cpp).
• Tests cover the boundary (exactly 2^32 succeeds, 2^32 + 1 throws) and are gated on POSIX + ≥16 GiB RAM for the heavy cases; the ffi test exercises the same C++ path with no memory cost on all platforms. The heavy tests carry 120 s per-test timeouts, which is long but justified given ~11 s debug-build runtime per case with 8 GiB peak RSS.

[robobun]

The automated review above reports no findings, so nothing changes because of it.

One update is coming. #39558 now adds the same length check to JSBuffer__bufferFromPointerAndLengthAndDeinit and JSBuffer__fromMmap (it throws RangeError: Out of memory, the error the ArrayBuffer entry points in that PR throw). I will make the JSBuffer.cpp and JSBuffer.h hunks here identical to that PR, so the two merge cleanly in either order, and update the tests to that error. The rest of this PR stays: the spawnSync output arms, finalize on the spawnSync error path, the memfd error propagation, the shell rejection path, and the spawnSync, shell and ffi tests. #39558 does not cover those, and its check makes the spawnSync and shell paths reachable.

[robobun]

^{Updated 10:29 AM PT - Aug 18th, 2026}

✅ @robobun, your commit cff9d8c68bf9559e812ecae5c0e97d37a3a5014c passed in Build #100638! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 39564

That installs a local version of the PR into your bun-39564 executable, so you can run:

bun-39564 --bun

[claude]

I re-reviewed after cff9d8c and found no new issues — the pre-existing has_exception() early return now runs finalize too, so the sibling leak is closed. Given this touches native ownership hand-off (deallocator on the throw path in JSBuffer.cpp, the TOP_EXCEPTION_SCOPE → THROW_SCOPE change), subprocess finalize/error-propagation ordering around Box::from_raw, and the shell's reject callback contract, a human look is still worthwhile.

What was reviewed:

• rejectBytesNoCopyAboveArrayBufferLimit releases via the caller's deallocator before throwing; JSBuffer__fromMmap routes through unmapBufferBytes with the length packed into the context so the reject path unmaps too.
• create_buffer_from_box calls JSBuffer__bufferFromPointerAndLengthAndDeinit with a usize length and MarkedArrayBuffer_deallocator, so the spawnSync/pipe paths no longer trip the u32 cast and the throw path frees the box.
• spawn_maybe_sync now builds (stdout, stderr, resource_usage) into a single Result, runs finalize, then ?s — no borrow of subprocess survives past Box::from_raw; both early returns after the wait loop finalize.
• interpreter.rs uses take_error (unwraps the JSC::Exception cell) and shell.ts reject forwards the value straight to rej; the sole caller passes exactly one arg.

Extended reasoning...

Overview

Eleven files across the C++ JSC bindings, Rust JSC glue, subprocess/shell error paths, and three test files. The core change adds a MAX_ARRAY_BUFFER_SIZE guard to the two adopting Buffer entry points in JSBuffer.cpp (mirrored from #39558), then plumbs the newly-reachable throw through every producer: spawnSync's output arms move to create_buffer_from_box (no u32 hop), spawn_maybe_sync defers the ? until after finalize, to_js_buffer_from_fd/from_memfd return JsResult instead of JSValue::ZERO with a pending exception, and the shell's interpreter/JS reject pair carries the thrown value to the promise instead of the Exception cell.

Security risks

None identified. The added guard rejects oversized adopted buffers (turning a process abort into a catchable RangeError) and releases the bytes on the reject path, so it strictly reduces the DoS surface. No auth/crypto/permissions code touched.

Level of scrutiny

High. This is native memory ownership across the Rust/C++ boundary: the new reject path calls the caller's deallocator on bytes the caller already gave up, JSBuffer__bufferFromPointerAndLengthAndDeinit changes from a top-exception scope to a throw scope, and spawn_maybe_sync reorders Box::from_raw-based finalization against fallible calls. The shell change alters the reject callback's contract (from (code, stdout, stderr) — which was never actually reachable — to (error)). None of this is mechanical.

Other factors

My earlier inline comment (the pre-existing has_exception() return leaking the boxed subprocess) was addressed in cff9d8c; the author's reply correctly scopes the fix to the post-wait state and explains why earlier returns need different teardown. The PR carries tests for all three user-visible entry points (ffi runs everywhere; spawnSync/shell gate on 16 GiB RAM + POSIX). The JSBuffer.cpp/.h hunks are byte-identical to #39558 by design. All comment-cop threads are resolved. Deferring rather than approving because the memory-safety reasoning here (deallocator on throw, finalize ordering, exception-scope change) warrants a maintainer's eyes.

[robobun]

Scope note after the merge (from the self-review of this PR), for whoever picks up the rest of this class:

• Not covered here or in Bun.file().arrayBuffer(): stop panicking at 4 GiB, throw above the ArrayBuffer limit #39558: JSUint8Array__fromDefaultAllocator and JSArrayBuffer__fromDefaultAllocator in src/jsc/bindings/Uint8Array.cpp still call createFromBytes without a length check. They back the stream and blob consumers, so more than 2^32 bytes through await proc.stdout.bytes() on an async Bun.spawn, or through a body .bytes() / .arrayBuffer(), still abort. Bun::rejectBytesNoCopyAboveArrayBufferLimit (JSBuffer.h, on main now) fits both, with ArrayBuffer::from_default_allocator returning JsResult. The napi external buffer entry points are a separate case (napi_create_external_buffer and friends).
• spawn: hold the Subprocess as a RefPtr across spawn_maybe_sync #37700 changes the same tail of spawn_maybe_sync that this PR touched (the two finalize calls) and needs a rebase over it.