WebSocket: cap the 101 handshake head on the completed parse path

Closed
RO
2mo ago
+62-0

Problem

  • new WebSocket() accepts a 101 handshake head larger than the cap (--max-http-header-size, default 16 KB) whenever no recv() boundary lands inside the incomplete head. A 60 KB head split so the first read stays under the cap opens the connection.
  • The cap is checked only in the ShortRead arm of buffer_and_parse_head (src/http_jsc/websocket_client/WebSocketUpgradeClient.rs:717). The Ok(done) arm, which runs when the head parse completes, never checks the size. So the bound depends on read segmentation, not on the head size.

Fix

  • Check the cap in the Ok(done) arm too. Compare head_len (the header size alone), so pipelined WebSocket frames after the head are not counted.
  • Correct because the cap now applies to the header bytes on every path, not only when a read boundary splits an incomplete head. This matches the existing ShortRead check and Node and undici, which enforce a fixed 16 KB regardless of segmentation.
  • Verified: test/js/web/websocket/websocket-client-short-read.test.ts (new case, stock bun opens the connection and fails). Also ran the other 4 cases in that file, including the pipelined large-frame case that must still open.

Background

  • A WebSocket client first sends an HTTP/1.1 upgrade request, then reads the 101 response head. buffer_and_parse_head accumulates bytes and runs picohttp::Response::parse.
  • parse returns ShortRead while no \r\n\r\n is found, and Done once the head is complete. head_len (response.bytes_read) is the head size. Bytes after it are pipelined WebSocket frames.
  • max_http_header_size (default 16 KB) bounds the head so a peer cannot force unbounded buffering.

[human-review] gate passed · iteration 1 · 2 files touched

fails on main (without fix) 
ASAN without fix: 1 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/web/websocket/websocket-client-short-read.test.ts
bun test v1.4.3 (f42e98025)

test/js/web/websocket/websocket-client-short-read.test.ts:
(pass) WebSocket > short read on payload length [99.64ms]
(pass) WebSocket upgrade split across reads > large frame pipelined after split 101 header is not counted against the header-size cap [108.04ms]
239 |       },
240 |     });
241 | 
242 |     const { promise, resolve, reject } = Promise.withResolvers<string>();
243 |     const ws = new globalThis.WebSocket(`ws://127.0.0.1:${server.port}`);
244 |     ws.onopen = () => reject(new Error("unexpected open"));
                                       ^
error: unexpected open
      at <anonymous> (/workspace/bun/test/js/web/websocket/websocket-client-short-read.test.ts:244:34)
(fail) WebSocket upgrade split across reads > completed header larger than the cap is rejected even when split under the cap [102.86ms]
(pass) WebSocket upgrade split across reads > incomplete header larger than the cap is still rejected [84.91ms]
(pass) WebSocket buffered 
... (truncated)

release without fix: 1 FAILED
bun test v1.4.3-canary.1 (f42e98025)

test/js/web/websocket/websocket-client-short-read.test.ts:
(pass) WebSocket > short read on payload length [2.92ms]
(pass) WebSocket upgrade split across reads > large frame pipelined after split 101 header is not counted against the header-size cap [51.35ms]
239 |       },
240 |     });
241 | 
242 |     const { promise, resolve, reject } = Promise.withResolvers<string>();
243 |     const ws = new globalThis.WebSocket(`ws://127.0.0.1:${server.port}`);
244 |     ws.onopen = () => reject(new Error("unexpected open"));
                                       ^
error: unexpected open
      at <anonymous> (/workspace/bun/test/js/web/websocket/websocket-client-short-read.test.ts:244:34)
(fail) WebSocket upgrade split across reads > completed header larger than the cap is rejected even when split under the cap [53.38ms]
(pass) WebSocket upgrade split across reads > incomplete header larger than the cap is still rejected [51.21ms]
(pass) WebSocket buffered handshake data > terminating the client from its open handler while handshake bytes are buffered shuts down cleanly [13.24ms]

 4 pass
 1 fail
 5 expect() calls
Ran 5 tests across 1 fi
... (truncated)
passes on PR (with fix) 
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/web/websocket/websocket-client-short-read.test.ts
bun test v1.4.3 (f42e98025)

test/js/web/websocket/websocket-client-short-read.test.ts:
(pass) WebSocket > short read on payload length [81.49ms]
(pass) WebSocket upgrade split across reads > large frame pipelined after split 101 header is not counted against the header-size cap [1390.69ms]
(pass) WebSocket upgrade split across reads > completed header larger than the cap is rejected even when split under the cap [80.45ms]
(pass) WebSocket upgrade split across reads > incomplete header larger than the cap is still rejected [83.33ms]
(pass) WebSocket buffered handshake data > terminating the client from its open handler while handshake bytes are buffered shuts down cleanly [595.38ms]

 5 pass
 0 fail
 6 expect() calls
Ran 5 tests across 1 file. [8.24s]
__F:0:S:0

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 7283ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/2355] host-cxx deps/icu/host-obj/source/i18n/reldtfmt.cpp.o
[2/2355] host-cxx deps/icu/host-obj/source/i18n/regexcmp.cpp.o
[3/2355] host-cxx deps/icu/host-obj/source/i18n/regeximp.cpp.o
[4/2355] host-cxx deps/icu/host-obj/source/i18n/regexst.cpp.o
[5/2355] host-cxx deps/icu/host-obj/source/i18n/regextxt.cpp.o
[6/2355] host-cxx deps/icu/host-obj/source/i18n/remtrans.cpp.o
[7/2355] host-cxx deps/icu/host-obj/source/i18n/rematch.cpp.o
[8/2355] host-cxx deps/icu/host-obj/source/i18n/rbnf.cpp.o
[9/2355] host-cxx deps/icu/host-obj/source/i18n/region.cpp.o
[10/2355] host-cxx deps/icu/host-obj/source/i18n/rbtz.cpp.o
[11/2355] host-cxx deps/icu/host-obj/source/i18n/reldatefmt.cpp.o
[12/2355] host-cxx deps/icu/host-obj/source/i18n/repattrn.cpp.o
[13/2355] host-cxx deps/icu/host-obj/source/i18n/selfmt.cpp.o
[14/2355] host-cxx deps/icu/host-obj/source/i18n/scriptset.cpp.o
[15/2355] host-cxx deps/icu/host-obj/source/i18n/scientificnumberformatter.cpp.o
[16/2355] host-cxx deps/icu/host-obj/source/i18n/sharedbreakite
... (truncated)
diff hotspot 
.../websocket_client/WebSocketUpgradeClient.rs     |  3 ++
 .../websocket/websocket-client-short-read.test.ts  | 59 ++++++++++++++++++++++
 2 files changed, 62 insertions(+)

gate history · 1 passed · 0 rejected · iteration 1

evidence per changed file 
file                                                      reads  edits  tests
src/http_jsc/websocket_client/WebSocketUpgradeClient.rs       5      3      5
…st/js/web/websocket/websocket-client-short-read.test.ts      2      3      5