Conversation
When a client sends its first WebSocket frame(s) in the same TCP segment as the HTTP upgrade request, HttpContext::onData parsed the request head, adopted the socket into the WebSocket context, and returned without ever handing the remaining bytes in that read to the new context. The server answered 101, the connection stayed open, and the coalesced frames were silently dropped: message() never fired, a coalesced PING got no PONG, a coalesced malformed frame raised no protocol error. Fix: capture the bytes past the request head (which the parser already records on HttpRequest::head) and, after uncorking the 101 response, hand them to the adopted socket's on_data via us_dispatch_data. A bounds check on the captured pointer guards the parser's fallback-buffer path, where the HttpResponseData owning that buffer has already been destructed by upgrade().
WalkthroughThis PR preserves WebSocket bytes that arrive alongside an HTTP upgrade request, forwards them to the adopted socket, adds C linkage for ChangesCoalesced upgrade bytes forwarding
Sequence Diagram(s)sequenceDiagram
participant Client
participant HttpContext
participant WebSocket
Client->>HttpContext: send upgrade request + coalesced frame bytes in one write
HttpContext->>HttpContext: detect upgrade and record wsHead/wsHeadLen
HttpContext->>WebSocket: adopt socket, clear upgradedWebSocket
HttpContext->>WebSocket: us_dispatch_data(wsHead, wsHeadLen)
WebSocket->>Client: process frame(s)
Possibly related PRs
Suggested labels: websocket, bug-fix, tests Suggested reviewers: Jarred-Sumner, cirospaciari 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 7:09 PM PT - Jul 7th, 2026
✅ @robobun, your commit 187f35d86a880f5e3b77f525002b01fa105126cc passed in 🧪 To try this PR locally: bunx bun-pr 33692That installs a local version of the PR into your bun-33692 --bun |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I didn't find bugs, but this touches the core HttpContext::onData upgrade path with pointer-range checks against a buffer that may alias the parser's freed fallback storage, and it's the first C++ caller of us_dispatch_data — worth a human pass on the SSL path and the fallback-buffer edge case.
Extended reasoning...
Overview
The PR fixes Bun.serve dropping WebSocket frames that arrive in the same TCP read as the HTTP upgrade request. It changes packages/bun-uws/src/HttpContext.h (the onData hot path) to capture httpRequest->head when it points into the live read buffer and, after uncorking the 101 response, replay those bytes into the adopted socket via us_dispatch_data. packages/bun-usockets/src/internal/internal.h gains an extern "C" wrapper around the us_dispatch_* declarations (first C++ caller). A new 7-case test file exercises text/ping/multi/partial/close/malformed coalesced frames plus a control.
Security risks
Low but non-trivial. The pointer-range check (hd >= data && hd <= data+length) is the only guard against dispatching a span into the parser's fallback std::string, which upgrade() has already destroyed via ~HttpResponseData. The check is correct for the intended purpose, but pointer comparison across allocations is the kind of thing a maintainer should sign off on. No auth/crypto/permission surface is touched.
Level of scrutiny
High. HttpContext::onData is the per-read hot path for every Bun.serve HTTP request, and the upgrade branch now re-enters the dispatch layer (us_dispatch_data) with a buffer whose lifetime and provenance depend on parser internals (live read buffer vs. fallback). The change also reorders upgradedWebSocket = nullptr to before uncork/dispatch, and applies identically to the SSL=true instantiation where data is post-decrypt plaintext — both look right, but this is subtle enough that someone who owns uws/usockets should confirm the SSL routing and re-entrancy are safe.
Other factors
The fix is well-scoped and thoroughly tested for the non-SSL path; the PR description is precise about the fallback-buffer limitation. However, this is not a mechanical change — it introduces the first C++ call site for us_dispatch_data, relies on an implicit contract about where req->head can point, and leaves the split-head-across-reads case still dropping bytes (acknowledged, and better than UAF). Given the criticality of the code path and the memory-safety sensitivity, deferring to a human reviewer.
Exercises the HttpContext<true> instantiation: the SSL read path decrypts a TLS record into one plaintext dispatch, so the same bounds check and replay applies. 12/14 fail on main (both modes), 14/14 pass with the fix.
|
On the two points raised: SSL path. Added a TLS variant of every case in ed456ed ( Fallback-buffer edge case. The span's
|
![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 `@test/js/bun/websocket/websocket-server-upgrade-coalesced-frames.test.ts`:
- Around line 143-211: The websocket upgrade coalesced-frames tests are isolated
and I/O-bound, so they can run concurrently to reduce total runtime. Update each
affected `it(...)` in this suite to use concurrent execution, leveraging the
fact that `connectRaw` and the underlying `serve({ port: 0 })` setup do not
share state between cases. Keep the existing test logic and assertions
unchanged; only switch the test declarations to concurrent form.
🪄 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: bc404c35-bbd1-4be9-9504-3d00c4768ac2
📒 Files selected for processing (3)
packages/bun-usockets/src/internal/internal.hpackages/bun-uws/src/HttpContext.htest/js/bun/websocket/websocket-server-upgrade-coalesced-frames.test.ts
There was a problem hiding this comment.
The nit I raised is fixed and I didn't find further issues, but this touches HttpContext::onData — the core Bun.serve request path — with pointer-range reasoning around the parser's fallback buffer and a re-entrant us_dispatch_data into the adopted socket, so it's worth a human look from someone with uWS internals context.
Extended reasoning...
Overview
This PR fixes Bun.serve's WebSocket server dropping frames that arrive in the same TCP read as the HTTP upgrade request. Three files:
packages/bun-usockets/src/internal/internal.h— wraps theus_dispatch_*declarations inextern "C"so the first C++ caller links correctly. Mechanical.packages/bun-uws/src/HttpContext.h— inonData, when the request handler detectsupgradedWebSocket, capture the span of bytes past the request head (viahttpRequest->head, guarded by auintptr_tbounds check to exclude the parser's fallback buffer whichupgrade()has already destroyed). After uncorking the 101, feed those bytes to the adopted socket viaus_dispatch_data. Also movesupgradedWebSocket = nullptrahead of the uncork.- New test file with 7 scenarios × {TCP, TLS} = 14 cases (text frame, ping, multiple frames, partial frame, close, malformed, and a no-coalesced-bytes control).
My earlier inline nit (connect await outside the cleanup try) was addressed in 64ec35b and the thread is resolved.
Security risks
The change reads bytes from the same buffer onData was already handed and forwards them to the WebSocket parser that would have received them on the next read anyway, so no new attacker-controlled surface is introduced. The main risk is memory safety: httpRequest->head can point into the parser's fallback buffer, which is freed during upgrade(). The PR guards this with a uintptr_t range comparison against the live onData buffer and skips replay when the check fails. The author's explanation (span .data()/.empty() don't dereference; integer comparison avoids cross-allocation pointer-comparison UB) is sound, but this is exactly the kind of subtle lifetime reasoning a maintainer familiar with consumePostPadded's buffer management should confirm.
Level of scrutiny
High. HttpContext<SSL>::onData runs for every byte of every Bun.serve request — it is one of the hottest and most correctness-critical paths in the codebase. The change is small and well-scoped, but it (a) reorders upgradedWebSocket = nullptr relative to uncork(), (b) synchronously re-enters the loop dispatch layer via us_dispatch_data from inside an onData frame, and (c) relies on pointer-range checks to avoid a UAF. Each is reasoned about in the PR/comments, but together they exceed what I'm comfortable approving without a human who owns this code signing off.
Other factors
Test coverage is thorough (both SSL and non-SSL, partial frames, control frames, malformed frames, and a negative control), the bug-hunting pass found nothing, and all review threads are resolved. The CodeRabbit it.concurrent suggestion was reasonably declined. Deferring rather than approving purely on the basis of code-path criticality.
There was a problem hiding this comment.
No issues found, but this touches the HTTP→WebSocket upgrade path in vendored uWS with pointer-range checks and a re-entrant us_dispatch_data call that can run user JS — worth a human look at the buffer-lifetime and reentrancy reasoning.
Extended reasoning...
Overview
This PR fixes Bun.serve silently dropping WebSocket frames that arrive in the same TCP read as the HTTP upgrade request. It touches three files: packages/bun-usockets/src/internal/internal.h (wraps us_dispatch_* declarations in extern "C" for the first C++ caller), packages/bun-uws/src/HttpContext.h (captures bytes past the request head via httpRequest->head, bounds-checks the pointer against the live read buffer via uintptr_t comparison, then feeds them to the adopted socket via us_dispatch_data after uncorking the 101), and a new 14-case test file covering TCP+TLS with text/ping/multi/partial/close/malformed/control variants.
Security risks
No injection or auth surface. The pointer-range check (hd >= data && hd <= data + length) guards against reading the parser's freed fallback buffer; the author's follow-up comment explains the uintptr_t cast makes this plain integer comparison and the false branch leaves wsHeadLen = 0. The dispatched bytes come from the original read buffer, which is live until onData returns. Untrusted client bytes are handed to the existing WebSocket frame parser, which already handles adversarial input.
Level of scrutiny
High. This is vendored uWebSockets C++ on the hot Bun.serve upgrade path. The change involves: (1) buffer-lifetime reasoning across the HTTP parser's fallback-buffer edge case, (2) the first C++ call site for us_dispatch_data, which routes to WebSocketContext::onData and can synchronously run user message/close handlers (the return value is captured and is_closed/is_shut_down are checked first, but reentrancy in this layer is subtle), and (3) reordering upgradedWebSocket = nullptr ahead of uncork(). Each looks correct in isolation and the author addressed the SSL path and fallback-buffer edge in a follow-up comment, but this is exactly the class of change where a maintainer familiar with uWS internals (cirospaciari / Jarred-Sumner per the suggested reviewers) should confirm the lifetime and reentrancy reasoning.
Other factors
Test coverage is thorough (7 cases × TCP/TLS, including a no-coalesced-bytes control) and follows repo conventions well. My earlier inline nit about server cleanup on connect failure was addressed in 64ec35b. The bug-hunting system found no issues. The fix does not cover the fallback-buffer case (bytes are still dropped there), which the author acknowledges as "identical to current behaviour" — a maintainer may want to weigh whether that's acceptable or worth a follow-up.
|
Status: ready for review. The new test file ( CI red on this branch has been unrelated to the diff:
All review-bot threads are resolved. |
|
Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-07 and it conflicts with main. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main. |
… upgrade request (#43153) ### Problem - A client sends frames in the same TCP read as the upgrade request. `Bun.serve` answers 101 and never sees them: `server saw: ["later"] ping events: 0 pongs on the wire: 0`. A frame cut by the read closes the connection later. - After `server.upgrade()`, `HttpContext::onData` (`packages/bun-uws/src/HttpContext.h:560`) stops the HTTP parser, sends the 101 and returns the WebSocket (line 746). Nothing parses the rest of the read. ### Fix - `consumePostPadded` reports how much of the read it used. `onData` sends the 101, then gives the rest to the WebSocket with `us_dispatch_data`, in the same call. It stores no bytes. A declared request body is never handed over. - This matches `ws` on Node, whose upstream test now passes. A `server.upgrade()` in a later event-loop turn still gets `400 Bad Request` (#43149 tracks the `ws` shim). - Only for the socket of this read: `upgradedWebSocket` can name another connection's WebSocket. Without the check, a test shows one connection's bytes in another's `message` handler. - Verified: `test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts` (8 of 13 fail before), three tests in `test/js/first_party/ws/ws.test.ts`, plus the websocket, `serve.test.ts` and node:http upgrade suites. Self-reviewed (Notes). ### Background - `HttpResponse::upgrade` destroys the socket's HTTP state, moves the socket into the WebSocket context in place, and sets `upgradedWebSocket` so that `onData` sees the change. - A request head that spans two reads is parsed from the parser's `fallback` buffer, which the upgrade frees. So the parser reports an offset, not a pointer. - `us_dispatch_data` is how the event loop delivers read bytes to a socket. Its first C++ caller needs `extern "C"`. <details><summary>Notes</summary> **Repro** (bun only). Before: `server saw: ["later"] ping events: 0 pongs on the wire: 0`. After: `server saw: ["early","later"] ping events: 1 pongs on the wire: 1`. ```js const net = require("node:net"); const crypto = require("node:crypto"); function frame(opcode, payload) { const p = Buffer.from(payload), mask = crypto.randomBytes(4); const out = Buffer.alloc(6 + p.length); out[0] = 0x80 | opcode; out[1] = 0x80 | p.length; mask.copy(out, 2); for (let i = 0; i < p.length; i++) out[6 + i] = p[i] ^ mask[i % 4]; return out; } const seen = []; let pings = 0; const server = Bun.serve({ port: 0, hostname: "127.0.0.1", fetch(req, server) { if (server.upgrade(req)) return; return new Response("no", { status: 400 }); }, websocket: { message(ws, msg) { seen.push(String(msg)); if (String(msg) === "later") ws.close(1000); }, ping() { pings++; }, }, }); const key = crypto.randomBytes(16).toString("base64"); const upgrade = `GET / HTTP/1.1\r\nHost: 127.0.0.1:${server.port}\r\nUpgrade: websocket\r\nConnection: Upgrade\r\nSec-WebSocket-Key: ${key}\r\nSec-WebSocket-Version: 13\r\n\r\n`; const c = net.connect(server.port, "127.0.0.1", () => { c.write(Buffer.concat([Buffer.from(upgrade), frame(0x1, "early"), frame(0x9, "p")])); }); let got = Buffer.alloc(0), sentLater = false; c.on("data", d => { got = Buffer.concat([got, d]); if (!sentLater && got.includes("\r\n\r\n")) { sentLater = true; c.write(frame(0x1, "later")); } }); c.on("close", () => { const body = got.subarray(got.indexOf("\r\n\r\n") + 4); let pongs = 0; for (let i = 0; i < body.length; ) { if ((body[i] & 0x0f) === 0xa) pongs++; i += 2 + (body[i + 1] & 0x7f); } console.log("server saw:", JSON.stringify(seen), "ping events:", pings, "pongs on the wire:", pongs); server.stop(true); }); ``` **Who sees this.** No user reported it. RFC 6455 section 4.1 tells a client to wait for the 101, and browsers do. A client that writes the request and the first frame back to back sees it only when TCP puts both in one read, so it loses frames some of the time and gets no error. gorilla/websocket rejects such a client on purpose. The `ws` package on Node parses the frames. This PR takes the `ws` behavior, because an open connection that lost data with no signal is the worst of the three. **Where the boundary is.** The fix applies when `server.upgrade()` runs before the dispatch of the request returns to the parser. That includes an `async` handler whose awaits need no new turn of the event loop (`await Promise.resolve()`, `await req.text()` on a GET). Two tests cover that. After a timer or I/O, the read is over. The parser has then read the frame bytes as a pipelined request, `getHeaders` fails, and uWS writes `HTTP/1.1 400 Bad Request` with `Connection: close`. `server.upgrade()` then returns `false`. A test pins this. One exception is left as it is: early bytes that can still begin a request line (for example the single byte `0x41`) wait in the parser's buffer, and the upgrade frees that buffer. **`ws` shim.** #43149 has the full table. A `handleUpgrade()` inside the 'upgrade' event takes the fixed path. A `handleUpgrade()` in a later task, a `verifyClient` that answers later, and frames that arrive in a read of their own before a deferred `handleUpgrade()` still lose the frames: they are in `head` or in the socket's stream, and `src/js/thirdparty/ws.js` has no way to give bytes to the native WebSocket. That needs a design decision, so it has three `it.todo` tests and the issue. The new test `handles data passed along with the upgrade request` is a port of the test of the same name in websockets/ws `test/websocket-server.test.js`. **The check on the socket.** `upgradedWebSocket` is one field per HTTP context. `HttpResponse::upgrade` sets it when any socket of the context is in `onData`. Two ways lead to a value that belongs to another connection. (1) A handler of connection A resolves a promise of connection B, and B's `server.upgrade()` runs in the microtask checkpoint of A's dispatch. The test `never reach the WebSocket of another connection` covers this: with the check removed in a local build, the `message` handler of B received the frame that A sent. (2) A `server.upgrade()` from a request body handler (node:http with a body on the upgrade request, `handleUpgrade()` from `req.on("end")`) leaves the field set. I instrumented a build: the next request on another connection saw the stale field in its request handler (`consumed=32 length=44`). #43163 tracks that bug. Both happen on main today, and this PR does not change what main does there. #37463 fixes (1) at its source. The two PRs are independent and work in either order. With #37463, a connection whose synchronous upgrade is followed by another connection's upgrade in the same dispatch also gets its frames. The check has to stay with #37463 too, because of (2). **`upgrade()` adopts in place.** On linux x64, `sizeof(WebSocketData)` is 160 and `sizeof(HttpResponseData<SSL>)` is 224, and `us_socket_adopt` keeps the block when the new ext is not larger. If a future layout makes the adopt move the socket, the check fails, the frames are dropped as before, and the new tests fail. **Request bodies.** An upgrade request can declare a body. Node reads it as the body of the request. `server.upgrade()` never looked at it. On main, the bytes of such a body in the same read are dropped and the WebSocket works. In a later read they go to the WebSocket parser, which closes the connection. This PR keeps both. When the request declared a body (a `Content-Length` above 0, or chunked), the parser returns the count `HttpParserResult::WHOLE_READ` and `onData` hands nothing over. Every other count is the exact end of the request head, also for a head that fills the parser's 16 KiB buffer for split heads (a test covers that size). The first push of this PR handed the body over too, and that closed connections that main keeps open. Three tests in the new file and one in `ws.test.ts` pin it: they pass on main, fail on the first push, and pass now. The count is a named value and not a new field, because `HttpParserResult` is 16 bytes and comes back in two registers. **Cost on the normal path.** One pointer copy at the top of `consumePostPadded`. The other new code runs only after a handler took the socket. The request handler lambda has no new captures: it lives in a `MoveOnlyFunction` with a 16-byte inline buffer, and a larger closure would allocate on every `onData` call. **Earlier work.** #33692 fixed the same bug in July and was closed as stale, with no judgment on the fix. This PR also covers a request head that spans two reads and a read that starts with the rest of another request's body, adds no lambda captures, and has the check on the socket. **Self-review.** Addressed: the description of the failure (a parser that starts in the middle of a frame, not only a drop), the boundary (a turn of the event loop, not an `await`), the scope of "clear failure" (Bun.serve only), the port of the upstream `ws` test, tests for the microtask upgrade and for the body-tail offset, and the `ws` shim gaps (todo tests and #43149). Rejected: to fold #37463 in and drop the check on the socket. #37463 is open and green on its own, and case (2) above needs the check with or without it. **Reentrancy.** A handler that runs the event loop inside itself after `server.upgrade()` (for example `Bun.build` with an async plugin `setup()`) can let a later read reach the WebSocket before these bytes. HTTP reads have the same property (#42794). **Related open PRs in the same files.** #37463 (`HttpResponse.h`, the `isParsingHttp` lines of `HttpContext.h`), #42789 (the fallback return in `HttpParser.h`, a textual conflict only: `had` stays a local there), #39843 (the uncork lines above the new block), #38128, #39802. **Why a new test file.** `websocket-server.test.ts` does not pass as a whole under a debug ASAN build on my machine: its subprocess-client tests time out on main without this change. A run of that file before and after the fix proves nothing. The directory already has one file for each raw-frame topic (`websocket-server-rsv-frames`, `-unmasked-frames`, `-upgrade-reentrant`). **Suites run with the debug build:** the new file (36 runs, all pass), `test/js/first_party/ws/ws.test.ts`, `test/js/bun/websocket/`, `test/js/bun/http/serve.test.ts`, `request-smuggling.test.ts`, `http-server-chunking.test.ts`, `bun-server.test.ts`, `node-http-with-ws.test.ts`, `node-http-req-socket-pause.test.ts`, `node-http-connect.test.ts`. The subprocess-client tests in `websocket-server.test.ts` time out on my machine with and without this change, and pass when run alone. Two `serve.test.ts` tests fail on my machine for reasons of the machine (it runs as root, and its network blocks the external address). </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 0 · 5 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 10 failed, 3 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts test/js/first_party/ws/ws.test.ts bun test v1.4.3 (c6b7fcb) test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts: 146 | // One write, so the request and the frames reach the server in one read. 147 | client.socket.write(Buffer.concat([Buffer.from(upgradeRequest), text("early"), ping("p")])); 148 | expect(await client.status()).toBe("HTTP/1.1 101 Switching Protocols"); 149 | client.socket.write(text("later")); 150 | 151 | expect(await client.framesUntil("text:echo:later")).toEqual(["text:echo:early", "pong:p", "text:echo:later"]); ^ error: expect(received).toEqual(expected) [ - "text:echo:early", - "pong:p", "text:echo:later", ] - Expected - 2 + Received + 0 at <anonymous> (/workspace/bun/test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts:151:57) 169 | 170 | client.socket.write(Buffer.concat([Buffer.from(upgradeRequest), text ... (truncated) release without fix: 4 failed, 3 skipped bun test v1.4.3-canary.1 (becf408) test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts: 78 | // Only observed through the races below. 79 | failed.promise.catch(() => {}); 80 | socket.on("error", error => failed.reject(error)); 81 | socket.on("close", () => { 82 | closed.resolve(); 83 | failed.reject(new Error("the server closed the socket")); ^ error: the server closed the socket at <anonymous> (/workspace/bun/test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts:83:25) at emit (node:events:100:22) at <anonymous> (node:net:2350:20) 78 | // Only observed through the races below. 79 | failed.promise.catch(() => {}); 80 | socket.on("error", error => failed.reject(error)); 81 | socket.on("close", () => { 82 | closed.resolve(); 83 | failed.reject(new Error("the server closed the socket")); ^ error: the server closed the socket at <anonymous> (/workspace/bun/test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts:83:25) at emit (node:events:100:22) at <anonymous> (node:net:2350:2 ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: 3 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts test/js/first_party/ws/ws.test.ts bun test v1.4.3 (c6b7fcb) test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts: (pass) frames in the same read as the upgrade request > are delivered in order, and a ping gets its pong (tls: false) [738.45ms] (pass) frames in the same read as the upgrade request > are delivered when server.upgrade() runs after `await Promise.resolve()` [381.05ms] (pass) frames in the same read as the upgrade request > are delivered when server.upgrade() runs after `await req.text()` [380.86ms] (pass) frames in the same read as the upgrade request > a frame that the read cuts short is completed by the next read [397.03ms] (pass) frames in the same read as the upgrade request > are delivered in order, and a ping gets its pong (tls: true) [680.67ms] (pass) frames in the same read as the upgrade request > a Content-Length body in the same read as the upgrade request is not parsed as frames [131.43ms] (pass) frames in the same read as the u ... (truncated) release with fix: 3 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 666ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [0/27] 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�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` packages/bun-usockets/src/internal/internal.h | 6 + packages/bun-uws/src/HttpContext.h | 16 +- packages/bun-uws/src/HttpParser.h | 26 +- .../websocket-server-upgrade-early-frames.test.ts | 344 +++++++++++++++++++++ test/js/first_party/ws/ws.test.ts | 133 +++++++- 5 files changed, 518 insertions(+), 7 deletions(-) ``` </details> **gate history** · 2 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests packages/bun-usockets/src/internal/internal.h 1 2 45 packages/bun-uws/src/HttpContext.h 6 5 46 packages/bun-uws/src/HttpParser.h 3 4 45 …websocket/websocket-server-upgrade-early-frames.test.ts 2 5 30 test/js/first_party/ws/ws.test.ts 2 3 31 ``` </details> <!-- robobun:evidence:end -->
1 participant
What
Bun.serve's WebSocket server silently dropped any frames that arrived in the same TCP read as the HTTP upgrade request. The server answered101 Switching Protocols, the connection stayed open, butmessage()never fired for the coalesced frame, a coalesced PING got no PONG, and a coalesced malformed frame raised no protocol error. A later frame on the same connection was processed normally, so the bytes were dropped, not buffered.TCP has no message boundaries; whether the first frame lands in the same
read()as the request head is segmentation luck (an eager client doing back-to-backwrite()s, a proxy flushing both in onewritev, loopback). RFC 6455 section 4.1 tells the client to wait for the 101 before sending frames, which is why browsers never trip this, but the server still must not lose bytes already on its receive stream. Bun's own WebSocket client handles the mirror case correctly.Repro
Cause
HttpContext<SSL>::onDatacalls the HTTP parser, which invokes the route handler;server.upgrade()adopts the socket into the WebSocket context and stashes it onhttpContextData->upgradedWebSocket. The request-handler lambda then returnsnullptr,consumePostPaddedreturns, andonDatauncorks the adopted socket and returns it. Thelength - consumedbytes remaining in that read are never handed to the new context.Fix
HttpRequest::headalready records the bytes past the request head (it backs theheadargument of Node'supgradeevent). When the request-handler lambda detectsupgradedWebSocket, capture that span (only when it points into the live read buffer; the parser's fallback buffer is destroyed byupgrade()). After uncorking the 101 response, feed the captured bytes to the adopted socket viaus_dispatch_data, which routes toWebSocketContext<SSL>::onData.The
us_dispatch_*declarations ininternal.hwere not wrapped for C++ linkage; this is the first C++ caller.Verification
Six cases fail on
main(text frame, PING, multiple frames, partial frame, CLOSE, malformed) and a no-coalesced-bytes control passes; all seven pass with the fix.websocket-server.test.tsand the otherwebsocket-server-*.test.tsfiles are unchanged.