Skip to content

Stop aborting when stream arrayBuffer/bytes consumers mix chunk types past 2 GiB - #37239

Open
robobun wants to merge 6 commits into
mainfrom
farm/5b3ce9f6/stream-mixed-2gb-abort
Open

Stop aborting when stream arrayBuffer/bytes consumers mix chunk types past 2 GiB#37239
robobun wants to merge 6 commits into
mainfrom
farm/5b3ce9f6/stream-mixed-2gb-abort

Conversation

@robobun

@robobun robobun commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

Fixes an uncatchable process abort (panic(main thread): abort() called, exit 134) in Bun.readableStreamToArrayBuffer and Bun.readableStreamToBytes when the stream's chunks mix strings with binary data and their total size reaches 2 GiB:

const n = 1100000000;
const rs = new ReadableStream({
  start(c) {
    c.enqueue(new Uint8Array(n));
    c.enqueue(new Uint8Array(n));
    c.enqueue("x"); // any string chunk forces the mixed path
    c.close();
  },
});
await Bun.readableStreamToArrayBuffer(rs); // aborted the whole process

Cause

concatenateChunks (src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp) staged the mixed string+binary concatenation in a WTF::Vector<uint8_t>. It guarded the size sum with CheckedSize (which only catches 64-bit wraparound), then called bytes.reserveInitialCapacity(total). WTF::Vector caps capacities at INT32_MAX and CRASH()es rather than failing (isValidCapacityForVector in wtf/Vector.h), so any total in [2^31, 2^64) aborted before a catchable out-of-memory path could run.

All-binary chunk arrays were unaffected: they take the flattenArrayOfBuffersIntoArrayBufferOrUint8Array arm, which already assembles into a JSC::ArrayBuffer and handles >2 GiB totals fine. Only adding a string chunk to the mix hit the Vector.

Fix

Assemble the mixed arm directly into the result JSC::ArrayBuffer (tryCreateUninitialized), mirroring the all-binary arm:

  • capacities are size_t, so results up to JSC's 4 GiB MAX_ARRAY_BUFFER_SIZE now work, matching what the all-binary arm already produces for the same data
  • totals past 4 GiB make tryCreateUninitialized return null, which throws a catchable out-of-memory RangeError instead of aborting
  • the final Vector to ArrayBuffer copy is gone (the bytes are written in place once)

The sizing pass now records each binary chunk's measured byte length, and the write pass clamps every copy to that measurement, so a chunk that changed size in between (e.g. a growable SharedArrayBuffer grown from a worker) can neither overrun the buffer nor leave uninitialized bytes exposed; if a chunk shrank or detached, the result is re-created at the actual written size, matching the previous Vector behavior.

The text consumers have the same crash shape in different functions; that is fixed separately in #37237. The Blob-side equivalents were fixed in #37215.

How did you verify your code works?

  • Reproduced the abort on 1.4.0-canary.1 (9d519e8ca) for both consumers; both now return a 2200000001-byte result, and the all-binary arm is unchanged
  • Added 4 subprocess tests to test/js/bun/util/readablestreamtoarraybuffer.test.ts: both consumers with mixed chunks totaling 2 GiB + 3 bytes (asserting byte markers across both chunk boundaries and the UTF-8 string bytes at offset 2^30), and both consumers with mixed totals past 4 GiB (asserting a catchable RangeError instead of an abort). All 4 fail on the released build (subprocess aborts) and pass with this change
  • Verified small-input behavior is unchanged against the released build: chunk-type mix (view/ArrayBuffer/string), offset subarray views, multi-byte and lone-surrogate strings, empty chunks, detached buffers, and the TypeError for invalid chunk types
  • test/js/web/streams/streams.test.js, test/js/bun/stream/direct-readable-stream.test.tsx, test/js/web/fetch/stream-fast-path.test.ts, test/js/web/fetch/body-clone.test.ts, test/js/web/streams/compression.test.ts all pass

[review] gate passed · iteration 0 · 2 files touched

fails on main (without fix)
ASAN without fix: 6 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/util/readablestreamtoarraybuffer.test.ts
bun test v1.4.0 (8326d1bd3)

