Bun.serve: make server.upgrade() honor handshake headers deleted from req.headers

Closed
RO
2mo ago
+55-8

Problem

  • In a Bun.serve handler, req.headers.delete("sec-websocket-protocol") then server.upgrade(req) in the same tick still answers 101 with Sec-WebSocket-Protocol: chat. Deleting sec-websocket-key, sec-websocket-version or upgrade still upgrades. With any await before upgrade() the same code honors the deletion (101 without a protocol, 400, 426, 400).
  • Cause: the header read in upgrade() (src/runtime/server/server_body.rs:1836-1881). It reads the five handshake fields from the Request's FetchHeaders, then fills each field that is still empty from r.header(name) on the live uWS request. A deleted field is empty, so the raw line comes back. After an await the uWS request is gone and there is no fallback.

Fix

  • Once request.headers exists, read the handshake from it alone. Read the raw uWS request only when no Headers object was ever created.
  • Correct because a Bun.serve Request builds its FetchHeaders from every field of the uWS request (Request.rs:252). A field missing from it later was removed by the handler. The post-await path already worked this way.
  • Verified: test/js/bun/websocket/websocket-server.test.ts (new it.each, the "before any await" case fails on canary f42e98025). Also ran the rest of that file and the other server.upgrade() test files.

Background

  • server.upgrade(req) validates the opening handshake (RFC 6455 §4.2.1) from these fields, echoes the first offered subprotocol, and derives Sec-WebSocket-Accept from the key.
  • A Bun.serve Request is lazy: req.headers is created from the uWS request on first access. Until fetch() awaits, the Request points at the live uWS request. When fetch() returns a pending promise, the server snapshots url and headers and detaches it.
Notes

Repro matrix on canary f42e98025 (raw request offers Sec-WebSocket-Protocol: chat, superchat), synchronous upgrade():

req.headers.delete(...)beforeafter (same as post-await, which is unchanged)
sec-websocket-protocol101, Sec-WebSocket-Protocol: chat101, no protocol header
sec-websocket-key101400 (upgrade() returns false)
sec-websocket-version101426 Upgrade Required
upgrade101400 (upgrade() returns false)

The NodeHTTPResponse branch of upgrade() (node:http servers) is a separate code path and is not touched.

An await Promise.resolve() before upgrade() does not reach the detached path: the microtask runs before the request leaves the HTTP parser's stack. The test uses setImmediate for the "after an await" case.

Other suites run locally on the debug build: websocket-server-upgrade-reentrant.test.ts, websocket-upgrade-signal-gc.test.ts, bun-server.test.ts, test/js/web/websocket/websocket.test.js. Under the debug ASAN build, ServerWebSocket > send() > (benchmark) (330k compressed echo messages) times out at 30 s in this container regardless of this change. It never touches req.headers and takes the unchanged raw-request branch.

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/websocket/websocket-server.test.ts