Skip to content

TextDecoder: throw when the result cannot be allocated or is too long - #47278

Merged
dylan-conway merged 3 commits into
mainfrom
claude/textdecoder-ascii-alloc-bug-wm16yt
Oct 29, 2026
Merged

dylan-conway merged 3 commits into
mainfrom
claude/textdecoder-ascii-alloc-bug-wm16yt

Conversation

@dylan-conway

@dylan-conway dylan-conway commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

What does this PR do?

new TextDecoder().decode(bytes) loses data or kills the process when the result has no room, if the bytes are all ASCII. With one non-ASCII byte in the input the same call already throws.

all-ASCII input 1.4.4 main this PR
the string cannot be allocated returns "" abort() throws ERR_MEMORY_ALLOCATION_FAILED
2^31 bytes or more returns "" returns "" throws ERR_STRING_TOO_LONG

A reader that joins decoded chunks loses a piece in silence. Repro (Linux, sh -c 'ulimit -v 4194304; bun min.js'):

const bytes = new Uint8Array(32 << 20).fill(97), dec = new TextDecoder(), keep = [];
try {
  for (;;) { const s = dec.decode(bytes); if (s.length !== bytes.length) { console.log("WRONG: copy " + keep.length + " has " + s.length + " characters, not " + bytes.length); break; } keep.push(s); }
} catch (e) { console.log("caught " + e.message); }
1.4.4     WRONG: copy 32 has 0 characters, not 33554432
main      panic(main thread): abort() called          (exit 134)
this PR   caught Failed to allocate memory

Root cause

JsHeap::create_latin1_string is infallible by its type, and both ASCII roads of TextDecoder (utf-8 and latin1/windows-1252) use it.

  • For 2 characters or more it reaches JSC__JSString__createUninitializedLatin1, which had RELEASE_ASSERT(text) on a tryCreateUninitialized that can return null.
  • Over the maximum length it falls back to EncodedSlice::to_js, where a null WTF::String becomes "".

The fix

  • bindings.cpp: the RELEASE_ASSERT is gone. The function, now JSC__JSString__tryCreateUninitializedLatin1, returns an empty value when the allocation is refused.
  • bindgen.rs: new JsHeap::try_create_latin1_string, whose error is ERR_STRING_TOO_LONG or ERR_MEMORY_ALLOCATION_FAILED. It shares one copy routine with create_latin1_string, which stays infallible for its other callers (out of memory there now goes through bun_core::out_of_memory() rather than an assert).
  • TextDecoder.rs: the two ASCII call sites use it. These two lines are the fix for the report.
  • Lengths 0 and 1 no longer allocate at all: they are the VM's empty and single-character strings.

The same crash in the rest of decode()

Three more allocations in the same function aborted on the same refusal (memory allocation of N bytes failed, on 1.4.4 too). They now throw ERR_MEMORY_ALLOCATION_FAILED:

  • the join with bytes carried over from a {stream: true} chunk,
  • windows-1252 with a non-ASCII byte,
  • UTF-16.

The decoder after such an error

These error paths are new, so what they leave behind is new too:

  • "do not flush" is still set before decoding, as the spec has it. A flush that throws ends the stream, and the next call starts a new one with nothing carried into it.
  • A {stream: true} chunk that throws keeps what UTF-8 and UTF-16 carried from earlier chunks (partial character, BOM seen), so the same chunk can be decoded again.
  • The encoding_rs arm is unchanged.

#[inline(never)] on decode_utf16

decode_with_encoding_rs already has it. Inlined, the UTF-16 loop and the UTF-8 arm are one function of about 17 KB, and while working on this, edits to one kept changing the speed of the other (measured without LTO): a change to the UTF-16 capacity arithmetic alone cost a 16-byte UTF-8 decode 9%, and the UTF-16 loop ran 11 to 28% slower per unit depending on the code around it. Out of line, both effects are gone.

Not in this PR

  • new TextDecoderStream() (utf-8, not fatal) has its own C++ road, streamingUTF8Decode. It already throws in these cases, but for an over-long chunk or a failed join it is a plain RangeError: Out of memory with no code, where the other streams now give the two codes above.
  • node:path passes strings of any length to the infallible create_latin1_string, so it still ends the process when out of memory.
  • A fatal UTF-16 decoder that rejects a {stream: true} chunk goes on from that chunk's last odd byte or lead surrogate, so it can garble or reject a valid next chunk: [00 dc 41] throws, then [42 00] gives U+4241. UTF-8 and the encoding_rs arm drop what was pending. main and 1.4.4 do the same as this PR here.

How did you verify your code works?

New tests. All three fail on main and on 1.4.4, and pass with this PR on a debug+ASAN build and on a release build.

  • test/js/node/buffer-oom.test.ts, "TextDecoder reports an allocation that fails": every encoding arm, TextDecoderStream, and the decoder's state afterwards. What refuses the allocation depends on the build, so that each CI lane runs what it can:
    • release on Linux: ulimit -v. Address space is used up with pages that are never written, so the child takes no memory to speak of and well under a second.
    • ASAN: max_allocation_size_mb, for what Rust allocates. The input is a WebAssembly.Memory, which is mapped rather than malloc'ed, so it can be longer than the cap.
    • debug: also BUN_JSC_maxSingleAllocationSize, for what WTF allocates.
  • test/js/web/encoding/text-decoder.test.js, "throws for more characters than a string can hold…": the synthetic limit, in a child process.

Existing tests. On both builds: test/js/web/encoding, test/js/node/fs/fs-oom.test.ts. Debug only, where it is not skipped: test/internal/bindgen.test.ts. Release only: test/js/node/path, test/js/node/string_decoder, test/js/web/streams/streams.test.js.

By hand

  • The repro above, and the same loop with 16 KiB chunks, { stream: true }, latin1 and UTF-16: all end in catch. 2^31 − 1 bytes still decode.
  • BUN_JSC_validateExceptionChecks=1 is clean on the new error paths.
  • 503,107 decode() calls in 96,000 rounds (91,337 of them distinct) on random chunkings of BOMs, split surrogates and partial sequences (4 encodings × fatal × ignoreBOM × random stream) answer the same on main, 1.4.4 and this PR, round for round. The debug build asserts that the UTF-16 reservation is never outgrown, and it did not fire in that run.
  • 17 cases of ordinary streaming behaviour print the same as Node 24.

Performance. main against this PR at the same commit, both built from source with ThinLTO (no PGO). ns per decode(), median of 10 runs in random order, pinned to one core, useConcurrentGC=0 to take GC thread scheduling out of the measurement.

input main this PR
utf-8 1 B 28.1 16.9 −40.0%
utf-8 2 B 30.5 30.6 +0.3%
utf-8 16 B 35.4 32.2 −8.9%
utf-8 64 B 32.8 33.0 +0.3%
utf-8 256 B 44.4 45.4 +2.3%
utf-8 1 KiB 107.0 109.8 +2.6%
utf-8 64 KiB 5842.9 5803.6 −0.7%
latin1 16 B 30.4 30.9 +1.8%
latin1 1 KiB 97.7 99.2 +1.5%
utf-16le 16 B 48.0 57.0 +19.0%
utf-16le 256 B 225.9 232.8 +3.1%
utf-16le 4 KiB 3024.2 3129.2 +3.5%
utf-16le 64 KiB 48807.5 49419.2 +1.3%
utf-16be 4 KiB 3605.3 3093.0 −14.2%
  • Known cost: 9 to 13 ns on every UTF-16 call, which shows on short input (a second run had 2 B at 40.7 → 53.7 and 16 B at 47.8 → 59.6). What I tried:
    • It is not the fallible allocation: with Vec::with_capacity put back in the same structure the cost is the same, and Vec::try_with_capacity recovers 1.4 ns. Nor is it the size of the allocation.
    • Moving the whole UTF-16 arm out of line, with the loop inlined in it and working on a local copy of the carried state, halves the cost (2 B +16%), but with ThinLTO the UTF-16LE loop is then 37% slower from 4 KiB up (without LTO it is 4% faster, and the UTF-16BE loop 16% slower). The loop in a function of its own is the one shape that was steady in both.
  • UTF-8 and latin1 are within the run-to-run spread of main.
  • Full collections over 100,000 decodes of 64 KiB: 604, against 602 on main.
  • bench/snippets/text-decoder.mjs shows no consistent difference: two runs of the same binary differ by more than the two builds do.
  • node:path is unchanged, except that an answer of one character (path.normalize("/a/..")) is about 35% faster (measured without LTO).

Generated by Claude Code

`decode()` of all-ASCII bytes aborted the process when the string could not
be allocated (released versions returned "" instead), and returned "" for
2^31 bytes or more. It now throws ERR_MEMORY_ALLOCATION_FAILED or
ERR_STRING_TOO_LONG, as it already did for input with a non-ASCII byte.

The other allocations in `decode()` aborted on the same refusal and now throw
too: the join with bytes carried from a `{stream: true}` chunk, windows-1252
with a non-ASCII byte, and UTF-16.

A `{stream: true}` chunk that fails this way keeps what UTF-8 and UTF-16
carried from earlier chunks, so it can be decoded again. A failed flush still
ends the stream.

Co-authored-by: dylanc <dylanc@anthropic.com>
@robobun

robobun commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator
Updated 1:53 AM PT - Oct 29th, 2026

✅ @claude, your commit b6b0de92cb2fd43c1456ccc7049312ce4dc22bb3 passed in Build #134878! 🎉


🧪   To try this PR locally:

bunx bun-pr 47278

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

bun-47278 --bun

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 0e99938 and b6b0de9.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between d44b8c3 and 0e99938.

Files selected for processing: 1
  • src/jsc/JSString.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

Latin-1 string creation now supports fallible allocation. TextDecoder uses fallible allocations for decoded output and updates buffered decoder state around failures. New tests cover allocation failures and oversized decode results.

Changes

Fallible string creation and decoding

Layer / File(s) Summary
Fallible Latin-1 string creation
src/jsc/JSString.rs, src/jsc/bindings/bindings.cpp, src/jsc/bindgen.rs, docs/project/bindgen.mdx
JavaScriptCore adds single-character creation and fallible Latin-1 allocation. Bindgen adds try_create_latin1_string, which returns errors for excessive length or allocation failure. The documentation describes the distinction between fallible and process-ending allocation paths.
Fallible decoding and decoder state
src/runtime/webcore/TextDecoder.rs
TextDecoder uses fallible reservations and string creation across UTF-8, UTF-16, Latin-1, and CP1252 decoding. It updates buffered bytes, surrogate, and BOM state around errors and stream boundaries.
Allocation-failure and size-limit tests
test/js/node/buffer-oom.test.ts, test/js/web/encoding/text-decoder.test.js
Tests cover allocation failures during decoding and streaming, plus oversized decode results across encodings. They also check decoder behavior after failures and at the configured size limit.

Suggested reviewers: jarred-sumner, robobun

Priority: Normal

Merge Risk: Minimal · previous review 0e999

No concrete merge-blocking issue is established by the supplied changes. Merge readiness remains subject to normal build and test checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title names the main change: TextDecoder.decode() now throws when the result cannot be allocated or exceeds the maximum length, instead of returning an empty string or aborting. The changeset matc…
Description check Passed The description follows the required template. It has the 'What does this PR do?' section, which explains the problem, the root cause, and the fix. It also has the 'How did you verify your code works?…
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.
Full details: Title check

Explanation

The title names the main change: TextDecoder.decode() now throws when the result cannot be allocated or exceeds the maximum length, instead of returning an empty string or aborting. The changeset matches this. The title is specific and accurate, and it does not cover the UTF-16, stream-join, and windows-1252 changes, which is acceptable for a title.

Full details: Description check

Explanation

The description follows the required template. It has the 'What does this PR do?' section, which explains the problem, the root cause, and the fix. It also has the 'How did you verify your code works?' section, which lists new tests, existing test suites, manual checks, and performance measurements. It adds a root-cause section, a note on decoder state after errors, and a list of items out of scope. These extra sections are useful for review. The description is complete and specific.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked the UTF-8 arm's new unconditional self.buffered = pending write (the old code gated it on !FLUSH): to_utf16_alloc_maybe_buffered::<_, true> only ever returns a leftover length of 0, so a flush still carries nothing into the next stream. The UTF-8 fatal-error-after-OOM-on-stream concern was also examined and ruled out, since buffered is left untouched on the allocation error paths by design.

Extended reasoning...

The change replaces an abort in TextDecoder's Latin1/UTF-8/UTF-16 allocation paths with catchable ERR_MEMORY_ALLOCATION_FAILED / ERR_STRING_TOO_LONG errors, adds fallible string-creation helpers in bindgen.rs/JSString.rs/bindings.cpp, and reorders when stream state is committed; it touches no auth, injection, or data-exposure surface. Three inline findings (utf-16 error-code inconsistency at the 2^31 boundary, a measured utf-16 short-input regression, and stale lead_surrogate state after a fatal utf-16 error) already signal that a human look is needed, so this note only records what else was checked.

Comment thread src/runtime/webcore/TextDecoder.rs Resolved
Comment thread src/runtime/webcore/TextDecoder.rs Resolved
Comment thread src/runtime/webcore/TextDecoder.rs Resolved

Copy link
Copy Markdown
Member Author

The three failures in build 134770 so far are not from this change. All are in test/aot, on the test-aot-drawn steps:

test also failing now on
perf/generated-code-of-idioms-* 39 other open PRs, e.g. #47276, #47273, #47262
perf/generated-code-of-operations-* 23 other open PRs, e.g. #47276, #47271, #47264
generated-births-and-uses-of-one-use-of-two-kinds-of-value-* #47275, #47252, #47236, #47216, #47079, #47076

This diff touches nothing under src/aot or test/aot, no bindgen schema, and nothing there refers to the binding it renames. The two perf/ tests compare instruction counts of compiled programs with pinned numbers, and those pins were last written in #45631, before the WebKit upgrade in #47125.

There is no fix to bring in: none of the commits on main since this branch was cut touches those tests, and #47129 comes with a WebKit upgrade of its own. I cannot re-run BuildKite jobs from here.


Generated by Claude Code

claude and others added 2 commits October 29, 2026 03:06
…cii-alloc-bug-wm16yt

Co-authored-by: dylanc <dylanc@anthropic.com>
…cii-alloc-bug-wm16yt

Co-authored-by: dylanc <dylanc@anthropic.com>

Copy link
Copy Markdown
Member Author

Build 134878 is red only because its eight macOS test-bun jobs (4 aarch64, 4 x64) expired: they waited 83 minutes in the test-darwin queue, were never given an agent and never started. They have been retried and are queued again.

All 180 jobs that ran passed: Linux glibc and musl on x64 and aarch64, ASAN, hardened, cargo-test, and Windows on x64 and aarch64. That includes the new ulimit -v tests on aarch64 and musl. The three test/aot failures of the previous build are gone now that main (#47129, #47299) is merged in.


Generated by Claude Code

@dylan-conway
dylan-conway merged commit 69f4077 into main Oct 29, 2026
10 checks passed
@dylan-conway
dylan-conway deleted the claude/textdecoder-ascii-alloc-bug-wm16yt branch October 29, 2026 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants