Throw instead of aborting when stream text() consumers accumulate more than 2^31-1 bytes - #37237
Conversation
WalkthroughChangesStream text consumers now enforce the maximum accumulated string size and use fallible allocation and append operations. Tests cover queue-backed streams, direct streams, Stream text accumulation safety
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp:738-740— TheappendChunkBytesloop infinishTextAccumulatorreturns viaRETURN_IF_EXCEPTION/if (!appended) returnwithout callingreleaseAccumulated(), while every other error return in this function — including the three this PR just added at ~728, ~734, ~753 — does. The PR's newtryGrow/tryAppendOOM throws insideappendChunkBytesnow flow through this un-released return, so for consistency with the function's own "release at every return below" comment it'd be worth callingreleaseAccumulated()before both loop returns too. (Delayed release only, not a leak — the sole caller's owner sink is a fresh local — so not blocking.)Extended reasoning...
What the issue is
finishTextAccumulator(BunStreamConsumers.cpp) opens with the invariant comment "release at every return below (the owner can outlive this call by a lot)" and defines areleaseAccumulatedlambda that resetsaccumulator.pieces(the WriteBarrier'd chunk buffers rooted on the owner cell). The pieces loop at lines 738-746, however, has two returns that do not call it:for (auto& piece : accumulator.pieces) { JSValue value = piece.get(); if (!value) continue; bool appended = appendChunkBytes(vm, globalObject, value, bytes); RETURN_IF_EXCEPTION(scope, WTF::String()); // <- no releaseAccumulated() if (!appended) return WTF::String(); // <- no releaseAccumulated() }
Every other error return in the function calls
releaseAccumulated()first — including the three new returns this PR adds (theestimatedLength > MaxLengththrow at ~728, thetryReserveInitialCapacityfailure at ~734, and the trailing-ropetryAppendfailure at ~753).The code path this PR opens
Before this PR,
appendChunkBytescould only throw viaasString(chunk)->value(globalObject)(rope resolve OOM) or the trailingthrowTypeErrorfor a bad chunk type. This PR adds three new OOM throw sites insideappendChunkBytes:bytes.tryGrow(oldSize + byteLength)failing on a string chunk whose UTF-8 expansion pushes past the reserved capacity (line ~333)bytes.tryAppend(view->span())failing for an ArrayBufferView chunk (line ~347)bytes.tryAppend(impl->span())failing for an ArrayBuffer chunk (line ~354)
All of these set the pending exception and return
false, which flows through the loop'sRETURN_IF_EXCEPTION(scope, WTF::String())at line 743 — without releasing the accumulator. So the PR simultaneously (a) added new OOM paths that reach this return and (b) addedreleaseAccumulated()to the three sibling error returns immediately above and below it, but left the loop return itself as-is.Why the impact is minimal (nit-level)
This is a delayed release, not a leak, and in practice releases nothing sooner:
- The only caller of
finishTextAccumulatorisconvertChunksToText(line 630), where the owner is a fresh localJSBunStandaloneTextSinkcreated two lines earlier. On the exception return, control unwinds out ofconvertChunksToTextand the sink becomes unreachable — it (and itsm_accumulator.pieces) is collected at the next GC cycle. - On this error path the same chunk buffers are also still held by the caller's
MarkedArgumentBuffer valuesand by the originalchunksJSArray (releaseInternalChunkArrayonly runs on the success path ofconvertChunkArrayPromise). So even ifreleaseAccumulated()ran here, the underlying ArrayBuffers would still be rooted until the caller returns. - The loop-return code itself is pre-existing unchanged context — the diff shows these lines as context, not additions.
appendChunkBytescould already throw before this PR (rope-resolve OOM, type error), so the un-released return already existed; the PR extends its reachable surface rather than introducing it.
Step-by-step walkthrough
- A queue-backed stream enqueues one small binary chunk followed by one large string chunk (e.g. 1.5 GB of UTF-16 code units that expand to >2 GB of UTF-8), so
estimatedLengthstays underMaxLengthand the mixed-chunk arm runs. convertChunksToTextcreates a freshJSBunStandaloneTextSink, writes both chunks intosink->m_accumulator, then callsfinishTextAccumulator.estimatedLength(~1.5e9) passes both new guards;tryReserveInitialCapacity(1.5e9)succeeds.- The pieces loop calls
appendChunkByteson the string chunk;utf8ByteLengthWithReplacementreports ~3 GB;bytes.tryGrow(oldSize + 3e9)fails;throwOutOfMemoryErrorsets the pending exception;appendChunkBytesreturnsfalse. - Back in the loop,
RETURN_IF_EXCEPTION(scope, WTF::String())returns immediately — without callingreleaseAccumulated().sink->m_accumulator.piecesstill holds two WriteBarrier'd JSValues. convertChunksToTextsees the exception and returns{}; the sink local goes out of scope. At the next GC the sink and its pieces are collected. The chunk buffers were also still rooted byvaluesand the caller's chunks array anyway.
How to fix
Call
releaseAccumulated()on both loop exits, matching the sibling returns:bool appended = appendChunkBytes(vm, globalObject, value, bytes); if (scope.exception() || !appended) [[unlikely]] { releaseAccumulated(); RETURN_IF_EXCEPTION(scope, WTF::String()); return WTF::String(); }
(or equivalently expand
RETURN_IF_EXCEPTIONinline and add the release before both returns). This closes the gap with the function's stated "release at every return below" invariant and REVIEW.md's "same error-path sequence as sibling exit sites" rule.
|
Good catch on the append-loop exit: it was the one error return in finishTextAccumulator that skipped releaseAccumulated(), and the new out-of-memory throws in appendChunkBytes route through it. Fixed in ed3b2b3 so that path releases the accumulator like the sibling exits. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp`:
- Around line 330-334: Reject UTF-8-expanded input before allocation in
BunStreamConsumers.cpp:330-334 by validating the encoded byte total before
tryGrow. In BunStreamConsumers.cpp:718-751, replace unchecked rope.utf8() with a
sized, fallible encoding path that enforces the byte limit. In
JSDirectStreamController.cpp:274-315, size each string before String::utf8() and
reject aggregates exceeding the limit before allocation.
🪄 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: 878d3add-b756-4f18-8cbe-13e12f9265ef
📒 Files selected for processing (3)
src/jsc/bindings/webcore/streams/BunStreamConsumers.cppsrc/jsc/bindings/webcore/streams/JSDirectStreamController.cpptest/js/web/streams/streams-string-limit.test.ts
There was a problem hiding this comment.
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 `@src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp`:
- Around line 345-347: Validate the current binary span size before every binary
tryAppend: in src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp:345-347
and src/jsc/bindings/webcore/streams/JSDirectStreamController.cpp:292-312, check
addition overflow and exceedsStringLimit(bytes.size() + span.size()) before
vector growth, preserving the catchable out-of-memory error path. In
test/js/web/streams/streams-string-limit.test.ts:81-133, add queue-backed and
direct-stream cases that resize a small resizable ArrayBuffer beyond the limit
after enqueue/write and assert RangeError: Out of memory.
🪄 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: b2f29f1f-ff11-4b87-8c36-852a9f111b51
📒 Files selected for processing (4)
src/jsc/bindings/webcore/streams/BunStreamConsumers.cppsrc/jsc/bindings/webcore/streams/JSDirectStreamController.cppsrc/jsc/bindings/webcore/streams/WebStreamsInternals.htest/js/web/streams/streams-string-limit.test.ts
There was a problem hiding this comment.
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/web/streams/streams-string-limit.test.ts`:
- Around line 137-148: Update the stream test around the ab.resize call in
pull() to print a distinct success marker immediately after
ab.resize(2400000000) returns, then require that marker in stdout alongside the
expected consumer error. Keep the existing rejection assertion, ensuring the
test only passes when resizing succeeds and Bun.readableStreamToText(rs)
subsequently rejects through the intended direct-stream append path.
🪄 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: da956ed2-c763-4baa-85e0-015135642f6a
📒 Files selected for processing (1)
test/js/web/streams/streams-string-limit.test.ts
|
Updated 7:25 PM PT - Aug 16th, 2026
✅ @robobun, your commit 0189c88f42c2cafb55d8f86609567517babca1b8 passed in 🧪 To try this PR locally: bunx bun-pr 37237That installs a local version of the PR into your bun-37237 --bun |
There was a problem hiding this comment.
I reviewed this PR (including the post-9df4ceb changes) and found no bugs. Because it touches ~100 lines of allocation-sensitive C++ in the streams text-consumer paths and adds a new cross-file encoding helper, a human look would still be worthwhile.
What was reviewed:
appendUTF8WithinStringLimit: verifiedoldSize + byteLengthcannot overflowsize_t(both operands bounded well below 2^63) and thatwriteUTF8is bounded by the grown span.finishTextAccumulator/finishTextSink: confirmed the double-compare againstStringImpl::MaxLengthguards thesize_tcast, and every new error return releases the accumulator (directly, or viaendTextSink's unconditional reset).tryAppendon binary spans: confirmed it validates capacity before allocating, so a resizable buffer grown past the limit betweenwrite()andend()rejects catchably (covered by the new test).- Tests: subprocess pattern drains all pipes concurrently, asserts the combined
{stdout, stderr, exitCode}, andskipIf-gates on 8GB RAM.
Extended reasoning...
Overview
The PR converts three uncatchable abort() paths in the stream text consumers into catchable RangeError: Out of memory rejections when accumulated chunks exceed StringImpl::MaxLength (2^31-1). It touches finishTextAccumulator and appendChunkBytes in BunStreamConsumers.cpp, finishTextSink in JSDirectStreamController.cpp, adds a shared appendUTF8WithinStringLimit helper declared in WebStreamsInternals.h, and adds a 6-case subprocess test file.
Since my prior inline comment (10:41Z, addressed in 5d81613), commits 9df4ceb–af1ee12 substantially extended the fix: the String::utf8() scratch-allocation abort was identified as a third crash path and replaced with a simdutf size-then-write helper at every string-piece and rope encode site, and three more tests were added (big-ASCII-fits, UTF-8-expansion-overflows, resize-after-write). All review-thread items — mine, comment-cop's, and CodeRabbit's three findings — are resolved.
Security risks
None identified. The change strictly tightens validation: sizes are checked against StringImpl::MaxLength before allocation, and all Vector growth switches to fallible try* variants. No new user-controlled data reaches a parser or path; the error is the same throwOutOfMemoryError these consumers already throw at the string limit.
Level of scrutiny
Medium-high. This is C++ in the JSC bindings streams subsystem — allocation sizing, WTF::Vector capacity semantics, and UTF-8 byte-length arithmetic on untrusted stream data. The transformation is mechanical (crashing ops → try* + pre-checks) and the new helper is a straightforward extraction of existing size-then-write logic, but the file's memory-safety stakes and the number of call sites changed put it past what I'd auto-approve.
Other factors
- The evidence block shows all 6 tests fail on main (ASAN and release) and pass with the fix; the sibling
streams.test.js/body.test.ts/ etc. suites still pass. - The new test file runs ~15s under ASAN and
skipIf-gates on available memory, so it won't flake on small runners. - The one behavioral edge I raised earlier (direct sink rejects when >2GiB is written then detached before
end()) was intentionally kept as a rejection with a corrected comment — the old silently-drop behavior was itself wrong. concatenateChunks(thetoArrayBuffer/toBytessibling) still has the same unboundedreserveInitialCapacity/grow; the PR description explicitly scopes that to #37239, which is reasonable since ArrayBuffer results may legitimately exceed 2^31-1.
There was a problem hiding this comment.
I reviewed this PR and the bug-hunting pass found no issues. Both of my earlier inline comments (undrained stderr in the test helper; the direct-sink estimate-overcount edge) have been addressed, as have the CodeRabbit findings on String::utf8() scratch aborts and resizable-buffer growth. Because this is non-trivial C++ touching allocation and size-limit paths in the streams text consumers, a maintainer sign-off would still be worthwhile.
Checked: appendUTF8WithinStringLimit size arithmetic (oldSize+byteLength cannot overflow size_t; exceedsStringLimit gates before tryGrow); finishTextAccumulator now releases the accumulator on every error exit including the append-loop path; finishTextSink error exits are covered by endTextSink's unconditional m_textAccumulator.reset(); the six subprocess tests drain stdout/stderr concurrently, assert the combined {stdout,stderr,exitCode} object, and the resize test now proves ab.resize() succeeded before the rejection.
Extended reasoning...
Overview
The PR converts three process-abort paths in the stream .text() consumers into catchable RangeError: Out of memory when accumulated bytes exceed StringImpl::MaxLength (2^31-1). It touches BunStreamConsumers.cpp (finishTextAccumulator, appendChunkBytes, new appendUTF8WithinStringLimit), JSDirectStreamController.cpp (finishTextSink), a header declaration in WebStreamsInternals.h, and adds test/js/web/streams/streams-string-limit.test.ts with six subprocess tests gated on ≥8GB RAM.
Security risks
None introduced. The change is defensive: it replaces WTF::Vector CRASH() and WTF::String::utf8() RELEASE_ASSERT sites with fallible tryReserveInitialCapacity/tryGrow/tryAppend and an up-front exceedsStringLimit check, so oversized user input now rejects instead of aborting. The size addition in appendUTF8WithinStringLimit (oldSize + byteLength) cannot overflow on 64-bit (both operands are bounded well below 2^33). The estimate check compares in double first before casting to size_t.
Level of scrutiny
Moderate-to-high. This is C++ bindings code in a hot user-facing subsystem, with size arithmetic on user-controlled data — exactly the class REVIEW.md flags for careful review. The change itself is narrow and strictly-more-defensive (every previous unchecked append/grow/utf8() becomes a checked-and-throw), so the correctness risk is low, but it is not a mechanical change: it required understanding WTF::Vector's INT32_MAX capacity cap, String::utf8()'s 2×/3× scratch-allocation assert, and the direct sink's write()/end() timing gap for resizable buffers. That is enough non-obvious reasoning that a maintainer should confirm the approach.
Other factors
- The PR went through several bot-review rounds (comment-cop on comment length; CodeRabbit on the
String::utf8()abort and resizable-buffer growth; my own on stderr draining and the direct-sink estimate invariant). All threads are resolved and the fixes are visible in the current diff. - The evidence block shows the mechanical gate passed on iteration 2 with fails-on-main (ASAN: 6 failed, release: 2 failed) / passes-on-PR (all pass) on both build profiles, and 10k+ related tests green locally.
- The one intentional behavior change I flagged earlier (direct sink now rejects when >2GiB was written then detached before
end(), where it previously silently returned the truncated live bytes) is documented in the reworded code comment and is the safer outcome. finishTextSinkdoes not call areleaseAccumulated()on its error returns, unlike its twin — verified this is correct becauseendTextSinkunconditionally resetsm_textAccumulatorafterfinishTextSinkreturns, before checking the exception scope.- The sibling
concatenateChunks(toArrayBuffer/toBytes with mixed chunks) still uses uncheckedreserveInitialCapacity/grow/append; the PR description explicitly scopes that to #37239, which is reasonable given the different result-size constraint.
|
CI state for the final sha (699e685): everything is green except the darwin 14 x64 test-bun job, which fails only on test/napi/napi.test.ts. That failure is marked pre-existing by the CI tooling (it fails identically on main and on the previous build of this branch before an empty retrigger commit) and has been reported for triage separately; nothing in this PR touches NAPI. The fail-before/pass-after evidence check and all lanes exercising the changed code passed. Ready for review. |
…g limit Bun.readableStreamToText, Response(stream).text(), and the direct-stream text sink abort the process (uncatchable SIGABRT) when the accumulated binary chunks total more than 2^31-1 bytes, even though every individual chunk fits. finishTextAccumulator guarded its reserveInitialCapacity estimate against uint32 max, but WTF::Vector<uint8_t> caps capacity at INT32_MAX and CRASH()es above it, so totals in [2^31, 2^32) aborted before the existing exceedsStringLimit() throw could run. The direct stream's finishTextSink had no guard at all and hit the same CRASH() through incremental append. Check the estimated length against WTF::StringImpl::MaxLength up front (the estimate only undercounts, so exceeding the limit is final) and use tryReserveInitialCapacity/tryGrow/tryAppend for the byte vector so capacity and allocation failures surface as the same catchable out-of-memory RangeError these consumers already throw at the string limit.
A regression back to the abort writes the panic banner and ASAN report
to stderr; asserting the combined { stdout, stderr, exitCode } object
surfaces that output directly in the failure diff instead of an
empty-vs-expected stdout comparison.
appendChunkBytes can now throw out-of-memory, and the loop return it flows through was the one error exit in finishTextAccumulator that skipped releaseAccumulated().
User code runs between write() and end(), so a detach can make the estimate overcount there; the rejection is still the intended result.
…ng::utf8() String::utf8() RELEASE_ASSERTs once its conversion scratch passes INT32_MAX (2x the length for 8-bit strings, 3x for 16-bit with lone surrogates), so a mixed stream with a near-limit string chunk still aborted inside the encode even when the UTF-8 total fit the string limit. Add appendUTF8WithinStringLimit: size with the simdutf sizer, reject past the string limit, then write straight into the byte vector (lone surrogates become U+FFFD, matching the chunk appenders). Use it for string pieces and the trailing rope in both finish arms.
699e685 to
0189c88
Compare
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs; the one inline nit I raised earlier (the direct-sink estimate comment) was addressed. Because this is native C++ touching size/capacity arithmetic and allocation-failure semantics in the streams text consumers, a human look would still be worthwhile.
What was reviewed:
appendUTF8WithinStringLimit: overflow ofoldSize + byteLength— bounded well below SIZE_MAX (vector capped at INT32_MAX, string UTF-8 length ≤ 3·MaxLength).estimatedLengthdouble→size_t cast — guarded by the preceding> static_cast<double>(MaxLength)check, so the cast is in range.- Accumulator release on the new error exits in
finishTextAccumulator;finishTextSink's caller already resets on all paths. tryAppend/tryGrowcover the resizable-buffer-grown-after-write case without a redundant pre-check (test added).
Extended reasoning...
Overview
The PR replaces three process-abort paths in the stream text consumers with catchable RangeError: Out of memory when accumulated bytes exceed StringImpl::MaxLength (2^31-1). It touches finishTextAccumulator in BunStreamConsumers.cpp, finishTextSink in JSDirectStreamController.cpp, adds a shared appendUTF8WithinStringLimit helper (declared in WebStreamsInternals.h), and adds six subprocess tests in test/js/web/streams/streams-string-limit.test.ts. The mechanical changes are: (1) an up-front exceedsStringLimit check on the code-unit estimate, (2) fallible tryReserveInitialCapacity/tryGrow/tryAppend in place of crashing reserveInitialCapacity/grow/append, (3) simdutf-sized direct UTF-8 writes replacing WTF::String::utf8() (whose scratch allocation RELEASE_ASSERTs past INT32_MAX), and (4) releasing the accumulator on the append-loop error exit that previously skipped it.
Security risks
None identified. This is a DoS-class fix (uncatchable abort → catchable rejection) on user-controlled stream input; it does not introduce new parsing, does not relax any bounds check, and validates sizes before allocating. The size arithmetic was checked for overflow: oldSize + byteLength in the new helper is bounded by INT32_MAX + 3·INT32_MAX on 64-bit; the static_cast<size_t>(estimatedLength) is only reached after the double comparison against MaxLength passes, so the cast cannot be out of range.
Level of scrutiny
High. This is native C++ in the Web Streams subsystem touching size/index arithmetic on external data — the category REVIEW.md flags as most-blocked. The changes look correct and the test coverage is thorough (queue-backed, direct, Response().text(), mixed ASCII under limit, mixed non-ASCII whose UTF-8 expansion exceeds the limit, and a resizable buffer grown past the limit between write() and end()), with fail-before/pass-after evidence on debug+ASAN. But this is not a simple/mechanical change and warrants a maintainer sign-off.
Other factors
The PR went through three gate iterations with substantive fixes applied in response to automated feedback: paragraph-length comments folded, the String::utf8() scratch-allocation abort was identified and fixed via the new shared helper, my earlier note on the direct-sink estimate comment was addressed by rewording, a resizable-buffer test was added, and a resize-success marker was added to that test so it can't pass for the wrong reason. All inline threads are resolved, CI is green modulo a pre-existing unrelated darwin NAPI failure, and the bug hunting system found nothing this run. The sibling concatenateChunks crash for readableStreamToArrayBuffer/readableStreamToBytes is explicitly out of scope (handled in #37239).
2 participants
Problem
Bun.readableStreamToText(rs)andnew 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").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 itsreserveInitialCapacityestimate withestimatedLength < numeric_limits<uint32_t>::max(), a check in the wrong width:WTF::Vector<uint8_t>caps capacity at INT32_MAX (isValidCapacityForVector) andCRASH()es above it inWTF::VectorBufferBase<uint8_t>::allocateBuffer, so totals in [2^31, 2^32) aborted in the reserve before the function'sexceedsStringLimit()throw was reached.finishTextSink(JSDirectStreamController.cpp) had no size guard at all and hit the sameCRASH()while growing the vector through incrementalappend.WTF::String::utf8(), whichRELEASE_ASSERTs 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:
estimatedLengthagainstWTF::StringImpl::MaxLength(via the existingexceedsStringLimit) 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.tryReserveInitialCapacity/tryGrow/tryAppendfor every vector operation so capacity and allocation failures surface as catchable errors instead ofCRASH().appendUTF8WithinStringLimit: size with the simdutf sizer, reject past the string limit before allocating, then write straight into the byte vector. This removes theString::utf8()scratch-allocation aborts, and lone surrogates now consistently become U+FFFD on every mixed-path arm (string chunks already did).finishTextAccumulatorthat skipped it.The thrown error is the same
RangeError: Out of memorythese consumers already throw at the string limit (the synthetic-limit tests intest/js/web/streams/streams.test.jspin 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):Response(stream).text()with 3 binary chunks summing to 2^31+1 bytes: abort before,threw RangeError Out of memorywith exit 0 afterString::utf8()encode before even though the UTF-8 total fits, now resolves with the full 1200000001-char textAll 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/readableStreamToByteswith 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.[review] gate passed · iteration 5 · 4 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 6 passed · 0 rejected · iteration 5
evidence per changed file