Bun.serve: require Connection: Upgrade token and single Sec-WebSocket-Key in server.upgrade()

Closed
RO
2mo ago
+107-10

Follow-up to #35298. Three more RFC-invalid opening handshakes were still answered with 101 Switching Protocols and reached the app's open():

import net from "node:net";
const server = Bun.serve({
  port: 0,
  fetch(req, srv) { if (srv.upgrade(req)) return; return new Response("no", { status: 400 }); },
  websocket: { open() {}, message() {} },
});
const KEY = "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==";
const V = "Sec-WebSocket-Version: 13";
// all three get "HTTP/1.1 101 Switching Protocols":
await hs(["Upgrade: websocket", "Connection: close", KEY, V]);                        // want 400
await hs(["Upgrade: websocket", KEY, V]);                                             // Connection absent, want 400
await hs(["Upgrade: websocket", "Connection: Upgrade", KEY,
          "Sec-WebSocket-Key: eHh4eHh4eHh4eHh4eHh4eA==", V]);                          // two keys, want 400

RFC 6455 section 4.2.1 requires the client handshake to carry a Connection header whose token list includes Upgrade, and section 11.3.1 says Sec-WebSocket-Key MUST NOT appear more than once.

Cause

The validation added in #35298 checks the Upgrade token, the Sec-WebSocket-Key shape, and Sec-WebSocket-Version, but never reads Connection. On the synchronous path the key is read via uws_req_get_header, which returns only the first field line for a given name, so a second Sec-WebSocket-Key is invisible to the shape check.

Fix

server.upgrade() now also returns false (so the caller's fetch() responds) when Connection is absent or its token list does not contain upgrade, and when Sec-WebSocket-Key appears more than once.

The raw-uWS path walks the request's header list once via a new uws_req_ws_handshake_scan shim to count Sec-WebSocket-Key field lines and test every Connection value's token list; uws_req_get_header cannot answer either question. On the async/FetchHeaders path, HTTPHeaderMap::add already comma-joins repeated field lines, so a token scan over fast_get(Connection) covers every line and a repeated key fails the existing is_valid_sec_websocket_key length check.

Connection: keep-alive, Upgrade, lowercase Connection: upgrade, and the token split across two Connection field lines are still accepted.

Verification

Extended the existing server.upgrade() validates the opening handshake block in test/js/bun/websocket/websocket-server.test.ts with the three rejected cases, the substring-is-not-a-token case, and the accepted token-list / multi-line cases, on both the synchronous and async/req.headers-materialized paths. With src/ reverted the new Connection-missing assertions fail with status 101; with the fix, both tests pass (30 assertions).

no test proof · iteration 1 · 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