Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
43 changes: 37 additions & 6 deletions src/http/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1048,8 +1048,16 @@
bun_core::RacyCell::new([picohttp::Header::ZERO; MAX_REQUEST_HEADERS]);

// this doesn't need to be stack memory because it is immediately cloned after use
static SHARED_RESPONSE_HEADERS_BUF: bun_core::RacyCell<[picohttp::Header; 256]> =
bun_core::RacyCell::new([picohttp::Header::ZERO; 256]);
const MAX_RESPONSE_HEADERS_INLINE: usize = 256;
static SHARED_RESPONSE_HEADERS_BUF: bun_core::RacyCell<
[picohttp::Header; MAX_RESPONSE_HEADERS_INLINE],
> = bun_core::RacyCell::new([picohttp::Header::ZERO; MAX_RESPONSE_HEADERS_INLINE]);

// Spillover for responses with more than MAX_RESPONSE_HEADERS_INLINE header
// fields. Sized on demand to the line count of the response buffer, so the
// 1 MB byte cap (MAX_RESPONSE_HEADER_BUFFER) bounds the allocation.
static SHARED_RESPONSE_HEADERS_OVERFLOW: bun_core::RacyCell<Vec<picohttp::Header>> =
bun_core::RacyCell::new(Vec::new());

// the first packet for Transfer-Encoding: chunked
// is usually pretty small or sometimes even just a length
Expand All @@ -1072,11 +1080,17 @@
unsafe { &mut *SHARED_REQUEST_HEADERS_BUF.get() }
}
#[inline]
pub(super) fn response_headers() -> &'static mut [picohttp::Header; 256] {
pub(super) fn response_headers() -> &'static mut [picohttp::Header; MAX_RESPONSE_HEADERS_INLINE]
{
// SAFETY: see module-level INVARIANT.
unsafe { &mut *SHARED_RESPONSE_HEADERS_BUF.get() }
}
#[inline]
pub(super) fn response_headers_overflow() -> &'static mut Vec<picohttp::Header> {
// SAFETY: see module-level INVARIANT.
unsafe { &mut *SHARED_RESPONSE_HEADERS_OVERFLOW.get() }
}
#[inline]
pub(super) fn single_packet_small_buffer() -> &'static mut [u8; 16 * 1024] {
// SAFETY: see module-level INVARIANT.
unsafe { &mut *SINGLE_PACKET_SMALL_BUFFER.get() }
Expand Down Expand Up @@ -3729,12 +3743,29 @@
return;
}

