diff --git a/src/http/InternalState.rs b/src/http/InternalState.rs index 96e6aee2fce4..55b58a342177 100644 --- a/src/http/InternalState.rs +++ b/src/http/InternalState.rs @@ -27,7 +27,6 @@ pub struct InternalState<'a> { pub transfer_encoding: Encoding, pub encoding: Encoding, - pub content_encoding_i: u8, pub chunked_decoder: bun_picohttp::phr_chunked_decoder, pub decompressor: Decompressor, pub stage: Stage, @@ -65,7 +64,6 @@ pub struct InternalState<'a> { pub struct InternalStateFlags { pub allow_keepalive: bool, pub received_last_chunk: bool, - pub did_set_content_encoding: bool, pub is_redirect_pending: bool, pub is_libdeflate_fast_path_disabled: bool, pub resend_request_body_on_redirect: bool, @@ -98,7 +96,6 @@ impl InternalStateFlags { Self { allow_keepalive: true, received_last_chunk: false, - did_set_content_encoding: false, is_redirect_pending: false, is_libdeflate_fast_path_disabled: false, resend_request_body_on_redirect: false, @@ -126,7 +123,6 @@ impl Default for InternalState<'_> { flags: InternalStateFlags::new(), transfer_encoding: Encoding::Identity, encoding: Encoding::Identity, - content_encoding_i: u8::MAX, chunked_decoder: bun_picohttp::phr_chunked_decoder::default(), decompressor: Decompressor::None, stage: Stage::Pending, diff --git a/src/http/lib.rs b/src/http/lib.rs index 73324013b770..0e5f9554ad8a 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -1048,8 +1048,16 @@ static SHARED_REQUEST_HEADERS_BUF: bun_core::RacyCell<[picohttp::Header; MAX_REQ 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 header-block line count, so the 1 MB byte +// cap (MAX_RESPONSE_HEADER_BUFFER) on the header block bounds the slot count. +static SHARED_RESPONSE_HEADERS_OVERFLOW: bun_core::RacyCell> = + bun_core::RacyCell::new(Vec::new()); // the first packet for Transfer-Encoding: chunked // is usually pretty small or sometimes even just a length @@ -1072,11 +1080,17 @@ mod scratch { 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 { + // 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() } @@ -3729,12 +3743,34 @@ impl<'a> HTTPClient<'a> { 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 header-block line count (a strict + // upper bound on the field count) and reparse. Count only up + // to the terminating CRLF CRLF when present so a newline-dense + // body in the same read doesn't inflate the slot count. + let buf = to_read!(); + let header_end = bun_core::strings::index_of(buf, b"\r\n\r\n") + .map(|i| i + 4) + .unwrap_or(buf.len()); + let needed = bun_core::strings::count_char(&buf[..header_end], b'\n'); + let overflow = scratch::response_headers_overflow(); + overflow.resize(needed, picohttp::Header::ZERO); + parse_result = picohttp::Response::parse_parts( + to_read!(), + overflow.as_mut_slice(), + Some(&mut amount_read), + ); + } + let response = match parse_result { Ok(r) => r, Err(picohttp::ParseResponseError::ShortRead) => { // `MAX_HTTP_HEADER_SIZE` (default 16 KB) is the *server*/ @@ -3767,8 +3803,12 @@ impl<'a> HTTPClient<'a> { }; // we save the successful parsed response - // SAFETY: response borrows SHARED_RESPONSE_HEADERS_BUF / response_message_buffer, - // both of which outlive this fn; widen to 'static for storage. + // SAFETY: `response` borrows SHARED_RESPONSE_HEADERS_BUF (fixed + // static) or SHARED_RESPONSE_HEADERS_OVERFLOW (Vec; buffer moves on + // resize) plus response_message_buffer / incoming_data. The erased + // borrow is overwritten with `Response::default()` at the top of + // each loop iteration before the next resize/append, and + // `clone_metadata()` deep-copies before this fn returns. // Rebind `response` to the detached `'static` copy so it no longer // borrows `to_read` (lets the `to_read` reassignment below pass // borrowck — `RawSlice::slice` ties output to `&to_read`). @@ -3827,20 +3867,9 @@ impl<'a> HTTPClient<'a> { } }; // handle_response_metadata may mutate `response`; mirror it back so - // clone_metadata() sees the up-to-date headers regardless of the - // content-encoding branch below. + // clone_metadata() sees the up-to-date headers. self.state.pending_response = Some(response); - if (self.state.content_encoding_i as usize) < response.headers.list.len() - && !self.state.flags.did_set_content_encoding - { - // if it compressed with this header, it is no longer because we will decompress it - self.state.flags.did_set_content_encoding = true; - self.state.content_encoding_i = u8::MAX; - // we need to reset the pending response because we removed a header - self.state.pending_response = Some(response); - } - if should_continue == ShouldContinue::Finished { if self.state.flags.is_redirect_pending { self.do_redirect::(ctx, socket); @@ -4997,7 +5026,7 @@ impl<'a> HTTPClient<'a> { let mut location: &[u8] = b""; let mut pretend_304 = false; let mut is_server_sent_events = false; - for (header_i, header) in response.headers.list.iter().enumerate() { + for header in response.headers.list.iter() { match hash_header_name(header.name()) { h if h == hash_header_const(b"Content-Length") => { // RFC 9110 section 9.3.6: a client MUST ignore @@ -5049,18 +5078,14 @@ impl<'a> HTTPClient<'a> { || strings::eql_case_insensitive_ascii_check_length(value, b"x-gzip") { self.state.encoding = Encoding::Gzip; - self.state.content_encoding_i = header_i as u8; } else if strings::eql_case_insensitive_ascii_check_length( value, b"deflate", ) { self.state.encoding = Encoding::Deflate; - self.state.content_encoding_i = header_i as u8; } else if strings::eql_case_insensitive_ascii_check_length(value, b"br") { self.state.encoding = Encoding::Brotli; - self.state.content_encoding_i = header_i as u8; } else if strings::eql_case_insensitive_ascii_check_length(value, b"zstd") { self.state.encoding = Encoding::Zstd; - self.state.content_encoding_i = header_i as u8; } } } diff --git a/src/http_jsc/websocket_client/WebSocketUpgradeClient.rs b/src/http_jsc/websocket_client/WebSocketUpgradeClient.rs index bdb85a240d4b..06f8152a0a0f 100644 --- a/src/http_jsc/websocket_client/WebSocketUpgradeClient.rs +++ b/src/http_jsc/websocket_client/WebSocketUpgradeClient.rs @@ -927,7 +927,10 @@ impl HTTPClient { 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; @@ -986,7 +989,10 @@ impl HTTPClient { // 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; @@ -1227,7 +1233,10 @@ impl HTTPClient { 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; diff --git a/src/picohttp/lib.rs b/src/picohttp/lib.rs index 4917de2b390f..4dc4eefbca2b 100644 --- a/src/picohttp/lib.rs +++ b/src/picohttp/lib.rs @@ -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); @@ -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) } diff --git a/test/js/bun/http/fetch-header-count-limit.test.ts b/test/js/bun/http/fetch-header-count-limit.test.ts index c12684890953..e5221da062c7 100644 --- a/test/js/bun/http/fetch-header-count-limit.test.ts +++ b/test/js/bun/http/fetch-header-count-limit.test.ts @@ -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"; @@ -105,3 +105,71 @@ 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("newline-dense body in the same write as a >256-field header block", async () => { + // The overflow scratch is sized from the header-block line count, not the + // body, so a body full of LFs must not affect parsing. + const body = Buffer.alloc(8192, "\n").toString(); + await using server = createServer(socket => { + socket.on("error", () => {}); + socket.once("data", () => { + let head = "HTTP/1.1 200 OK\r\n"; + for (let i = 0; i < 300; i++) head += `X-H-${i}: v${i}\r\n`; + socket.end(head + `Content-Length: ${body.length}\r\nConnection: close\r\n\r\n` + body); + }); + }).listen(0, "127.0.0.1"); + await once(server, "listening"); + 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(body); + expect(res.headers.get("x-h-299")).toBe("v299"); + }); + + 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"); + }); +});