ws client: cap close-reason transcode buffer at 125 to remove unsafe cast#30778
ws client: cap close-reason transcode buffer at 125 to remove unsafe cast#30778robobun wants to merge 9 commits into
Conversation
|
Updated 7:28 AM PT - May 21st, 2026
❌ @robobun, your commit bbf49b2 has 2 failures in
502 Bad Gatewaycloudflare 🧪 To try this PR locally: bunx bun-pr 30778That installs a local version of the PR into your bun-30778 --bun |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
WalkthroughThe PR bounds close-frame reason encoding to RFC limits, removes an unsafe buffer cast, adds a regression test exercising long Unicode close reasons, and JSON-stringifies the build-time OVERLAY_CSS value. ChangesWebSocket close frame payload safety
Build-time CSS serialization
Suggested reviewers:
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
robobun
force-pushed
the
farm/bfaf4a85/ws-close-reason-buf-overrun
branch
from
67ad0ca to
4cb9101
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/websocket/websocket-close-fragmented.test.ts`:
- Around line 148-160: The test currently discards child stderr ("stderr:
\"ignore\"") which drops crash diagnostics; change Bun.spawn so proc is created
with stderr: "pipe" (alongside stdout: "pipe"), await proc.stderr.text()
together with proc.stdout.text() and proc.exited, and update the assertion to
include stderr in the failure output; when asserting stderr is empty, filter out
the known ASAN startup warning coming from bunEnv before comparing so benign
ASAN messages don't fail the test.
- Around line 216-219: Replace the use of the string .repeat call in the
ws.close call: locate the ws.close(1000, "\\u0800".repeat(42)) invocation and
replace the repetitive-string construction with the repository pattern using
Buffer.alloc(...).toString() (i.e., build the repeated "\u0800" payload via
Buffer.alloc(count, fill).toString() rather than String.prototype.repeat) so the
test uses Buffer.alloc for creating the repetitive payload.
🪄 Autofix (Beta)
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: a9778c52-1a99-4cf3-a408-be0e12c1b2b4
📒 Files selected for processing (1)
test/js/web/websocket/websocket-close-fragmented.test.ts
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "-e", CLOSE_LONG_REASON_FIXTURE], | ||
| env: bunEnv, | ||
| stdout: "pipe", | ||
| stderr: "ignore", | ||
| }); | ||
| const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); | ||
| // With the fix: the fixture reaches the close listener, prints | ||
| // `close:1000`, and exits cleanly. Without the fix: the child aborts | ||
| // inside `WebSocket::close` before the listener fires — stdout is | ||
| // empty and exitCode is non-zero (SIGILL from the Rust panic). | ||
| expect({ stdout: stdout.trim(), exitCode }).toEqual({ | ||
| stdout: "close:1000", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Pipe the child stderr instead of discarding it.
This test is guarding a crash path; with stderr: "ignore", a regression only gives you stdout/exitCode and drops the most useful crash diagnostics.
♻️ Suggested change
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", CLOSE_LONG_REASON_FIXTURE],
env: bunEnv,
stdout: "pipe",
- stderr: "ignore",
+ stderr: "pipe",
});
- const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);
+ const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+ const cleanStderr = stderr
+ .split("\n")
+ .filter(line => !line.startsWith("WARNING: ASAN interferes"))
+ .join("\n")
+ .trim();
// With the fix: the fixture reaches the close listener, prints
// `close:1000`, and exits cleanly. Without the fix: the child aborts
// inside `WebSocket::close` before the listener fires — stdout is
// empty and exitCode is non-zero (SIGILL from the Rust panic).
- expect({ stdout: stdout.trim(), exitCode }).toEqual({
+ expect({ stdout: stdout.trim(), stderr: cleanStderr, exitCode }).toMatchObject({
stdout: "close:1000",
+ stderr: "",
exitCode: 0,
});Based on learnings, crash-prone spawned-process tests should keep subprocess output in the failure diff, and bunEnv-based tests should filter the known ASAN startup warning before asserting stderr is empty.
🤖 Prompt for 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.
In `@test/js/web/websocket/websocket-close-fragmented.test.ts` around lines 148 -
160, The test currently discards child stderr ("stderr: \"ignore\"") which drops
crash diagnostics; change Bun.spawn so proc is created with stderr: "pipe"
(alongside stdout: "pipe"), await proc.stderr.text() together with
proc.stdout.text() and proc.exited, and update the assertion to include stderr
in the failure output; when asserting stderr is empty, filter out the known ASAN
startup warning coming from bunEnv before comparing so benign ASAN messages
don't fail the test.
| // 42 code units × 3 UTF-8 bytes = 126 bytes — one byte past the | ||
| // 125-byte close-frame payload cap. C++ spec check bounds on UTF-16 | ||
| // code-unit count (42 < 123), so this reaches the native close path. | ||
| ws.close(1000, "\\u0800".repeat(42)); |
There was a problem hiding this comment.
Replace .repeat(42) with the repo’s Buffer.alloc(...).toString() pattern.
This fixture is building a repetitive string in a test, and this repo explicitly avoids .repeat() for that case.
♻️ Suggested change
- // 42 code units × 3 UTF-8 bytes = 126 bytes — one byte past the
+ // 126 UTF-8 bytes total — one byte past the
// 125-byte close-frame payload cap. C++ spec check bounds on UTF-16
// code-unit count (42 < 123), so this reaches the native close path.
- ws.close(1000, "\\u0800".repeat(42));
+ ws.close(1000, Buffer.alloc(126, "\\u0800").toString());As per coding guidelines, "To create repetitive strings in tests, use Buffer.alloc(count, fill).toString() instead of "string".repeat(count) for better performance in debug builds."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 42 code units × 3 UTF-8 bytes = 126 bytes — one byte past the | |
| // 125-byte close-frame payload cap. C++ spec check bounds on UTF-16 | |
| // code-unit count (42 < 123), so this reaches the native close path. | |
| ws.close(1000, "\\u0800".repeat(42)); | |
| // 126 UTF-8 bytes total — one byte past the | |
| // 125-byte close-frame payload cap. C++ spec check bounds on UTF-16 | |
| // code-unit count (42 < 123), so this reaches the native close path. | |
| ws.close(1000, Buffer.alloc(126, "\\u0800").toString()); |
🤖 Prompt for 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.
In `@test/js/web/websocket/websocket-close-fragmented.test.ts` around lines 216 -
219, Replace the use of the string .repeat call in the ws.close call: locate the
ws.close(1000, "\\u0800".repeat(42)) invocation and replace the
repetitive-string construction with the repository pattern using
Buffer.alloc(...).toString() (i.e., build the repeated "\u0800" payload via
Buffer.alloc(count, fill).toString() rather than String.prototype.repeat) so the
test uses Buffer.alloc for creating the repetitive payload.
There was a problem hiding this comment.
Thanks — 5a4e9b4 resolves the prettier-reflow nit; all my earlier comments are now addressed. Deferring to a human since this also bundles an unrelated bake-codegen.ts OVERLAY_CSS behavioral change and overlaps with #30722.
Extended reasoning...
Overview
The headline change is a targeted fix in src/http_jsc/websocket_client.rs: the close-reason transcode buffer is now [u8; 125] (matching send_close_with_body's Option<&mut [u8; 125]> signature), the cursor is built over a 123-byte view (MAX_REASON_BYTES) so overflow bails to the no-reason path at the RFC 6455 boundary, and the unsafe cast::<[u8; 125]>() is gone. A regression test in websocket-close-fragmented.test.ts spawns a child that calls ws.close(1000, '\\u0800'.repeat(42)) and asserts clean exit. The remaining 11 files are autofix.ci/rustfmt reflows of multi-target #[cfg(...)] / #[cfg_attr(...)] attributes (whitespace-only), a jsc_hooks.rs line-wrap, and one substantive one-liner in src/codegen/bake-codegen.ts wrapping OVERLAY_CSS in JSON.stringify(...).
Security risks
The fix removes UB (out-of-bounds slice via a narrowed array reference) and a process-aborting panic across extern "C" reachable from JS WebSocket.prototype.close. The 123-byte cap prevents emitting close frames with header-length 126/127 (the §5.2 extended-length sentinels). I don't see new attack surface introduced; the test fixture binds to 127.0.0.1:0. No auth/crypto/permissions code is touched.
Level of scrutiny
Moderate. The core change is small and well-reasoned, and three rounds of review feedback (off-by-2 on the 125 vs 123 cap, test-convention nits, prettier comment reflow) have all been incorporated. However:
- It rewrites RFC 6455 close-frame payload bounds in a hand-rolled protocol implementation — an area where off-by-one errors have wire-visible consequences.
- It bundles an unrelated build-system behavioral change (
bake-codegen.tsnowJSON.stringifys the CSS define) that isn't covered by the PR title/description. - robobun reports CI failures on Build #54713 (at f6cd1cc); 5a4e9b4 was pushed after that comment and CI status for it isn't reflected yet.
- github-actions flagged #30722 as fixing the same WebSocket overflow via a different approach (runtime clamp vs buffer shrink) — a human should pick one.
Other factors
All three of my prior inline threads are resolved by the current diff (the last via 5a4e9b4). The rustfmt attribute reflows are mechanical and zero-risk. The new test isolates the crash in a subprocess so a regression won't take down the runner, and asserts on stdout + exitCode per repo conventions. Given the bundled unrelated change, the open CI status, and the duplicate-PR coordination needed, this isn't a "merge without a human glance" PR.
|
CI red, but the failures are on |
robobun
force-pushed
the
farm/bfaf4a85/ws-close-reason-buf-overrun
branch
from
4db0921 to
8041ffc
Compare
|
Rebased onto main. Conflict in
Release bun on main now includes #30679's JSON lexer fix, so the |
| // `send_close_with_body` takes `&mut [u8; 125]`; keep the array at | ||
| // 125 bytes so the borrow covers the full provenance and matches | ||
| // the other caller (line 1006, server-initiated close echo). |
There was a problem hiding this comment.
🟡 nit: the new comment references "the other caller (line 1006, server-initiated close echo)" — hardcoded line numbers in comments rot as soon as anything above them is edited in this ~2000-line file. The semantic anchor ("server-initiated close echo") is already enough to find the call site; suggest dropping "line 1006, ".
Extended reasoning...
What the issue is
The comment added at src/http_jsc/websocket_client.rs:1635 reads:
// the other caller (line 1006, server-initiated close echo).
This embeds a hardcoded source line number as a cross-reference. In a ~2000-line file, any edit above line 1006 — a new import, a doc comment, a refactor of an earlier method — shifts the target, and the comment silently goes stale. The comment is being added fresh in this PR, so it's the right time to fix it before it lands.
Why the line number adds no durable value
The comment already includes a stable semantic anchor: "server-initiated close echo". A reader can grep for send_close_with_body (there are exactly two callers) or for "close echo" and find the other call site regardless of where it has drifted to. The line number is supplementary, not the primary anchor — which means when it drifts it becomes pure noise: a reader who jumps to line 1006 and finds unrelated code has to fall back on the semantic description anyway.
Step-by-step proof of rot
- Today: the other
send_close_with_bodycall is at line 1006 (verified). - A future PR adds a 10-line helper near the top of
impl<const SSL: bool> WebSocket<SSL>. - The server-initiated close echo is now at line 1016; the comment still says "line 1006".
- A reader following the comment lands in the middle of an unrelated function body.
- The reader has to grep for "server-initiated close echo" /
send_close_with_bodyanyway — exactly what they'd have done if the line number weren't there.
Addressing the "too trivial" objection
One reviewer noted the semantic anchor mitigates the rot, so the line number is merely belt-and-suspenders. That's true, and is why this is a nit, not a blocker. But the comment is brand-new in this PR — the marginal cost of dropping four characters now is zero, vs. leaving a known-to-rot reference in a freshly-authored comment. Since the PR is already touching this exact line, it's the cheapest possible time to address it.
Impact
Zero runtime impact. Pure comment maintainability.
Fix
Drop the line number, keep the semantic referent:
// the other caller (server-initiated close echo).
|
CI red again on same pattern as before: Looking at recent builds, ASAN shard 17 has failed with exit -1 on multiple unrelated builds (54920, 55200, 55217) — it's a known flaky infra lane, not specific to this PR. Different agents each time ( Diff is green on every lane that runs to completion, including the ASAN job on shards 0-16, 18, 19. Re-roll already used. Needs a maintainer to re-run the red shard or merge. |
…cast WebSocket<SSL>::close allocated a 128-byte `close_reason_buf` on the stack, transcoded the reason into it (UTF-16→UTF-8 via `to_owned_slice` or Latin-1→UTF-8 via `copy_latin1_into_utf8`), then pointer-cast the buffer to `&mut [u8; 125]` and passed it to `send_close_with_body` with `body_len = cursor.position()`. The cursor position could reach 128, and the downstream `&data[..body_len]` slice overruns the 125-byte array reference. A 42-code-unit UTF-16 reason of U+0800 passes the C++ spec check (`reason.length() > 123`) but transcodes to 126 UTF-8 bytes, panicking the runtime with `range end index 126 out of range for slice of length 125`. The panic crosses the `extern "C" fn close` boundary and aborts the process. Shrink the transcode buffer to 125 bytes so: - `cursor.write_all` fails with WriteZero when the transcoded UTF-8 exceeds 125 bytes (same `break 'inner` path the Latin-1 arm already takes on overflow), falling through to the no-reason `close` frame. - The fixed-size array is passed directly to `send_close_with_body`, eliminating the unsafe pointer cast. Regression test: send `close(1000, '\u0800'.repeat(42))` to a raw handshake server and wait for the close event without the runtime panicking. Fixes #30771
The shipped `build/release/bun` binary in the tree pre-dates #30679's JSON lexer fix, so when it runs `bake-codegen.ts` during `bun bd` or `bun run build:release` the old lexer aborts on the leading `*` of the minified CSS `define` value: *{box-sizing:border-box;...} ^ error: Unsupported syntax: Operators are not allowed in JSON Main is otherwise unbuildable with the shipped release bun. Wrap OVERLAY_CSS in `JSON.stringify` — matches the canonical `Bun.build` `define` pattern and parses cleanly under every JSON lexer generation.
… frame Follow-up to the buffer-overrun fix: the 125-byte transcode buffer let a reason of 124 or 125 UTF-8 bytes through, and `send_close_with_body` then wrote header-length 126/127 — the RFC 6455 §5.2 extended-length sentinels. Control frames have no extended length encoding, so the receiver misparses the mask key as length and rejects the frame. Keep the array at 125 bytes (to match the `Option<&mut [u8; 125]>` signature on the other caller) but build the cursor over a 123-byte view so `WriteZero` / `NoSpaceLeft` bail at the correct boundary. Also address review nits on the regression test: drop the `panicked` stderr-substring assertion (the stdout + exitCode checks already cover the regression in two independent ways; the substring check is exactly the pattern CLAUDE.md warns against) and drop the per-test timeout (covered by the runner default).
autofix.ci kept collapsing three `//` comments wedged between the test arrow's `}` and the `30_000` timeout onto the `},` line in reverse reading order, and after three repeated bot commits the Format job gave up with 'refusing to patch repeated bot commits'. Move the comment to the line above `test(...)` where prettier won't touch it, and let prettier collapse the call back to its preferred single-line form.
robobun
force-pushed
the
farm/bfaf4a85/ws-close-reason-buf-overrun
branch
from
8041ffc to
bbf49b2
Compare
|
Rebased onto main. Conflict in Kept this PR's version: Rust tests still pass: |
|
Closing: #30771 is already fixed on main via #30722 (runtime This PR's contribution is now purely stylistic (buffer sized to match the Happy to leave open if the cleanup is wanted; otherwise please close. |
| // RFC 6455 §5.5.1: the close-frame payload is ≤ 125 bytes total and | ||
| // the 7-bit header length field is the payload length — there is no | ||
| // extended-length encoding for control frames. `send_close_with_body` | ||
| // prepends the 2-byte status code separately and computes the header | ||
| // length as `(body_len + 2) & 0x7F`. A reason of 124 or 125 UTF-8 | ||
| // bytes therefore writes a header length of 126 or 127, which are | ||
| // the 16/64-bit extended-length sentinels per §5.2 and malform the | ||
| // frame. Cap the transcode cursor at 123 bytes so the overflow arms | ||
| // below (`WriteZero` / `NoSpaceLeft`) bail via `break 'inner` at the | ||
| // correct boundary. | ||
| const MAX_REASON_BYTES: usize = 123; |
There was a problem hiding this comment.
🟡 The new comment justifying MAX_REASON_BYTES = 123 says send_close_with_body "computes the header length as (body_len + 2) & 0x7F" and that a 124/125-byte reason "writes a header length of 126 or 127 … and malform[s] the frame" — but after the 2026-05-21 rebase, send_close_with_body does let body_len = body_len.min(123); (line 1346) before computing the header length, so that scenario can no longer occur. The 123-byte cursor cap is still worthwhile — it routes overflow to a clean no-reason close instead of letting .min(123) truncate mid-UTF-8-sequence and trip terminate(InvalidUtf8) at line 1379 — but the comment's stated rationale is now factually wrong and should be updated (or trimmed, per src/CLAUDE.md's "avoid adding comments where not necessary").
Extended reasoning...
What the issue is
The comment block added at src/http_jsc/websocket_client.rs:1665–1675 justifies MAX_REASON_BYTES = 123 like this:
send_close_with_bodyprepends the 2-byte status code separately and computes the header length as(body_len + 2) & 0x7F. A reason of 124 or 125 UTF-8 bytes therefore writes a header length of 126 or 127, which are the 16/64-bit extended-length sentinels per §5.2 and malform the frame.
That description of send_close_with_body is no longer accurate. The current implementation (lines 1342–1367, brought in via #31129 on main and untouched by this PR) reads:
fn send_close_with_body(&mut self, code: u16, body: Option<&mut [u8; 125]>, body_len: usize) {
// ... reason text is limited to 123 bytes.
let body_len = body_len.min(123); // ← line 1346
...
header.set_len(((body_len + 2) & 0x7F) as u8); // ← line 1367The .min(123) clamp runs before the header length is computed, so body_len + 2 is at most 125 and the 126/127 extended-length sentinels can never be emitted regardless of what the caller passes.
How the comment went stale
The rationale was correct when written. Per the PR timeline, the original 2026-05-15 review identified the 124/125-byte malformed-frame edge against the then-current send_close_with_body, which had no clamp; commit 0754956 added the 123-byte cursor cap and this comment to fix it. On 2026-05-21 the branch was rebased onto a main that now includes #31129's independent .min(123) defense inside send_close_with_body (the rebase note even says main has "another variation of the overflow fix"). The comment was carried forward unchanged and now describes callee behavior that no longer exists.
Step-by-step proof
Take the comment's own example — a reason that transcodes to 124 UTF-8 bytes — and suppose the cursor cap were absent so 124 reaches send_close_with_body:
send_close_with_body(code, Some(&mut close_reason_buf), 124)is called.- Line 1346:
let body_len = 124.min(123)→body_len = 123. - Line 1367:
header.set_len(((123 + 2) & 0x7F) as u8)→set_len(125). - 125 is a valid 7-bit literal length per RFC 6455 §5.2 — not 126 or 127, not an extended-length sentinel. The frame is well-formed.
So the comment's central claim ("writes a header length of 126 or 127 … and malform the frame") is unreachable on the post-rebase code.
Why the cursor cap still has value (the correct rationale)
Without MAX_REASON_BYTES, a 124-byte UTF-8 reason would reach send_close_with_body, get hard-truncated to 123 bytes by .min(123), and — if byte 123 falls mid-multibyte-sequence — fail is_valid_utf8 at line 1379, calling self.terminate(ErrorCode::InvalidUtf8) instead of sending any close frame. The cursor cap makes overflow take break 'inner and fall through to the clean send_close_with_body(code, None, 0) path. That is a real, useful behavioral difference — it's just not the malformed-frame story the comment tells.
Impact
Zero runtime impact — this is purely comment accuracy. But the comment is brand-new in this PR (not pre-existing prose), it makes a concrete present-tense claim about a specific function's behavior, and that claim is verifiably false against the file as it stands. A future reader auditing send_close_with_body will find the .min(123) clamp, see this comment asserting it doesn't exist, and have to spend time reconciling the two. src/CLAUDE.md says "avoid adding comments where not necessary"; an inaccurate 10-line comment is worse than none.
Fix
Either trim the comment to the load-bearing fact:
// RFC 6455 §5.5.1: close-frame payload is 2-byte status + ≤123-byte reason.
// Cap the cursor at 123 so overflow falls through to the no-reason close
// rather than reaching send_close_with_body's .min(123) truncation (which
// could split a UTF-8 sequence and trip terminate(InvalidUtf8)).
const MAX_REASON_BYTES: usize = 123;or drop the "computes the header length as (body_len + 2) & 0x7F … 126 or 127 … malform the frame" sentences entirely, since send_close_with_body now defends that boundary itself.
1 participant
Summary
Fixes #30771 —
WebSocket<SSL>::closeinsrc/http_jsc/websocket_client.rstranscodes the close reason into a 128-byte stack buffer and then pointer-casts the buffer to&mut [u8; 125]before callingsend_close_with_body(.., body_len = cursor.position()). When the transcoded UTF-8 lands in(125, 128], the downstream&data[..body_len]read on the 125-byte array reference panics at runtime — and the panic crosses theextern "C"boundary, aborting the process.Repro
A UTF-16 reason of 42 ×
U+0800(126 UTF-8 bytes) passes the C++reason.length() > 123spec check becauselength()is code units, not UTF-8 bytes:Runtime panic:
Same failure mode via the Latin-1 arm (
copy_latin1_into_utf8can transcode 64 × 0x80 → 128 UTF-8 bytes) and the UTF-8 arm (any 126-byte pre-tagged UTF-8 slice).Cause
Three transcoding arms in
close()each write intoclose_reason_buf: [u8; 128]:cursor.write_all(&to_owned_slice())— succeeds up to 128 bytes before returningWriteZero.cursor.write_all(str.slice())— same.copy_latin1_into_utf8(dst, str.slice())— returnsNoSpaceLeftonly when the buffer ran out before the source, so up to 128 bytes of output slips through on a 128-byte output.Then
close_reason_buf.as_mut_ptr().cast::<[u8; 125]>()is dereferenced as&mut [u8; 125]. The reference only carries 125 bytes of provenance;send_close_with_body's&data[..body_len]withbody_len > 125panics on the bounds check (and is UB under Stacked/Tree Borrows even without the panic).Fix
Shrink
close_reason_bufto[u8; 125]. The existing overflow handling in all three arms (break 'inner→ fall through to the no-reasonsend_close_with_body(code, None, 0)) now triggers exactly when the transcoded UTF-8 exceeds the RFC 6455 125-byte payload cap, matching the Zig original'swrote.ptr[0..125]intent without the unsafe cast. The array reference we hand tosend_close_with_bodynow covers the full 125-byte provenance, and the cast is gone.A
debug_assert!(wrote_len <= close_reason_buf.len())catches any regression that reintroduces the bug.Test
test/js/web/websocket/websocket-close-fragmented.test.tsgains'close() with reason that transcodes beyond 125 UTF-8 bytes does not crash': a raw TCP server completes the WebSocket handshake, the client callsclose(1000, '\u0800'.repeat(42)), and the test waits for'close'on the client.Before:
bun testaborts withrange end index 126 out of range for slice of length 125.After:
2 pass / 0 fail.