diff --git a/src/http_jsc/websocket_client/WebSocketUpgradeClient.rs b/src/http_jsc/websocket_client/WebSocketUpgradeClient.rs index a3f837982049..6ad5eddb7747 100644 --- a/src/http_jsc/websocket_client/WebSocketUpgradeClient.rs +++ b/src/http_jsc/websocket_client/WebSocketUpgradeClient.rs @@ -925,11 +925,6 @@ impl HTTPClient { let me = unsafe { &mut *this.as_ptr() }; let mut body = data; if !me.body.is_empty() { - if me.body.len().saturating_add(data.len()) > bun_http::max_http_header_size() { - // SAFETY: `me`'s last use is above; no `&mut Self` spans this call. - unsafe { Self::terminate(this.as_ptr(), ErrorCode::InvalidResponse) }; - return; - } me.body.extend_from_slice(data); body = &me.body; } @@ -954,13 +949,16 @@ impl HTTPClient { } Err(picohttp::ParseResponseError::ShortRead) => { if me.body.is_empty() { - if data.len() > bun_http::max_http_header_size() { - // SAFETY: `me`'s last use is above; no `&mut Self` spans this call. - unsafe { Self::terminate(this.as_ptr(), ErrorCode::InvalidResponse) }; - return; - } me.body.extend_from_slice(data); } + // ShortRead means no \r\n\r\n was found, so every byte in + // `body` is part of an incomplete header — cap that, not + // total bytes received (which may include pipelined + // WebSocket frames once the header does complete). + if me.body.len() > bun_http::max_http_header_size() { + // SAFETY: `me`'s last use is above; no `&mut Self` spans this call. + unsafe { Self::terminate(this.as_ptr(), ErrorCode::InvalidResponse) }; + } return; } }; @@ -985,11 +983,6 @@ impl HTTPClient { let me = unsafe { &mut *this }; let mut body = data; if !me.body.is_empty() { - if me.body.len().saturating_add(data.len()) > bun_http::max_http_header_size() { - // SAFETY: `me`'s last use is above; no `&mut Self` spans this call. - unsafe { Self::terminate(this, ErrorCode::InvalidResponse) }; - return; - } me.body.extend_from_slice(data); body = &me.body; } @@ -1017,13 +1010,15 @@ impl HTTPClient { } Err(picohttp::ParseResponseError::ShortRead) => { if me.body.is_empty() { - if data.len() > bun_http::max_http_header_size() { - // SAFETY: `me`'s last use is above; no `&mut Self` spans this call. - unsafe { Self::terminate(this, ErrorCode::InvalidResponse) }; - return; - } me.body.extend_from_slice(data); } + // ShortRead means no \r\n\r\n was found, so every byte in + // `body` is part of an incomplete header — cap that, not + // total bytes received. + if me.body.len() > bun_http::max_http_header_size() { + // SAFETY: `me`'s last use is above; no `&mut Self` spans this call. + unsafe { Self::terminate(this, ErrorCode::InvalidResponse) }; + } return; } }; @@ -1233,11 +1228,6 @@ impl HTTPClient { // Process as if it came directly from the socket let mut body = data; if !me.body.is_empty() { - if me.body.len().saturating_add(data.len()) > bun_http::max_http_header_size() { - // SAFETY: `me`'s last use is above; no `&mut Self` spans this call. - unsafe { Self::terminate(this, ErrorCode::InvalidResponse) }; - return; - } me.body.extend_from_slice(data); body = &me.body; } @@ -1262,13 +1252,16 @@ impl HTTPClient { } Err(picohttp::ParseResponseError::ShortRead) => { if me.body.is_empty() { - if data.len() > bun_http::max_http_header_size() { - // SAFETY: `me`'s last use is above; no `&mut Self` spans this call. - unsafe { Self::terminate(this, ErrorCode::InvalidResponse) }; - return; - } me.body.extend_from_slice(data); } + // ShortRead means no \r\n\r\n was found, so every byte in + // `body` is part of an incomplete header — cap that, not + // total bytes received (which may include pipelined + // WebSocket frames once the header does complete). + if me.body.len() > bun_http::max_http_header_size() { + // SAFETY: `me`'s last use is above; no `&mut Self` spans this call. + unsafe { Self::terminate(this, ErrorCode::InvalidResponse) }; + } return; } }; diff --git a/test/js/web/websocket/websocket-client-short-read.test.ts b/test/js/web/websocket/websocket-client-short-read.test.ts index 7df1c5183969..3a0650c34b6b 100644 --- a/test/js/web/websocket/websocket-client-short-read.test.ts +++ b/test/js/web/websocket/websocket-client-short-read.test.ts @@ -100,6 +100,148 @@ describe("WebSocket", () => { }); }); +describe("WebSocket upgrade split across reads", () => { + function makeAccept(key: string): string { + const hasher = new Bun.CryptoHasher("sha1"); + hasher.update(key); + hasher.update("258EAFA5-E914-47DA-95CA-C5AB0DC85B11"); + return hasher.digest("base64"); + } + + // Unmasked binary frame with a 64-bit length header and `n` zero bytes of payload. + function bigBinaryFrame(n: number): Uint8Array { + const header = new Uint8Array(10); + header[0] = 0x82; // FIN + binary + header[1] = 127; // 64-bit length follows + header[6] = (n >>> 24) & 0xff; + header[7] = (n >>> 16) & 0xff; + header[8] = (n >>> 8) & 0xff; + header[9] = n & 0xff; + const out = new Uint8Array(10 + n); + out.set(header, 0); + return out; + } + + test("large frame pipelined after split 101 header is not counted against the header-size cap", async () => { + // First read delivers a partial status line (ShortRead -> buffered); second + // read delivers the rest of the 101 header plus a >16KB binary frame in one + // segment. The header-size cap must only apply to bytes that are provably + // header (the ShortRead accumulator), not to pipelined frame bytes. + const PAYLOAD = 20000; // > default max_http_header_size (16384) + + using server = Bun.listen<{ buf: string; done: boolean }>({ + hostname: "127.0.0.1", + port: 0, + socket: { + open(socket) { + socket.data = { buf: "", done: false }; + }, + data(socket, chunk) { + const st = socket.data; + if (st.done) return; + st.buf += chunk.toString("latin1"); + if (!st.buf.includes("\r\n\r\n")) return; + st.done = true; + const m = /Sec-WebSocket-Key:\s*(\S+)/i.exec(st.buf); + if (!m) { + socket.end(); + return; + } + const accept = makeAccept(m[1]); + + // First segment: partial status line -> client buffers via ShortRead. + socket.write("HTTP/1.1 101 "); + socket.flush(); + + // Second segment: header tail + a >16KB frame, written together so + // they arrive in the same read on the client. + setTimeout(() => { + const tail = + "Switching Protocols\r\n" + + "Upgrade: websocket\r\n" + + "Connection: Upgrade\r\n" + + `Sec-WebSocket-Accept: ${accept}\r\n` + + "\r\n"; + const tailBytes = new TextEncoder().encode(tail); + const frame = bigBinaryFrame(PAYLOAD); + const packet = new Uint8Array(tailBytes.length + frame.length); + packet.set(tailBytes, 0); + packet.set(frame, tailBytes.length); + socket.write(packet); + socket.flush(); + }, 50); + }, + }, + }); + + const { promise, resolve, reject } = Promise.withResolvers<{ open: boolean; bytes: number }>(); + let opened = false; + const ws = new globalThis.WebSocket(`ws://127.0.0.1:${server.port}`); + ws.binaryType = "arraybuffer"; + ws.onopen = () => { + opened = true; + }; + ws.onmessage = ev => { + const data = ev.data as ArrayBuffer; + resolve({ open: opened, bytes: data.byteLength }); + }; + ws.onerror = ev => reject(new Error("ws error: " + (ev as ErrorEvent).message)); + ws.onclose = ev => { + if (!ev.wasClean) reject(new Error(`unclean close: ${ev.code} ${ev.reason}`)); + }; + + try { + expect(await promise).toEqual({ open: true, bytes: PAYLOAD }); + } finally { + ws.close(); + } + }); + + test("incomplete header larger than the cap is still rejected", async () => { + // >16KB of header bytes with no terminating blank line must still fail. + using server = Bun.listen<{ buf: string; done: boolean }>({ + hostname: "127.0.0.1", + port: 0, + socket: { + open(socket) { + socket.data = { buf: "", done: false }; + }, + data(socket, chunk) { + const st = socket.data; + if (st.done) return; + st.buf += chunk.toString("latin1"); + if (!st.buf.includes("\r\n\r\n")) return; + st.done = true; + + socket.write("HTTP/1.1 101 Switching Protocols\r\n"); + socket.flush(); + setTimeout(() => { + // 20KB of header field bytes, no \r\n\r\n terminator. + const pad = Buffer.alloc(20000, "a"); + socket.write(Buffer.concat([Buffer.from("X-Pad: "), pad])); + socket.flush(); + }, 50); + }, + }, + }); + + const { promise, resolve, reject } = Promise.withResolvers(); + const ws = new globalThis.WebSocket(`ws://127.0.0.1:${server.port}`); + ws.onopen = () => reject(new Error("unexpected open")); + ws.onerror = ev => resolve((ev as ErrorEvent).message ?? "error"); + ws.onclose = ev => { + if (ev.wasClean) reject(new Error("unexpected clean close")); + }; + + try { + const msg = await promise; + expect(msg).toContain("Invalid response"); + } finally { + ws.close(); + } + }); +}); + describe("WebSocket buffered handshake data", () => { test("terminating the client from its open handler while handshake bytes are buffered shuts down cleanly", async () => { // A raw TCP "websocket server" that appends a complete text frame to the 101