Copy the message string when constructing Error/AggregateError from a ZigString - #31451
Open
Jarred-Sumner wants to merge 2 commits into
Open
Copy the message string when constructing Error/AggregateError from a ZigString#31451Jarred-Sumner wants to merge 2 commits into
Jarred-Sumner wants to merge 2 commits into
Claude / Claude Code Review
completed
May 27, 2026 in 8m 36s
Code review found 1 important issue
Found 3 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 0 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/jsc/bindings/bindings.cpp:3462-3465 |
toStringCopy leaks external-tagged ZigString buffers |
Annotations
Check failure on line 3465 in src/jsc/bindings/bindings.cpp
claude / Claude Code Review
toStringCopy leaks external-tagged ZigString buffers
Switching to `toStringCopy` here drops the `isTaggedExternalPtr` (bit 62) ownership-transfer path that `toString` honored, so callers that `mark_global()` to hand the buffer to C++ now leak it. This is active for `createAggregateError`: `src/jsc/VirtualMachine.rs:2579-2591` heap-allocates the message via `bun_core::heap::release(...)`, calls `message.mark_global()`, and explicitly relies on C++ adopting it as an `ExternalStringImpl` freed via `free_global_string` — after this change the buffer i
Loading