test/js/bun/util/readablestreamtoarraybuffer.test.ts:
(pass) readableStreamToArrayBuffer does not call a patched Promise.prototype.then [19.94ms]
86 |     });
87 |     const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);
88 |     if (stdout.trim() === "SKIP") return;
89 |     // String comparison instead of JSON.parse: a crashed child diffs against "" here
90 |     // rather than failing with an unrelated parse error.
91 |     expect(stdout.trim()).toBe(
                               ^
error: expect(received).toBe(expected)

Expected: "{"constructor":"ArrayBuffer","byteLength":2147483653,"markers":[170,171,120,121,122,186,187,33,63]}"
Received: ""

      at <anonymous> (/workspace/bun/test/js/bun/util/readablestreamtoarraybuffer.test.ts:91:27)
(fail) readableStreamToArrayBuffer handles mixed string+binary chunks totaling over 2 GiB [419.26ms]
135 |         stdout: "pipe",
136 |         stderr: "inherit",
137 |       });
138 | 
... (truncated)

release without fix: 6 FAILED
bun test v1.4.0-canary.1 (8326d1bd3)

test/js/bun/util/readablestreamtoarraybuffer.test.ts:
(pass) readableStreamToArrayBuffer does not call a patched Promise.prototype.then [0.27ms]
============================================================
Bun Canary v1.4.0-canary.1 (8326d1bd3) Linux x64
Linux Kernel v6.17.0 | glibc v2.41
CPU: sse42 popcnt avx avx2 avx512
Args: "/workspace/bun/build/release/bun" "-e" "\n      const GIB = 1073741824;\n      let a, b;\n      try {\n        a = new Uint8Array(GIB);\n        b = new ArrayBuffer(GIB);\n      } catch {\n        console.log("...
Features: bunfig jsc tsconfig 
Builtins: "bun:main" 

Elapsed: 5ms | User: 2ms | Sys: 3ms
RSS: 32.37 MB | Peak: 49.61 MB | Commit: 2.19 GB | Faults: 0 | Machine: 34.36 GB

panic(main thread): abort() called
oh no: Bun has crashed. This indicates a bug in Bun, not your code.

To send a redacted crash report to Bun's team,
please file a GitHub issue using the link below:

 https://bun.report/1.4.0/la28326d1bgCgkggC21klBq4635B_gpmpuDmsnhuDkvn4/Dotk1oEu3x86B8rlzoE2il1oEkst9lE2r00kEso10kE8/r9lE06w0kEs+8voEuj13tEottlwE8v5+6B0ji14CAa

curl: (22) The requested URL returned error: 403
86 |     });
87 | 
... (truncated)
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/util/readablestreamtoarraybuffer.test.ts
bun test v1.4.0 (8326d1bd3)

test/js/bun/util/readablestreamtoarraybuffer.test.ts:
(pass) readableStreamToArrayBuffer does not call a patched Promise.prototype.then [18.16ms]
(pass) readableStreamToArrayBuffer handles mixed string+binary chunks totaling over 2 GiB [1892.27ms]
(pass) readableStreamToArrayBuffer throws instead of aborting when mixed chunks exceed the 4 GiB ArrayBuffer maximum [302.93ms]
(pass) readableStreamToBytes handles mixed string+binary chunks totaling over 2 GiB [1873.98ms]
(pass) readableStreamToBytes throws instead of aborting when mixed chunks exceed the 4 GiB ArrayBuffer maximum [294.42ms]
(pass) readableStreamToBytes handles a string chunk larger than 2 GiB [17616.50ms]
(pass) chunks mutated by Array.prototype accessor reentry during concatenation are clamped [321.76ms]
(pass) an async start() promise is adopted observably, like Node [11.75ms]

 8 pass
 0 fail
 22 expect() calls
Ran 8 tests across 1 file. [24.68s]
__F:0:S:0

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     c881f1ae3b
  features     baseline

22 deps, 120 codegen, 1175 objects in 641ms

