Skip to content

websocket: only cap header bytes, not pipelined frames, during upgrade #32394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alii merged 1 commit into from
Jun 16, 2026
Merged

websocket: only cap header bytes, not pipelined frames, during upgrade #32394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 23 additions & 30 deletions src/http_jsc/websocket_client/WebSocketUpgradeClient.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,11 +925,6 @@ impl<const SSL: bool> HTTPClient<SSL> {
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;
}
Expand All @@ -954,13 +949,16 @@ impl<const SSL: bool> HTTPClient<SSL> {
}
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;
}
};
Expand All @@ -985,11 +983,6 @@ impl<const SSL: bool> HTTPClient<SSL> {
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;
}
Expand Down Expand Up @@ -1017,13 +1010,15 @@ impl<const SSL: bool> HTTPClient<SSL> {
}
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;
}
};
Expand Down Expand Up @@ -1233,11 +1228,6 @@ impl<const SSL: bool> HTTPClient<SSL> {
// 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;
}
Expand All @@ -1262,13 +1252,16 @@ impl<const SSL: bool> HTTPClient<SSL> {
}
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;
}
};
Expand Down
142 changes: 142 additions & 0 deletions test/js/web/websocket/websocket-client-short-read.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
},
},
});

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<string>();
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
Expand Down
Loading