let shared_resp = scratch::response_headers();
let response = match picohttp::Response::parse_parts(
let mut parse_result = picohttp::Response::parse_parts(
to_read!(),
shared_resp,
scratch::response_headers(),
Some(&mut amount_read),
);
if matches!(
parse_result,
Err(picohttp::ParseResponseError::TooManyHeaders)
) {
// More than MAX_RESPONSE_HEADERS_INLINE fields. Size the
// overflow scratch to the line count (a strict upper bound on
// the field count) and reparse. The 1 MB byte cap below
// remains the only hard limit on the response header block.
let overflow = scratch::response_headers_overflow();
let needed = bun_core::strings::count_char(to_read!(), b'\n');
overflow.resize(needed, picohttp::Header::ZERO);

Check warning on line 3761 in src/http/lib.rs

View check run for this annotation

Claude / Claude Code Review

Overflow Vec sized to full-buffer newline count; retained at high-water mark

The overflow Vec is sized to `count_char(to_read!(), b'\n')`, but `to_read!()` at this point is the whole receive buffer — including any body bytes that arrived in the same read — so a >256-field response followed by a newline-dense body in the same packet(s) can size the Vec far beyond the actual header count (each `picohttp::Header` slot is 32 bytes, and a ~1 MB buffer of mostly-`\n` bytes maps to ~10-16 MB of slots). `SHARED_RESPONSE_HEADERS_OVERFLOW` is a process-lifetime static that is only
Comment thread
robobun marked this conversation as resolved.
parse_result = picohttp::Response::parse_parts(
to_read!(),
overflow.as_mut_slice(),
Some(&mut amount_read),
);

Check warning on line 3766 in src/http/lib.rs

View check run for this annotation

Claude / Claude Code Review

Stale SAFETY comment on detach_lifetime() omits new overflow Vec backing store

The `SAFETY` comment at [src/http/lib.rs:3801-3802](https://github.com/oven-sh/bun/blob/6e2b024886d9f89177e0100e16d7e68f4b5d03c3/src/http/lib.rs#L3801-L3802) for `unsafe { response.detach_lifetime() }` still says `response` borrows only `SHARED_RESPONSE_HEADERS_BUF / response_message_buffer`. On the new overflow path, `response.headers.list` instead borrows `SHARED_RESPONSE_HEADERS_OVERFLOW`'s heap buffer — whose element storage moves on the next `resize()`, so the "outlive this fn" phrasing no
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
}
let response = match parse_result {
Ok(r) => r,
Err(picohttp::ParseResponseError::ShortRead) => {
// `MAX_HTTP_HEADER_SIZE` (default 16 KB) is the *server*/
Expand Down
15 changes: 12 additions & 3 deletions src/http_jsc/websocket_client/WebSocketUpgradeClient.rs
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,10 @@ impl<const SSL: bool> HTTPClient<SSL> {

let response = match picohttp::Response::parse(body, &mut me.headers_buf) {
Ok(r) => r,
Err(picohttp::ParseResponseError::MalformedHttpResponse) => {
Err(
picohttp::ParseResponseError::MalformedHttpResponse
| picohttp::ParseResponseError::TooManyHeaders,
) => {
// SAFETY: `me`'s last use is above; no `&mut Self` spans this call.
unsafe { Self::terminate(this.as_ptr(), ErrorCode::InvalidResponse) };
return;
Expand Down Expand Up @@ -986,7 +989,10 @@ impl<const SSL: bool> HTTPClient<SSL> {
// Parse the response to find the end of headers
let response = match picohttp::Response::parse(body, &mut me.headers_buf) {
Ok(r) => r,
Err(picohttp::ParseResponseError::MalformedHttpResponse) => {
Err(
picohttp::ParseResponseError::MalformedHttpResponse
| picohttp::ParseResponseError::TooManyHeaders,
) => {
// SAFETY: `me`'s last use is above; no `&mut Self` spans this call.
unsafe { Self::terminate(this.as_ptr(), ErrorCode::InvalidResponse) };
return;
Expand Down Expand Up @@ -1227,7 +1233,10 @@ impl<const SSL: bool> HTTPClient<SSL> {

let response = match picohttp::Response::parse(body, &mut me.headers_buf) {
Ok(r) => r,
Err(picohttp::ParseResponseError::MalformedHttpResponse) => {
Err(
picohttp::ParseResponseError::MalformedHttpResponse
| picohttp::ParseResponseError::TooManyHeaders,
) => {
// SAFETY: `me`'s last use is above; no `&mut Self` spans this call.
unsafe { Self::terminate(this, ErrorCode::InvalidResponse) };
return;
Expand Down
13 changes: 13 additions & 0 deletions src/picohttp/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,11 @@ impl fmt::Display for StatusCodeFormatter {
pub enum ParseResponseError {
#[strum(serialize = "Malformed_HTTP_Response")]
MalformedHttpResponse,
/// picohttpparser filled every slot in the caller's `[Header]` scratch
/// before finding the terminating CRLF. The response may be valid; the
/// caller can retry with a larger buffer.
#[strum(serialize = "Response_Headers_Too_Large")]
TooManyHeaders,
ShortRead,
}
bun_core::impl_tag_error!(ParseResponseError);
Expand Down Expand Up @@ -618,6 +623,14 @@ impl<'a> Response<'a> {

match rc {
-1 => {
// picohttpparser returns -1 both for genuinely invalid input
// and when `num_headers` reaches `max_headers` before the
// terminating CRLF. In the overflow case `num_headers` is
// written back equal to the input capacity; every other -1
// path leaves it strictly below.
if !src.is_empty() && num_headers == src.len() {
return Err(ParseResponseError::TooManyHeaders);
}
bun_core::debug!("Malformed HTTP response:\n{}", BStr::new(buf));
Err(ParseResponseError::MalformedHttpResponse)
}
Expand Down
50 changes: 49 additions & 1 deletion test/js/bun/http/fetch-header-count-limit.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, test } from "bun:test";
import { describe, expect, test } from "bun:test";
import { once } from "node:events";
import { createServer } from "node:net";

Expand Down Expand Up @@ -105,3 +105,51 @@ test("default headers preserved when user headers overflow the buffer", async ()
expect(headerNames).toContain("user-agent");
expect(headerNames).toContain("accept");
});

// The response header field count is bounded only by the 1 MB byte cap on
// the header block; there is no fixed slot count. Node/undici behave the
// same way (llhttp caps bytes, not fields).
describe("fetch accepts responses with more than 256 header fields", () => {
async function serveNHeaders(n: number) {
const server = createServer(socket => {
socket.on("error", () => {});
socket.once("data", () => {
let head = "HTTP/1.1 200 OK\r\n";
for (let i = 0; i < n; i++) head += `X-H-${i}: v${i}\r\n`;
socket.end(head + "Content-Length: 2\r\nConnection: close\r\n\r\nok");
});
});
server.listen(0, "127.0.0.1");
await once(server, "listening");
return server;
}

test.concurrent.each([200, 255, 300, 1000])("%i header fields", async n => {
await using server = await serveNHeaders(n);
const { port } = server.address() as import("node:net").AddressInfo;
const res = await fetch(`http://127.0.0.1:${port}/`);
expect(res.status).toBe(200);
expect(await res.text()).toBe("ok");
// Spot-check the first field, the last field, and Content-Length so the
// whole header block was delivered without truncation.
expect(res.headers.get("x-h-0")).toBe("v0");
expect(res.headers.get(`x-h-${n - 1}`)).toBe(`v${n - 1}`);
expect(res.headers.get("content-length")).toBe("2");
});

test.concurrent("genuinely malformed response still rejects with Malformed_HTTP_Response", async () => {
await using server = createServer(socket => {
socket.on("error", () => {});
socket.once("data", () => socket.end("HTTP/1.1 200 OK\r\nbad header line\r\n\r\n"));
}).listen(0, "127.0.0.1");
await once(server, "listening");
const { port } = server.address() as import("node:net").AddressInfo;
let code: string | undefined;
try {
await fetch(`http://127.0.0.1:${port}/`);
} catch (e: any) {
code = e.code ?? e.name;
}
expect(code).toBe("Malformed_HTTP_Response");
});
});
Loading