ninja: Entering directory `/workspace/bun/build/release'
[1/1236] gen ErrorCode+*.h
[2/1236] install /workspace/bun
bun install v1.4.0-canary.1 (8326d1bd3)

Checked 107 installs across 153 packages (no changes) [11.00ms]
[3/1236] gen bindgenv2
[4/1236] install /workspace/bun/packages/bun-error
bun install v1.4.0-canary.1 (8326d1bd3)

Checked 1 install across 2 packages (no changes) [1.00ms]
[5/1236] fetch zlib
[zlib] up to date
[6/1236] fetch tinycc
[tinycc] up to date
[7/1235] fetch libjpeg-turbo
[libjpeg-turbo] up to date
[8/1235] install /workspace/bun/src/node-fallbacks
bun install v1.4.0-canary.1 (8326d1bd3)

Checked 129 installs across 147 packages (no changes) [12.00ms]
[9/1235] gen ProcessBindingConstants.lut.h
Generating /workspace/bun/build/release/codegen/ProcessBindingConstants.lut.h from /workspace/bun/src/jsc/bindings/ProcessBindingConstants.cpp
[10/1235] gen .bind.ts → GeneratedBindings.cpp
... (truncated)
diff hotspot
.../webcore/streams/BunStreamConsumers.cpp         |  79 ++++---
 .../bun/util/readablestreamtoarraybuffer.test.ts   | 245 +++++++++++++++++++++
 2 files changed, 291 insertions(+), 33 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file
file                                                     reads  edits  tests
src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp      0      0      0
test/js/bun/util/readablestreamtoarraybuffer.test.ts         0      0      0

@github-actions github-actions Bot added the claude label Aug 9, 2026
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

concatenateChunks measures all chunk types and writes mixed stream data directly into a shared ArrayBuffer. Tests cover inputs above 2 GiB, catchable failures above the 4 GiB limit, and chunk changes during concatenation.

Changes

Stream concatenation

Layer / File(s) Summary
Chunk measurement and buffer assembly
src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp
The sizing pass records string and binary chunk lengths. The write pass uses one ArrayBuffer, clamps binary copies, handles detached or shrinking chunks, and returns shared-buffer results.
Large-input conversion tests
test/js/bun/util/readablestreamtoarraybuffer.test.ts
Subprocess tests verify mixed inputs above 2 GiB, large UTF-8 strings, output markers and types, clean exit, catchable RangeError results above 4 GiB, and accessor reentry behavior.

Possibly related PRs

  • oven-sh/bun#36997: Addresses large buffer sizes beyond 2³² in a different implementation.
  • oven-sh/bun#37237: Modifies BunStreamConsumers.cpp stream chunk accumulation and catchable out-of-memory handling.
🚥 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 and concisely identifies the fix for aborts in stream arrayBuffer/bytes consumers with mixed chunks over 2 GiB.
Description check ✅ Passed The description includes both required sections and provides detailed cause, fix, scope, and verification results.

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@test/js/bun/util/readablestreamtoarraybuffer.test.ts`:
- Around line 80-88: In both stream conversion tests, move the proc.signalCode
and exitCode assertions before parsing or matching stdout. Ensure abnormal child
termination is reported first, then parse stdout and validate the expected
result only after confirming normal exit.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 97685aea-89da-46e3-a1fd-616bf27b5741

📥 Commits

Reviewing files that changed from the base of the PR and between 9008ae7 and 4b9d86e.

📒 Files selected for processing (2)
  • src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp
  • test/js/bun/util/readablestreamtoarraybuffer.test.ts

Comment thread test/js/bun/util/readablestreamtoarraybuffer.test.ts
Comment thread src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp Outdated
Comment thread src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp Outdated
Comment thread src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp Outdated
Comment thread src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp Outdated
Comment thread src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp Outdated
Comment thread src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp Outdated
Comment thread src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp Outdated
Comment thread src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp
Comment thread src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp
Comment thread src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. Because the fix replaces the staging Vector with tryCreateUninitialized + manual offset/memcpy arithmetic in a JSC binding — the memory-safety category REVIEW.md flags as most-blocked — a human look would still be worthwhile.

