Bun.wrapAnsi: throw a RangeError instead of aborting when a row or the row list cannot grow - #42833
Conversation
… grow Bun.wrapAnsi() keeps one Row per wrapped line in a WTF::Vector and each Row's text in another. Vector::append() calls CRASH() when the buffer cannot grow past INT32_MAX bytes, and the input sizes both: 128 MB of one-letter words at columns 1 is 2^26 rows of 32 bytes, and one UTF-16 row of 900 million characters passes the bound too. Every append now goes through tryAppend() and Bun::maxVectorSize<T>(), and a failure travels up to the binding, which throws RangeError: Out of memory. trimRowTrailingSpaces() compacts the row in place, so it no longer builds a second Vector for the tail.
|
Warning Review limit reached
On-demand reviews are free for the next 5 days. After that, they cost $0.25 per reviewed file. Or wait 7 minutes for your next included review. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (2)
Comment |
|
Updated 6:35 PM PT - Sep 15th, 2026
❌ @robobun, your commit efdb09f has 1 failures in
🧪 To try this PR locally: bunx bun-pr 42833That installs a local version of the PR into your bun-42833 --bun |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 traced the in-place rewrite of trimRowTrailingSpaces (src/jsc/bindings/wrapAnsi.cpp:307-323): write starts at lastVisibleEnd alongside it and advances by at most as much per iteration, so it never passes it, memmove covers the overlapping escape copy, and the final shrink(write - data) matches the old shrink + appendVector length. The new OOM throw in jsFunctionBunWrapAnsi runs under the existing DECLARE_THROW_SCOPE at :730, so it is not a missing-scope issue.
Extended reasoning...
Findings were reported inline, so this note only records what else was examined. I read the full C++ diff: every Vector::append site in the file now goes through Row::append/appendRow and propagates false up through wrapWord, the placeWord lambda, processLine, and wrapAnsiImpl to the host function. The in-place trimRowTrailingSpaces compaction was checked for overlap and final-length correctness (write cursor never overtakes the read cursor; memmove used for multi-char escape moves; self-assignment on the no-gap path is benign). The throw path uses the throw scope already declared in the host function and returns {}. Nothing here contradicts the inline findings; the residual StringBuilder abort and the boundary/comment nits stand as posted.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟣
src/jsc/bindings/wrapAnsi.cpp— Callers wrapping a UTF-16 input of roughly 976 million characters or more still get a process abort with exit 134 after this merges, although the title promises a catchable RangeError for growth failures. The row Vectors now fail soft, butjoinRowsWithAnsiPreservationat src/jsc/bindings/wrapAnsi.cpp:711 andresult.toString()at :720 append into a StringBuilder that CRASH()es on overflow and is never checked. Fix: checkresult.hasOverflowed()after each join and return nullopt from wrapAnsiImpl, so every growth failure in the function reaches the throw at :792, or record the exclusion with a todo test.Extended reasoning...
wrapAnsiImpl at :681-682 reserves input.size()*1.1 in an 8-bit StringBuilder. The first 16-bit append inside joinRowsWithAnsiPreservation (:711) reallocates to a 16-bit buffer, doubling bytes, and StringBuilder's non-overflow-recording mode calls CRASH() when the capacity passes the string limit. The PR description names this case explicitly and defers it to #42225. On the base the same input aborts at the same site, so the dismissal is factually right that the diff does not widen it. Why it still ranks: the merged behaviour is a partial contract. The host function at :791-794 throws only for nullopt, and nullopt is produced only by processLine. A user reading the release note tries
Bun.wrapAnsi(bigUtf16, 2**31)in try/catch and still loses the process. Population: anyone wrapping very large non-Latin-1 text (CJK logs) in a long-running server. Rate: once per such call. Remedy: switch the builder to OverflowHandler::RecordOverflow, check hasOverflowed() after :711 and before :720, and return std::nullopt.Verification: pre-existing. Trigger: a UTF-16 input large enough that the output StringBuilder must grow past the string length limit (PR says ~976M UTF-16 chars). Mechanism verified:
/home/claude/bun/src/jsc/bindings/wrapAnsi.cpp:681-682builds a defaultStringBuilder result; result.reserveCapacity(input.size() + input.size() / 10);(CrashOnOverflow mode, nohasOverflowed()check anywhere in the file —…
|
Reply to the review above.
|
There was a problem hiding this comment.
LGTM — the latest push adds the at-bound cases and trims the comments, and the fallible-append threading is consistent across every Row/row-list growth site.
What was reviewed:
- Every
.append/appendVectoronRow::m_dataandrowsinsrc/jsc/bindings/wrapAnsi.cppnow goes through[[nodiscard]]bool paths and each call site checks the result; the only raw appends left are on the outputStringBuilder, which the PR explicitly leaves to #42225. trimRowTrailingSpacesin-place compaction:writestarts equal toitand advances by at most whatitadvances, soconsumeANSIonly reads bytes not yet overwritten;memmovehandles the overlap.throwOutOfMemoryErroris under the existingThrowScopeand returns{}; the moved-outlastRowin the fold-back path is dropped by RAII on failure andlineRowsis discarded beforenulloptpropagates.- Tests are hermetic (
-echild,BUN_FEATURE_FLAG_SYNTHETIC_MEMORY_LIMITis read insrc/jsc/VirtualMachine.rs), drain pipes concurrently, and pin the exact 2048-row / 65,536 / 32,768-char bounds on both sides.
Extended reasoning...
Overview
The PR touches one native file, src/jsc/bindings/wrapAnsi.cpp, and its test file. It converts every growth of a Row<Char>::m_data Vector or the per-line Vector<Row<Char>> from append (which calls CRASH() on capacity overflow) into tryAppend guarded by Bun::maxVectorSize<T>(), threads a bool return through wrapWord, processLine and the placeWord lambda, changes wrapAnsiImpl to return std::optional<WTF::String>, and throws a RangeError: Out of memory from the host function on nullopt. trimRowTrailingSpaces is rewritten to compact in place instead of copying the tail into a second Vector. Since my prior review, commit efdb09f added at-bound cases (2048 rows / 65,536 Latin-1 / 32,768 UTF-16 chars fit, one more throws) and cut the describe-block comment to the single load-bearing fact, which addresses both of my earlier nits.
Security risks
None specific to this change. The only user-controlled input is the string and columns; the change turns a process abort on very large inputs into a catchable RangeError, which is strictly an improvement for DoS resistance. The size arithmetic m_data.size() + chars.size() <= maxVectorSize<Char>() cannot overflow because both operands are bounded by UINT_MAX >> 1. The in-place compaction in trimRowTrailingSpaces never reads past end and never writes ahead of its read cursor.
Level of scrutiny
Moderate. It is native C++ with a refactor of a buffer-manipulating routine, so I read the compaction loop carefully rather than trusting the description: write begins at lastVisibleEnd == it, escape sequences advance both by seqLen, spaces advance only it, so write <= it holds throughout and consumeANSI(it, end) only inspects bytes not yet overwritten; memmove is the correct primitive for the overlapping case. The bool threading is mechanical and I confirmed every call site consumes the result ([[nodiscard]] would also fail the build otherwise). The lastRowWidthDirty = true reorder before return wrapWord(...) is behavior-preserving since wrapWord does not read that flag. Both branches of the host function (is8Bit and UTF-16) go through the same optional check, and the error is thrown under the existing ThrowScope.
Other factors
The bug hunt ran dry with no findings, no CODEOWNER covers these paths, and no third-party objections are outstanding. The tests follow harness conventions (bunEnv spread, test.concurrent, Promise.all over stdout/stderr/exited, combined object assertion, exact error string), the env knob they rely on is read in src/jsc/VirtualMachine.rs and already used by other suites, and the repeat() sizes are small enough not to be slow in debug builds. The remaining StringBuilder appends are explicitly scoped out to a separate PR, which is a reasonable boundary.
…tor cannot grow (#42929) ### Problem - `Bun.sliceAnsi` and `Bun.stripANSI` each abort on a legal input: `panic(main thread): abort() called`, exit 134. Fuzzing found both, with no user report. - `sliceAnsi` keeps one 24-byte `Vector` entry (`pending`, `src/jsc/bindings/sliceAnsi.cpp:1257`) for each escape sequence that waits for the next visible character. `Vector::append` calls `CRASH()` at the 77,731,543rd: 78 MB of lone `\x9c` bytes. - `stripANSI` writes into a `Vector<Char>` as long as the input (`src/jsc/bindings/stripANSI.cpp:48`). A 16-bit string of 2^30 code units passes `INT32_MAX` bytes, so `Vector::grow` calls `CRASH()`. Bun 1.3.12 returns it (Notes). ### Fix - `sliceAnsi`: both lists grow with `tryAppend` behind `Bun::maxVectorSize<T>()`. On failure the binding throws `RangeError: Out of memory`, as #42833 and #42649 do. - `stripANSI`: the buffer is a `String` from `String::tryCreateUninitialized`, which holds every legal string, so the 2^30 input returns again. A failed allocation throws the same `RangeError`. - Outputs that fit do not change: 1.8 million seeded random calls match. The `String` buffer adds no copy (Notes). - Verified: `test/js/bun/util/sliceAnsi.test.ts` (1 new test), `test/js/bun/util/stripANSI.test.ts` (2 new tests). All 3 fail without the fix. The other ANSI suites pass (Notes). ### Background - `WTF::Vector<T>` holds `INT32_MAX / sizeof(T)` elements and grows by half. Past that, `append` and `grow` call `CRASH()` and `tryAppend` returns false. - A `WTF::String` holds 2^31 - 1 characters, 4 GiB when 16-bit. - A sequence waits in `pending` until the next visible character shows whether it is inside a grapheme cluster or past the range. - `Bun::maxVectorSize<T>()` is the `Vector` bound, lowered by `Bun__stringSyntheticAllocationLimit`. The tests set that to 64 KiB in a child. <details><summary>Notes</summary> **Repros, release builds on Linux x64: `main` against this branch** ```js // 1. Before: exit 134 at 4.7 GB peak. After: RangeError in 3.3 s, 4.6 GB. Bun.sliceAnsi("a" + "\x9c".repeat(77731543) + "b", 0, 2); // 77,731,542 sequences return the whole string before and after. "\x1b[m" gives the same counts (233 MB, 5.4 GB). // 2. Before: exit 134 after 2.3 s, 4.3 GB peak. After: returns 2^30 characters in 5.7 s, 8.4 GB peak. Bun.stripANSI("\x1b[31m" + "\u3042".repeat(2 ** 30)); ``` - In repro 2 the input takes 4 GiB (the repeated string and the flat copy of the rope). The buffer and the result take 2 GiB each. **Where each number comes from** - A `Pending` entry is 24 bytes, so the largest legal capacity is 89,478,485. `FastMalloc::nextCapacity` grows a full `Vector` from 77,731,542 to 116,597,313, which passes it. `tryAppend` refuses at the same step. `maxVectorSize` binds only when a test lowers the limit. #42833 has the same property. - `pendingHl` has one 24-byte tuple for each waiting hyperlink. It is never longer than `pending`, so the `pending` check covers its bound. Its `tryAppend` covers a failed allocation. - `SgrStyleState::entries` stays as it is. It holds one entry for each attribute slot, and `parseSgrParams` clamps a parameter below 1,000,000. **When stripANSI started to abort** - #28767 (in 1.3.12) replaced the `StringBuilder` with `Vector<Char>::grow(input.size())`. At that time `isValidCapacityForVector` accepted `UINT_MAX / sizeof(T)` elements, so the `Vector` held every legal string. - The WebKit upgrade #29161 (in 1.3.13) halved that bound to `(UINT_MAX >> 1) / sizeof(T)`. Bun 1.3.12 returns repro 2 and Bun 1.3.13 aborts. - `sliceAnsi` has had `pending` since #26963 added the function. The old bound only moved its abort to a higher count, so that half was never correct. No issue exists for either, so the tests are in the module test files. **The String buffer adds no copy** - `StringImpl::adopt(Vector&&)` moves the buffer only when the `Vector` allocator is `StringImplMalloc` (`wtf/text/StringImpl.h`). That is `FastCompactMalloc`, and a plain `Vector<Char>` uses `FastMalloc`, so it takes the `create(vector.span())` branch and copies. - The old path was `fastMalloc`, a `fastRealloc` in `shrinkToFit()` when the output was under half of the input, then that copy. The new path is `tryCreateUninitialized` for the buffer, then a copy into a second `tryCreateUninitialized` string of the exact length. Both allocations are fallible, so a failure of the second one also throws. - I also measured `StringImpl::tryReallocate` in place of the copy. It saves the copy for a large input with few sequences, and it keeps up to a third of slack on a small result. The copy keeps the memory of each result as it is today, so this PR uses the copy. **Benchmark of stripANSI**: release builds of `main` (b841a68) and of this change on it, LLVM 21, minimum of 6 interleaved runs. The runs are from before a293e34, which replaced `String(span)` with the same allocation and copy in a fallible form. | input (characters in, out) | `main` | this PR | |---|---|---| | SGR, 8-bit (19, 9) | 90.5 ns | 73.7 ns | | SGR, 16-bit (19, 9) | 87.2 ns | 69.4 ns | | hyperlink (44, 9) | 87.6 ns | 71.6 ns | | mixed SGR (69, 41) | 131.9 ns | 122.6 ns | | one escape (1,005, 1,000) | 286.1 ns | 258.5 ns | | one escape (64,005, 64,000) | 7.98 us | 7.72 us | | dense SGR (5,600, 1,600) | 14.22 us | 14.30 us | | dense SGR (212,992, 49,152) | 404.6 us | 408.9 us | | dense SGR, 16-bit (212,992, 49,152) | 421.2 us | 408.2 us | | no escape (16,384) | 175.5 ns | 176.1 ns | - A `sliceAnsi` benchmark (dense SGR, hyperlinks, 100 waiting sequences between characters) is within 2% of `main`. **No change for outputs that fit** - A seeded generator builds inputs from SGR, OSC 8, C1, unterminated and malformed sequences, control bytes, wide, zero-width, combining and surrogate characters. It calls `stripANSI` once and `sliceAnsi` three times for each input, with random ranges and ellipses. - A hash of 1.8 million results is equal on the two release builds. The debug ASAN build of this branch gives the same hash for the first 600,000. **The tests** - Two tests run a child with `BUN_FEATURE_FLAG_SYNTHETIC_MEMORY_LIMIT=65536`, the knob the #42649 and #42833 tests use. The bounds become 2730 waiting sequences and a buffer of 65,536 characters. - Each bound has an input that sits exactly at it and returns, and the same input one element longer, which throws: SGR sequences (Latin-1 and UTF-16), lone C1 ST bytes, hyperlinks, and the `stripANSI` buffer (Latin-1 and UTF-16). More cases: sequences at the end of the input, runs under the bound with visible characters between them, sequences before the range, and an input over the limit with nothing to strip. - Without the fix every input returns its normal length, so the failure is an assertion diff and not an abort. - The third test runs `stripANSI` at the real size: a 16-bit string of 1,073,741,826 code units. Every code unit is inside an `ESC ( x` sequence, so the output is empty and the child never writes to the buffer. It needs 2.2 GB (2.4 GB and 6 s in a debug ASAN build), and it skips under 8 GiB of memory, as `utf8-conversion-limit.test.ts` does. Without the fix the child exits with SIGABRT. - No test runs `sliceAnsi` at its real size (4.6 GB). - Other suites run on the debug ASAN build: `sliceAnsi-fuzz`, `wrapAnsi`, `wrapAnsi.npm`, `stringWidth`, and the rest of `sliceAnsi` and `stripANSI`. **Not changed here: `Bun.sliceAnsi` aborts that are not a `Vector`** #42931 tracks them. All five exit 134 on a release build of this branch. - The result fits, and a `StringBuilder` doubles its capacity past the longest 16-bit string. oven-sh/WebKit#631 (open) fixes the builder. - `Bun.sliceAnsi("\u3042".repeat(2 ** 30), 0, 4)`: `result.reserveCapacity(input.size())` (`sliceAnsi.cpp:876`) reserves 2^30 8-bit characters, and the first 16-bit append converts the buffer. 2.2 GB, for a result of 2 characters. - `Bun.sliceAnsi("\x1b[31m" + "a".repeat(2 ** 30), 0, 4, "\u2026")`: the same, and the ellipsis is the first 16-bit append. 2.2 GB, for a result of 14 characters. - `Bun.sliceAnsi("\x1b]8;;" + "\u3042".repeat(2 ** 30) + "\x07x", 0, 1)`: the builder of one hyperlink in `parseHyperlink` (`sliceAnsi.cpp:572`). 8.6 GB. - The result passes the string length limit. This is the `sliceAnsi` sibling of #42225 (`wrapAnsi`, open). - `Bun.sliceAnsi("\x1b[31m" + "a".repeat(2 ** 31 - 6), 1)`: `StringBuilder result`. 6.5 GB. - `Bun.sliceAnsi("a".repeat(2 ** 31 - 2), 1, undefined, "\u2026" + "\u200b".repeat(10))`: `makeString` on the ASCII fast path (`sliceAnsi.cpp:1400`). 2.2 GB. **Self-review** - Changes made after it: exact-bound test cases, the real-size `stripANSI` test, one-line comments, the list above, and the 1.3.12 to 1.3.13 account. - After the bot reviews: the exact-length copy in `stripANSI` is fallible too (a293e34). - Deferred: a `sliceAnsi` that needs no list (it scans the waiting run again when the next visible character arrives). That removes the throw. It touches the hot loop, so it is a PR of its own. - #42908 (open) edits other lines of `stripANSI.cpp`. Whichever lands second needs a small rebase. </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 0 · 4 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 3 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/bun/util/sliceAnsi.test.ts test/js/bun/util/stripANSI.test.ts bun test v1.4.3 (09bb546) test/js/bun/util/stripANSI.test.ts: (pass) Bun.stripANSI > returns same string object when no ANSI sequences present [171.84ms] (pass) Bun.stripANSI > returns new string when ANSI sequences are removed [2.47ms] (pass) Bun.stripANSI > "\u001b[31mred\u001b[39m" [2.01ms] (pass) Bun.stripANSI > "\u001b[32mgreen\u001b[39m" [0.85ms] (pass) Bun.stripANSI > "\u001b[33myellow\u001b[39m" [0.29ms] (pass) Bun.stripANSI > "\u001b[34mblue\u001b[39m" [0.30ms] (pass) Bun.stripANSI > "\u001b[35mmagenta\u001b[39m" [0.37ms] (pass) Bun.stripANSI > "\u001b[36mcyan\u001b[39m" [0.26ms] (pass) Bun.stripANSI > "\u001b[37mwhite\u001b[39m" [0.28ms] (pass) Bun.stripANSI > "\u001b[41mred background\u001b[49m" [0.30ms] (pass) Bun.stripANSI > "\u001b[42mgreen background\u001b[49m" [0.26ms] (pass) Bun.stripANSI > "\u001b[1mbold\u001b[22m" [0.26ms] (pass) Bun.stripANSI > "\u001b[2mdim\u001b[22m" [0.27ms] (pass) Bun.stripANSI > "\u001b[3mitalic\u001b[23m" [0.26ms] (pass) B ... (truncated) release without fix: all passed bun test v1.4.3-canary.1 (ee97718) test/js/bun/util/stripANSI.test.ts: (pass) Bun.stripANSI > returns same string object when no ANSI sequences present [2.25ms] (pass) Bun.stripANSI > returns new string when ANSI sequences are removed [0.05ms] (pass) Bun.stripANSI > "\u001b[31mred\u001b[39m" [0.02ms] (pass) Bun.stripANSI > "\u001b[32mgreen\u001b[39m" (pass) Bun.stripANSI > "\u001b[33myellow\u001b[39m" (pass) Bun.stripANSI > "\u001b[34mblue\u001b[39m" (pass) Bun.stripANSI > "\u001b[35mmagenta\u001b[39m" (pass) Bun.stripANSI > "\u001b[36mcyan\u001b[39m" (pass) Bun.stripANSI > "\u001b[37mwhite\u001b[39m" (pass) Bun.stripANSI > "\u001b[41mred background\u001b[49m" (pass) Bun.stripANSI > "\u001b[42mgreen background\u001b[49m" (pass) Bun.stripANSI > "\u001b[1mbold\u001b[22m" (pass) Bun.stripANSI > "\u001b[2mdim\u001b[22m" (pass) Bun.stripANSI > "\u001b[3mitalic\u001b[23m" (pass) Bun.stripANSI > "\u001b[4munderline\u001b[24m" (pass) Bun.stripANSI > "\u001b[5mblink\u001b[25m" (pass) Bun.stripANSI > "\u001b[7mreverse\u001b[27m" (pass) Bun.stripANSI > "\u001b[8mhidden\u001b[28m" (pass) Bun.stripANSI > "\u001b[9mstrikethrough\u001b[29m" (pass) Bun.stripANSI > "\u001b[38;5;1 ... (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/pr_gate.xml" test/js/bun/util/sliceAnsi.test.ts test/js/bun/util/stripANSI.test.ts bun test v1.4.3 (09bb546) test/js/bun/util/stripANSI.test.ts: (pass) Bun.stripANSI > returns same string object when no ANSI sequences present [188.15ms] (pass) Bun.stripANSI > returns new string when ANSI sequences are removed [2.74ms] (pass) Bun.stripANSI > "\u001b[31mred\u001b[39m" [2.55ms] (pass) Bun.stripANSI > "\u001b[32mgreen\u001b[39m" [0.48ms] (pass) Bun.stripANSI > "\u001b[33myellow\u001b[39m" [0.32ms] (pass) Bun.stripANSI > "\u001b[34mblue\u001b[39m" [0.28ms] (pass) Bun.stripANSI > "\u001b[35mmagenta\u001b[39m" [0.36ms] (pass) Bun.stripANSI > "\u001b[36mcyan\u001b[39m" [0.28ms] (pass) Bun.stripANSI > "\u001b[37mwhite\u001b[39m" [0.26ms] (pass) Bun.stripANSI > "\u001b[41mred background\u001b[49m" [0.30ms] (pass) Bun.stripANSI > "\u001b[42mgreen background\u001b[49m" [0.28ms] (pass) Bun.stripANSI > "\u001b[1mbold\u001b[22m" [0.62ms] (pass) Bun.stripANSI > "\u001b[2mdim\u001b[22m" [0.28ms] (pass) Bun.stripANSI > "\u001b[3mitalic\u001b[23m" [0.25ms] (pass) B ... (truncated) release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 709ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/8] gen cpp.rs (cppbind) [1/8] cargo bun_runtime → libbun_runtime.a �[1m�[33mwarning�[0m�[1m: binary `bun_shim_impl` should have a kebab-case name�[0m �[1m�[94m|�[0m �[1m�[94m 1�[0m �[1m�[94m|�[0m /workspace/bun/build/release/rust-target/.../bun_shim_impl �[1m�[94m|�[0m �[1m�[33m^^^^^^^^^^^^^�[0m �[1m�[94m|�[0m �[1m�[94m= �[0m�[1mnote�[0m: `cargo::non_kebab_case_bins` is set to `warn` by default �[1m�[96mhelp�[0m: to change the binary name to `bun-shim-impl`, convert `bin.name` �[1m�[94m--> �[0msrc/install/windows-shim/Cargo.toml:41:8 �[1m�[94m|�[0m �[1m�[94m41�[0m �[91m- �[0mname = �[91m"bun_shim_impl"�[0m �[1m�[94m41�[0m �[92m+ �[0mname = �[92m"bun-shim-impl"�[0m �[1m�[94m|�[0m �[1m�[33mwarning�[0m: `bun_shim_impl` (manifest) generated 1 warning �[1m�[33mwarning�[0m�[1m: `feature(generic_const_exprs)` is not supported with the next-generation trait solver�[0m �[1m�[94m--> �[0msrc/shell_parser/lib.rs:1:30 �[1m�[94m|�[0m �[1m�[94m1�[0m ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/jsc/bindings/sliceAnsi.cpp | 30 ++++++++------ src/jsc/bindings/stripANSI.cpp | 42 +++++++++++--------- test/js/bun/util/sliceAnsi.test.ts | 73 ++++++++++++++++++++++++++++++++++ test/js/bun/util/stripANSI.test.ts | 81 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 196 insertions(+), 30 deletions(-) ``` </details> **gate history** · 2 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/jsc/bindings/sliceAnsi.cpp 8 6 26 src/jsc/bindings/stripANSI.cpp 5 12 26 test/js/bun/util/sliceAnsi.test.ts 2 2 17 test/js/bun/util/stripANSI.test.ts 1 1 17 ``` </details> <!-- robobun:evidence:end -->
2 participants
Problem
Bun.wrapAnsi("a ".repeat(2 ** 26), 1)aborts:panic(main thread): abort() called, exit 134. Top frames:WTF::VectorBufferBase::allocateBuffer<FailureAction::Crash>(Vector.h:228) <-Vector::appendSlowCase<Bun::Row<unsigned char>>. The output (2^27 characters) is below the string length limit.processLinekeeps one 32-byteRowfor each wrapped row in aWTF::Vector(src/jsc/bindings/wrapAnsi.cpp:676).Vector::appendcallsCRASH()when it cannot grow: at the 51,821,029th row of one input line.tailcopy intrimRowTrailingSpaces.Fix
tryAppendbehindBun::maxVectorSize<T>(), as streams: throw instead of aborting when a script-sized container cannot grow #42649 does. A failure returnsfalseup to the binding, which throwsRangeError: Out of memory.trimRowTrailingSpacestrims in place, astrimLeadingSpacesdoes. It allocates nothing.test/js/bun/util/wrapAnsi.test.ts. Its 2 new tests hold 15 inputs that throw (10 of the 11 statements that grow a Vector) and 5 that return exactly at a bound. Both fail without the fix. Also thewrapAnsi.npm,sliceAnsi,stringWidth,stripANSIsuites.Background
WTF::Vector<T>holdsINT32_MAX / sizeof(T)elements and grows by half.appendcallsCRASH()when the next capacity passes that bound.tryAppendreturns false.Bun::maxVectorSize<T>()(VectorSizeLimit.h, from streams: throw instead of aborting when a script-sized container cannot grow #42649) is that bound, lowered byBun__stringSyntheticAllocationLimit. The tests set it to 64 KiB in a child, so the 2049th row reaches it.StringBuilderof the same function throw. It does not touch these Vectors.Notes
Repros, release builds on Linux x64: Bun 1.4.2 and canary 782c402 against this branch
Where each number comes from
sizeof(Row<Char>)is 32, so the largest legal capacity of the row list is 67,108,863.FastMalloc::nextCapacitygrows a full Vector from 51,821,028 to 77,731,542, which passes it.tryAppendrefuses at the same step, so the function now throws at the 51,821,029th row.maxVectorSizebinds only when a test lowers the limit. streams: throw instead of aborting when a script-sized container cannot grow #42649 has the same property.INT32_MAXbytes.trimRowTrailingSpacescopied the zero-width tail into a second Vector one character at a time, and that Vector failed to grow past 885,410,839 characters.A failed call
RangeErrorwith the same text comes from JSC when"a".repeat()runs out of memory.Not changed here
WTF::StringBuilderunderjoinRowsWithAnsiPreservation.wrapAnsiImplreserves 1.1 times the input length while the builder is still 8-bit, and the first 16-bit append doubles that reserve past the string limit. It is the output builder, which Throw ERR_STRING_TOO_LONG from Bun.wrapAnsi instead of aborting when the output passes the string length limit #42225 owns, so it is tracked apart from this PR.Bun.wrapAnsihas had these Vectors since Add Bun.wrapAnsi() for text wrapping with ANSI escape code preservation #26061 added it. This was never correct, so the test is in the module's test file.The tests
BUN_FEATURE_FLAG_SYNTHETIC_MEMORY_LIMIT=65536, the knob the streams: throw instead of aborting when a script-sized container cannot grow #42649 tests use. The bounds become 2048 rows, and 65,536 Latin-1 or 32,768 UTF-16 characters in a row.return falseconfirmed which statement fails in each case. Every one is covered except the first row of a line. That one fails only when the limit is below the 32 bytes of oneRow.No change for outputs that fit
columns1 to 10 and every option set. A hash of 1.9 million outputs is equal on the canary (09bb546 and 782c402) and on the debug ASAN build of this branch.hard.[human-review] gate passed · iteration 0 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 2 passed · 0 rejected · iteration 0
evidence per changed file