What was reviewed:

  • Traced that offset can never exceed total (each step advances by at most that chunk's measured length), so subspan(offset, measuredByteLength) and the memcpy never overrun.
  • Confirmed the offset < bytes.size() re-create path is the only way uninitialized tail bytes could otherwise escape (short string write, shrunk/detached chunk, clamped grown SAB all fall through to it), and that writes stay contiguous so there are no mid-buffer gaps.
  • Checked no user JS runs between the sizing and write passes (values.at, dynamicDowncast, span() are all non-throwing; tryCreateUninitialized allocates a refcounted C++ object, not a JS cell), so the only cross-pass size hazard is a concurrently grown SAB — which the std::min clamp handles.
  • Verified MarkedArgumentBuffer roots the chunks and RefPtr<ArrayBuffer> owns the result across the loop.
Extended reasoning...

Overview

The PR rewrites the mixed string+binary arm of concatenateChunks in src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp to assemble directly into a JSC::ArrayBuffer (via tryCreateUninitialized) instead of staging in a WTF::Vector<uint8_t>, whose INT32_MAX capacity cap CRASH()es on totals ≥ 2 GiB. The sizing pass now records each binary chunk's measured length so the write pass can clamp copies, and a final offset < total check re-creates the buffer at the actual written size to avoid exposing uninitialized bytes. Four subprocess tests are added covering both consumers at 2 GiB + 3 bytes (byte markers verified across boundaries) and > 4 GiB (catchable RangeError instead of abort).

Security risks

None in the traditional sense — this is a DoS-class fix (uncatchable abort → catchable error). The main risk area is memory safety: writing into an uninitialized ArrayBuffer with manual offset arithmetic. I traced every advance of offset and confirmed it is bounded by the sum of measured lengths, so neither the subspan nor the memcpy can overrun. The uninitialized-tail re-create covers every path where a chunk contributes fewer bytes than measured, and because offset only ever advances by what was actually written, writes stay contiguous with no mid-buffer uninitialized gaps.

Level of scrutiny

High. This is C++ in the JSC bindings layer, touches tryCreateUninitialized + raw memcpy, and lands squarely in REVIEW.md's "Native code: memory safety (the most-blocked category)". The change is ~50 lines and mirrors what the all-binary flattenArrayOfBuffersIntoArrayBufferOrUint8Array arm already does, but the manual bounds arithmetic and the two-pass measure/write shape with a shared-memory clamp are exactly the kind of thing a maintainer should eyeball once.

Other factors

  • No bugs from the bug-hunting system.
  • All bot feedback (CodeRabbit assertion ordering, comment-cop length nags) is resolved; the remaining two-line comments document the WTF::Vector cap and the clamp/re-create invariants, which are load-bearing.
  • Tests follow harness conventions (subprocess isolation for the abort failure mode, bunEnv/bunExe, stdout-as-string diff so a crashed child fails clearly, SKIP guard for allocation failure).
  • The one intentional behavior change vs. the old Vector path — a growable SAB grown between passes is now clamped to its measured size instead of appending the grown bytes — is called out in the PR description and is the safe choice.
  • I checked that no user JS can run between the sizing and write passes, so single-thread detach/resize cannot happen mid-assembly; only concurrent SAB growth can, and it is clamped.

Comment thread src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp
Comment thread src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. Because it rewrites the mixed-chunk concatenation path with an uninitialized ArrayBuffer, manual offset/memcpy arithmetic, and reentrant-mutation clamping — squarely in the memory-safety category — a human look would still be worthwhile.

What was reviewed

  • Traced that offset can never exceed total: string writes advance by their measured (immutable) size, binary writes advance by min(span, measured), so subspan(offset, measuredByteLength) and every memcpy stay in bounds.
  • Confirmed the write pass calls nothing that can GC or run JS (values.at, span(), writeUTF8, memcpy), so resultBuffer->data() and the rooted chunks stay valid throughout.
  • Checked the shrink/detach path re-creates from bytes.first(offset) so no uninitialized tail is exposed; the grow path is clamped so it cannot overrun.
  • Noted the old defensive written < byteLength shrink for strings is dropped in favor of the sizer/writer contract — the same contract encodeStringToUint8Array in this file already relies on.
Extended reasoning...

Overview

The PR rewrites the mixed string+binary arm of concatenateChunks in src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp (~80 lines net) to assemble directly into a JSC::ArrayBuffer via tryCreateUninitialized instead of staging in a WTF::Vector<uint8_t>, whose INT32_MAX capacity cap CRASH()es past 2 GiB. The sizing pass now records each chunk's measured length; the write pass clamps binary copies to that measurement and re-creates the result if any chunk shrank or detached between passes. It adds ~245 lines of subprocess tests to test/js/bun/util/readablestreamtoarraybuffer.test.ts covering both consumers at >2 GiB, the >4 GiB catchable-throw path, a >2 GiB single-string chunk, and Array.prototype accessor reentry that grows/shrinks/detaches an already-measured chunk.

Security risks

The change replaces a growable Vector with an uninitialized fixed-size buffer plus manual offset/memcpy bookkeeping, and explicitly reasons about hostile reentry (getter-driven resize/detach of chunks between measure and write). An off-by-one or mis-ordering here would be a heap overrun or an uninitialized-memory disclosure. My trace found the invariants hold — offset is bounded by the sum of measured sizes, binary copies are clamped, string slots use immutable materialized WTF::Strings, and any short write triggers a right-sized re-create — but this is exactly the class of code REVIEW.md flags as most-blocked.

Level of scrutiny

High. This is native C++ touching uninitialized allocation, size arithmetic on user-controlled totals, and cross-pass consistency under adversarial JS reentry. The fix also drops a release-build defensive shrink (if (written < byteLength) bytes.shrink(...)) in favor of trusting the sizer/writer contract and advancing by the measured size — consistent with encodeStringToUint8Array in the same file, but a behavioral tightening worth a maintainer's eye.

Other factors

The bug hunting system found nothing. Tests are thorough and demonstrably fail on the released build (subprocess aborts) and pass with the fix, including the adversarial reentry case that exercises the clamp and re-create paths. All prior bot feedback (CodeRabbit ordering nit, comment-cop length complaints) is resolved. The all-binary fast path (flattenArrayOfBuffersIntoArrayBufferOrUint8Array) is untouched. Given the memory-safety surface, deferring to a human reviewer rather than auto-approving.

robobun and others added 6 commits August 17, 2026 02:53
Bun.readableStreamToArrayBuffer and Bun.readableStreamToBytes staged
mixed string+binary chunk concatenation through a WTF::Vector<uint8_t>,
whose INT32_MAX capacity cap aborts the process instead of failing once
the total reaches 2 GiB. Assemble the bytes directly in the result
ArrayBuffer instead, mirroring the all-binary arm: capacities are
size_t, results up to the 4 GiB ArrayBuffer maximum work, and larger
totals throw a catchable out-of-memory error. This also drops the final
Vector-to-ArrayBuffer copy and clamps each chunk copy to its measured
size so a chunk resized mid-pass cannot overrun or expose uninitialized
bytes.
JSON.parse on the aborted child's empty stdout failed with a parse
error instead of showing what happened. Comparing the raw string keeps
the informative assertion first and diffs against "" on a crash.
- Advance string writes by the measured size: the encoder reports its
  byte count through a u32 that wraps to 0 for an exactly-2^32-byte
  string, which would have truncated the result via the right-sizing
  branch. The sizer/writer contract is what encodeStringToUint8Array
  already relies on.
- Gate the multi-GiB tests on os.totalmem() >= 10 GiB (Windows commits
  allocations upfront; low-memory Linux would OOM-kill, not throw).
- Test a string chunk larger than 2 GiB (UTF-16 source so the encode
  takes the simdutf path), and a string write landing past 2^31.
- Test chunk mutation via Array.prototype accessor reentry during the
  sizing pass: shrink and transfer take the re-create arm, a grow is
  clamped to the measured size instead of overrunning the pre-sized
  result buffer.
@Jarred-Sumner
Jarred-Sumner force-pushed the farm/5b3ce9f6/stream-mixed-2gb-abort branch from c7b97b2 to c881f1a Compare August 17, 2026 02:56

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. Because it rewrites a memory-safety-critical native path (uninitialized ArrayBuffer assembly with computed memcpy bounds and reentrancy clamping), a human look would still be worthwhile.

What was reviewed:

  • Bounds: every write-pass advance is ≤ the sizing-pass measuredByteLength, so offset cannot exceed total; subspan and memcpy stay in-bounds even when a chunk grows between passes.
  • Uninitialized exposure: the offset < bytes.size() re-create path covers shrink/detach; string writes now rely on the documented sizer/writer agreement (previously a runtime shrink) — traced both encoder helpers to confirm the contract.
  • GC/lifetime: the write pass makes no JS calls (uses values.at), and resultBuffer is a stable RefPtr<ArrayBuffer>, so the bytes span cannot dangle.
  • Tests: verified the marker offsets and the accessor-reentry expectations by hand for shrink/transfer/grow.
Extended reasoning...

Overview

The PR rewrites the mixed string+binary arm of concatenateChunks in src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp to assemble directly into a JSC::ArrayBuffer (via tryCreateUninitialized) instead of staging through a WTF::Vector<uint8_t>, whose INT32_MAX capacity cap CRASH()es on totals ≥ 2 GiB. It also records each binary chunk's measured size during the sizing pass and clamps the write-pass copy to that measurement, then re-creates the result at the actual written length if any chunk shrank/detached in between. Seven subprocess tests are added covering both consumers at 2 GiB and 4 GiB boundaries, a >2 GiB single-string chunk, and Array.prototype accessor reentry that resizes/transfers a chunk mid-concatenation.

Security risks

The primary risk is memory safety: writing into an uninitialized buffer with computed offsets, and defending against reentrant resize of user-controlled buffers. I traced the offset invariant (each contribution ≤ its measured size, so the running offset never exceeds total) and the uninitialized-tail handling (re-create at offset when short). The one behavioral tightening is that string writes now advance by the measured length without the old runtime shrink fallback — correctness depends on the documented sizer/writer agreement in utf8ByteLengthWithReplacement/writeUTF8, which is asserted in debug. No injection, auth, or data-exposure surface.

Level of scrutiny

High. This is native C++ on a hot user-reachable path, with an uninitialized allocation, raw memcpy, span arithmetic across two passes, and adversarial reentrancy considerations (getter reentry, cross-thread SAB growth). The fix is well-reasoned and the tests are unusually thorough, but per the approval guidelines this is not a simple/mechanical change.

Other factors

All prior review-thread items (CodeRabbit's stdout-assertion ordering, comment-cop trims) are resolved. The evidence block shows the new tests fail on the unfixed build (subprocess aborts) and pass with the fix on both debug+ASAN and release. The all-binary fast path (flattenArrayOfBuffersIntoArrayBufferOrUint8Array) is untouched. Given the memory-safety sensitivity and the load-bearing sizer/writer invariant, deferring to a human reviewer rather than auto-approving.

Jarred-Sumner pushed a commit that referenced this pull request Aug 18, 2026
…e than 2^31-1 bytes (#37237)

### Problem

`Bun.readableStreamToText(rs)` and `new Response(rs).text()` abort the
process (uncatchable SIGABRT, `panic(main thread): abort() called`) when
the stream's binary chunks sum to more than 2^31-1 bytes, even though
each individual chunk is well under the limit. The same abort happens
for the direct-stream text sink (`type: "direct"`).

```js
const n = 715827883; // 3*n = 2^31+1
const rs = new ReadableStream({
  start(c) {
    for (let i = 0; i < 3; i++) c.enqueue(new Uint8Array(n));
    c.close();
  },
});
await Bun.readableStreamToText(rs); // rc 134, nothing catchable
```

Node's equivalent (`new Response(rs).text()`) throws a catchable error.

### Cause

Three distinct abort paths in the text consumers' finish arms:

- `finishTextAccumulator` (BunStreamConsumers.cpp) guarded its
`reserveInitialCapacity` estimate with `estimatedLength <
numeric_limits<uint32_t>::max()`, a check in the wrong width:
`WTF::Vector<uint8_t>` caps capacity at INT32_MAX
(`isValidCapacityForVector`) and `CRASH()`es above it in
`WTF::VectorBufferBase<uint8_t>::allocateBuffer`, so totals in [2^31,
2^32) aborted in the reserve before the function's
`exceedsStringLimit()` throw was reached.
- `finishTextSink` (JSDirectStreamController.cpp) had no size guard at
all and hit the same `CRASH()` while growing the vector through
incremental `append`.
- Both arms encoded string pieces and the trailing rope through
`WTF::String::utf8()`, which `RELEASE_ASSERT`s once its conversion
scratch passes INT32_MAX (2x the length for 8-bit strings, 3x for 16-bit
strings containing lone surrogates). A mixed stream with a near-limit
string chunk aborted in the encode even when the actual UTF-8 total fit
the limit.

### Fix

In both finish arms:

- Check `estimatedLength` against `WTF::StringImpl::MaxLength` (via the
existing `exceedsStringLimit`) before touching the vector. Sizes are
recorded at write time (binary chunk sizes exact, string chunks as
UTF-16 code units, which UTF-8 re-encoding never shrinks below), so an
estimate past the limit is final and throwing early is correct. This
also fails fast, before copying gigabytes.
- Use `tryReserveInitialCapacity` / `tryGrow` / `tryAppend` for every
vector operation so capacity and allocation failures surface as
catchable errors instead of `CRASH()`.
- Encode string pieces and the trailing rope through a new shared
helper, `appendUTF8WithinStringLimit`: size with the simdutf sizer,
reject past the string limit before allocating, then write straight into
the byte vector. This removes the `String::utf8()` scratch-allocation
aborts, and lone surrogates now consistently become U+FFFD on every
mixed-path arm (string chunks already did).
- Release the accumulator on the chunk-append error path, which was the
one error exit in `finishTextAccumulator` that skipped it.

The thrown error is the same `RangeError: Out of memory` these consumers
already throw at the string limit (the synthetic-limit tests in
`test/js/web/streams/streams.test.js` pin that contract).

### Verification

`test/js/web/streams/streams-string-limit.test.ts`, five subprocess
tests (the file skips on machines with less than 8GB of RAM):

- queue-backed stream, direct stream, and `Response(stream).text()` with
3 binary chunks summing to 2^31+1 bytes: abort before, `threw RangeError
Out of memory` with exit 0 after
- mixed chunks with a 1.2e9-char ASCII string chunk: aborted in the
`String::utf8()` encode before even though the UTF-8 total fits, now
resolves with the full 1200000001-char text
- mixed chunks whose UTF-8 expansion passes the limit (1.2e9 U+00E9
chars, 2.4e9 UTF-8 bytes): abort before, catchable rejection after

All verified to fail on the unfixed build (release 1.4.0 canary and
debug+ASAN with the src changes stashed) and pass with the fix. Also
green locally: `streams.test.js`, `body.test.ts`, `body-stream.test.ts`,
`direct-readable-stream.test.tsx`, `utf8-bom.test.ts`,
`blob-oom.test.ts`, `stream-fast-path.test.ts` (10k+ tests).

The sibling crash for
`readableStreamToArrayBuffer`/`readableStreamToBytes` with mixed chunks
(`concatenateChunks`, same file) is a separate consumer with different
constraints (an ArrayBuffer result may legitimately exceed 2^31-1) and
is handled in #37239.

<!-- robobun:evidence:begin -->

---

**[review]** gate passed · iteration 5 · 4 files touched

<details><summary>fails on main (without fix)</summary>

```console
ASAN without fix: 6 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/streams/streams-string-limit.test.ts
bun test v1.4.0 (8326d1b)

test/js/web/streams/streams-string-limit.test.ts:
42 |           for (let i = 0; i < 3; i++) c.enqueue(new Uint8Array(n));
43 |           c.close();
44 |         },
45 |       })`),
46 |     );
47 |     expect(result).toEqual(threw);
                        ^
error: expect(received).toEqual(expected)

  {
-   "exitCode": 0,
+   "exitCode": 134,
    "stderr": "",
-   "stdout": 
- "threw RangeError Out of memory
- "
- ,
+   "stdout": "",
  }

- Expected  - 5
+ Received  + 2

      at <anonymous> (/workspace/bun/test/js/web/streams/streams-string-limit.test.ts:47:20)
(fail) text consumers reject binary chunks summing past 2^31-1 > queue-backed ReadableStream [483.96ms]
55 |           for (let i = 0; i < 3; i++) c.write(new Uint8Array(n));
56 |           c.end();
57 |         },
58 |       })`),
59 |     );
60 |     expect(result).toEqual(threw);
                        ^
error: expect(received).toEqual(expected)

  {
-   "exitCode": 0,
+   "exitCode": 134,
    "
... (truncated)

release without fix: 6 FAILED
bun test v1.4.0-canary.1 (8326d1b)

test/js/web/streams/streams-string-limit.test.ts:
42 |           for (let i = 0; i < 3; i++) c.enqueue(new Uint8Array(n));
43 |           c.close();
44 |         },
45 |       })`),
46 |     );
47 |     expect(result).toEqual(threw);
                        ^
error: expect(received).toEqual(expected)

  {
-   "exitCode": 0,
-   "stderr": "",
-   "stdout": 
- "threw RangeError Out of memory
+   "exitCode": 134,
+   "stderr": 
+ "============================================================
+ Bun Canary v1.4.0-canary.1 (8326d1b) Linux x64
+ Linux Kernel v6.17.0 | glibc v2.41
+ CPU: sse42 popcnt avx avx2 avx512
+ Args: "/workspace/bun/build/release/bun" "-e" "\n    const n = 715827883;\n    const rs = new ReadableStream({\n        start(c) {\n          for (let i = 0; i < 3; i++) c.enqueue(new Uint8Array(n));\n          c.c"...
+ Features: bunfig jsc tsconfig 
+ Builtins: "bun:main" 
+ 
+ Elapsed: 5ms | User: 1ms | Sys: 5ms
+ RSS: 32.17 MB | Peak: 46.52 MB | Commit: 2.19 GB | Faults: 0 | Machine: 34.36 GB
+ 
+ panic(main thread): abort() called
+ oh no: Bun has crashed. This indicates a bug in Bun, not your code.
+ 
+ To send a re
... (truncated)
```

</details>

<details><summary>passes on PR (with fix)</summary>

```console
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/streams/streams-string-limit.test.ts
bun test v1.4.0 (8326d1b)

test/js/web/streams/streams-string-limit.test.ts:
(pass) text consumers reject binary chunks summing past 2^31-1 > queue-backed ReadableStream [329.73ms]
(pass) text consumers reject binary chunks summing past 2^31-1 > direct ReadableStream [439.48ms]
(pass) text consumers reject binary chunks summing past 2^31-1 > Response(stream).text() [306.04ms]
(pass) text consumers reject binary chunks summing past 2^31-1 > mixed chunks with a big ASCII string chunk resolve when the UTF-8 total fits [8137.48ms]
(pass) text consumers reject binary chunks summing past 2^31-1 > mixed chunks whose UTF-8 expansion passes the limit reject [2418.01ms]
(pass) text consumers reject binary chunks summing past 2^31-1 > direct stream with a buffer grown past the limit after write() rejects [457.06ms]

 6 pass
 0 fail
 6 expect() calls
Ran 6 tests across 1 file. [14.49s]
__F:0:S:0

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 673ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/38] cxx obj/src/jsc/bindings/webcore/streams/JSCompressionStreamShared.cpp.o
[2/38] cxx obj/src/jsc/bindings/webcore/streams/CrossRealmTransform.cpp.o
[3/38] cxx obj/src/jsc/bindings/webcore/streams/JSCompressionStream.cpp.o
[4/38] cxx obj/src/jsc/bindings/webcore/streams/BunAsyncIterableSource.cpp.o
[5/38] cxx obj/src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp.o
[6/38] cxx obj/src/jsc/bindings/webcore/streams/JSByteLengthQueuingStrategy.cpp.o
[7/38] cxx obj/src/jsc/bindings/webcore/streams/JSCountQueuingStrategy.cpp.o
[8/38] cxx obj/src/jsc/bindings/webcore/streams/JSDecompressionStream.cpp.o
[9/38] cxx obj/src/jsc/bindings/webcore/streams/BunStreamSource.cpp.o
[10/38] cxx obj/src/jsc/bindings/webcore/streams/JSDirectStreamController.cpp.o
[11/38] cxx obj/src/jsc/bindings/webcore/streams/JSReadableByteStreamController.cpp.o
[12/38] cxx obj/src/jsc/bindings/webcore/streams/JSReadableStream.cpp.o
[13/38] cxx obj/src/jsc/bindings/webcore/streams/JSReadRequest.cpp.o
[14/38] cxx obj/src/jsc/binding
... (truncated)
```

</details>

<details><summary>diff hotspot</summary>

```
.../webcore/streams/BunStreamConsumers.cpp         |  70 +++++++---
 .../webcore/streams/JSDirectStreamController.cpp   |  29 +++-
 .../bindings/webcore/streams/WebStreamsInternals.h |   2 +
 test/js/web/streams/streams-string-limit.test.ts   | 155 +++++++++++++++++++++
 4 files changed, 231 insertions(+), 25 deletions(-)
```

</details>

**gate history** · 6 passed · 0 rejected · iteration 5

<details><summary>evidence per changed file</summary>

```
file                                                      reads  edits  tests
src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp       5      7      0
…c/bindings/webcore/streams/JSDirectStreamController.cpp      4      8      0
src/jsc/bindings/webcore/streams/WebStreamsInternals.h        2      3      0
test/js/web/streams/streams-string-limit.test.ts              5      7      0
```

</details>

<!-- robobun:evidence:end -->

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant