fetch: send every request header instead of silently dropping past the 250th

src/http/lib.rs

@@ -1047,9 +1047,16 @@ static PRINT_EVERY_I: AtomicUsize = AtomicUsize::new(0);
 
 // we always rewrite the entire HTTP request when write() returns EAGAIN
 // so we can reuse this buffer
-const MAX_REQUEST_HEADERS: usize = 256;
-static SHARED_REQUEST_HEADERS_BUF: bun_core::RacyCell<[picohttp::Header; MAX_REQUEST_HEADERS]> =
-    bun_core::RacyCell::new([picohttp::Header::ZERO; MAX_REQUEST_HEADERS]);
+const MAX_REQUEST_HEADERS_INLINE: usize = 256;
+static SHARED_REQUEST_HEADERS_BUF: bun_core::RacyCell<
+    [picohttp::Header; MAX_REQUEST_HEADERS_INLINE],
+> = bun_core::RacyCell::new([picohttp::Header::ZERO; MAX_REQUEST_HEADERS_INLINE]);
+
+// Spillover for requests with more than MAX_REQUEST_HEADERS_INLINE header
+// fields. Sized on demand in build_request() from the user header count so
+// every field is written. There is no request-side field-count cap.
+static SHARED_REQUEST_HEADERS_OVERFLOW: bun_core::RacyCell<Vec<picohttp::Header>> =
+    bun_core::RacyCell::new(Vec::new());
 
 // 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]> =
@@ -1071,11 +1078,16 @@ static SINGLE_PACKET_SMALL_BUFFER: bun_core::RacyCell<[u8; 16 * 1024]> =
 mod scratch {
     use super::*;
     #[inline]
-    pub(super) fn request_headers() -> &'static mut [picohttp::Header; MAX_REQUEST_HEADERS] {
+    pub(super) fn request_headers() -> &'static mut [picohttp::Header; MAX_REQUEST_HEADERS_INLINE] {
         // SAFETY: see module-level INVARIANT.
         unsafe { &mut *SHARED_REQUEST_HEADERS_BUF.get() }
     }
     #[inline]
+    pub(super) fn request_headers_overflow() -> &'static mut Vec<picohttp::Header> {
+        // SAFETY: see module-level INVARIANT.
+        unsafe { &mut *SHARED_REQUEST_HEADERS_OVERFLOW.get() }
+    }
+    #[inline]
     pub(super) fn response_headers() -> &'static mut [picohttp::Header; 256] {
         // SAFETY: see module-level INVARIANT.
         unsafe { &mut *SHARED_RESPONSE_HEADERS_BUF.get() }
@@ -2357,7 +2369,18 @@ impl<'a> HTTPClient<'a> {
         let header_entries = self.header_entries.slice();
         let header_names = header_entries.items_name();
         let header_values = header_entries.items_value();
-        let request_headers_buf = scratch::request_headers();
+
+        // Default headers that may be appended after user headers
+        // (Connection, User-Agent, Accept, Host, Accept-Encoding, Content-Length/Transfer-Encoding).
+        const MAX_DEFAULT_HEADERS: usize = 6;
+        let needed = header_names.len() + MAX_DEFAULT_HEADERS;
+        let request_headers_buf: &mut [picohttp::Header] = if needed <= MAX_REQUEST_HEADERS_INLINE {
+            scratch::request_headers().as_mut_slice()
+        } else {
+            let overflow = scratch::request_headers_overflow();
+            overflow.resize(needed, picohttp::Header::ZERO);
+            overflow.as_mut_slice()
+        };
 
         let mut override_accept_encoding = false;
         let mut override_accept_header = false;
@@ -2367,45 +2390,31 @@ impl<'a> HTTPClient<'a> {
         let mut add_transfer_encoding = true;
         let mut original_content_length: Option<&[u8]> = None;
 
-        // Reserve slots for default headers that may be appended after user headers
-        // (Connection, User-Agent, Accept, Host, Accept-Encoding, Content-Length/Transfer-Encoding).
-        const MAX_DEFAULT_HEADERS: usize = 6;
-        const MAX_USER_HEADERS: usize = MAX_REQUEST_HEADERS - MAX_DEFAULT_HEADERS;
-
         for (i, head) in header_names.iter().enumerate() {
             let name = self.header_str(*head);
             // Hash it as lowercase
             let hash = hash_header_name(name);
 
-            // Whether this header will actually be written to the buffer.
-            // Override flags must only be set when the header is kept, otherwise
-            // the default header is suppressed but the user header is dropped,
-            // leaving the header entirely absent from the request.
-            let will_append = header_count < MAX_USER_HEADERS;
-
             // Skip host and connection header
             // we manage those
             match hash {
                 h if h == hash_header_const(b"Content-Length") => {
-                    // Content-Length is always consumed (never written to the buffer).
                     original_content_length = Some(self.header_str(header_values[i]));
                     continue;
                 }
                 h if h == hash_header_const(b"Connection") => {
-                    if will_append {
-                        override_connection_header = true;
-                        let connection_value = self.header_str(header_values[i]);
-                        if bun_core::strings::eql_case_insensitive_ascii_check_length(
-                            connection_value,
-                            b"close",
-                        ) {
-                            self.flags.disable_keepalive = true;
-                        } else if bun_core::strings::eql_case_insensitive_ascii_check_length(
-                            connection_value,
-                            b"keep-alive",
-                        ) {
-                            self.flags.disable_keepalive = false;
-                        }
+                    override_connection_header = true;
+                    let connection_value = self.header_str(header_values[i]);
+                    if bun_core::strings::eql_case_insensitive_ascii_check_length(
+                        connection_value,
+                        b"close",
+                    ) {
+                        self.flags.disable_keepalive = true;
+                    } else if bun_core::strings::eql_case_insensitive_ascii_check_length(
+                        connection_value,
+                        b"keep-alive",
+                    ) {
+                        self.flags.disable_keepalive = false;
                     }
                 }
                 h if h == hash_header_const(b"if-modified-since") => {
@@ -2418,53 +2427,33 @@ impl<'a> HTTPClient<'a> {
                     }
                 }
                 h if h == hash_header_const(HOST_HEADER_NAME) => {
-                    if will_append {
-                        override_host_header = true;
-                    }
+                    override_host_header = true;
                 }
                 h if h == hash_header_const(b"Accept") => {
-                    if will_append {
-                        override_accept_header = true;
-                    }
+                    override_accept_header = true;
                 }
                 h if h == hash_header_const(b"User-Agent") => {
-                    if will_append {
-                        override_user_agent = true;
-                    }
+                    override_user_agent = true;
                 }
                 h if h == hash_header_const(b"Accept-Encoding") => {
-                    if will_append {
-                        override_accept_encoding = true;
-                    }
+                    override_accept_encoding = true;
                 }
                 h if h == hash_header_const(b"Upgrade") => {
-                    if will_append {
-                        let value = self.header_str(header_values[i]);
-                        if !bun_core::strings::eql_any_case_insensitive_ascii(
-                            value,
-                            &[b"h2", b"h2c"],
-                        ) {
-                            self.flags.upgrade_state = HTTPUpgradeState::Pending;
-                        }
+                    let value = self.header_str(header_values[i]);
+                    if !bun_core::strings::eql_any_case_insensitive_ascii(value, &[b"h2", b"h2c"]) {
+                        self.flags.upgrade_state = HTTPUpgradeState::Pending;
                     }
                 }
                 h if h == hash_header_const(CHUNKED_ENCODED_HEADER.name()) => {
                     if !self.flags.is_streaming_request_body {
                         continue;
                     }
                     // We don't want to override chunked encoding header if it was set by the user
-                    if will_append {
-                        add_transfer_encoding = false;
-                    }
+                    add_transfer_encoding = false;
                 }
                 _ => {}
             }
 
-            // Silently drop excess headers to stay within the fixed-size request header buffer.
-            if !will_append {
-                continue;
-            }
-
             request_headers_buf[header_count] =
                 picohttp::Header::new(name, self.header_str(header_values[i]));
 
@@ -2544,15 +2533,17 @@ impl<'a> HTTPClient<'a> {
         // SAFETY: every borrowed slice points into storage that outlives the
         // returned `Request` — `Method::as_str()` is `'static`; `url.pathname`
         // borrows `self.url` (lives for the client); `request_headers_buf` is
-        // the per-HTTP-thread `SHARED_REQUEST_HEADERS_BUF` static. Return as
-        // `'static` so callers don't pin `&mut self` for the rest of their fn.
+        // either the per-HTTP-thread `SHARED_REQUEST_HEADERS_BUF` static or the
+        // `SHARED_REQUEST_HEADERS_OVERFLOW` Vec (buffer moves on resize). Callers
+        // serialize the returned `Request` before the next `build_request()` call
+        // resizes, so the erased borrow never dangles. Return as `'static` so
+        // callers don't pin `&mut self` for the rest of their fn.
         picohttp::Request {
             method: self.method.as_str().as_bytes(),
             // SAFETY: `url.pathname` borrows `self.url`, which outlives the returned `Request`.
             path: unsafe { bun_ptr::detach_lifetime(self.url.pathname) },
             minor_version: 1,
-            // SAFETY: `request_headers_buf` is the per-HTTP-thread
-            // `SHARED_REQUEST_HEADERS_BUF` static, outliving the returned `Request`.
+            // SAFETY: see the block-level comment above.
             headers: unsafe { bun_ptr::detach_lifetime(&request_headers_buf[0..header_count]) },
             bytes_read: 0,
         }

test/js/web/fetch/fetch_headers.test.js

@@ -72,6 +72,43 @@ describe("Headers", async () => {
     }
   });
 
+  it.each([251, 300])("sends every request header field on the wire (%i user headers)", async N => {
+    // build_request() previously capped user headers at 250 (256-slot scratch
+    // minus 6 defaults) and silently dropped the rest; the origin must receive
+    // every field.
+    const { promise: gotHead, resolve, reject } = Promise.withResolvers();
+    const srv = net.createServer(s => {
+      let b = Buffer.alloc(0);
+      s.on("data", d => {
+        b = Buffer.concat([b, d]);
+        if (b.indexOf("\r\n\r\n") >= 0) {
+          resolve(b.toString("latin1").split("\r\n\r\n")[0]);
+          s.end("HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok");
+        }
+      });
+      s.once("error", reject);
+    });
+    srv.listen(0, "127.0.0.1");
+    await once(srv, "listening");
+    try {
+      const port = srv.address().port;
+      const headers = {};
+      for (let i = 0; i < N; i++) headers["x-" + String(i).padStart(4, "0")] = "v";
+      const [res, head] = await Promise.all([fetch(`http://127.0.0.1:${port}/`, { headers }), gotHead]);
+      await res.text();
+      const lines = head.split("\r\n").slice(1);
+      const received = lines.filter(l => l.startsWith("x-")).sort();
+      const expected = Object.keys(headers)
+        .map(k => `${k}: v`)
+        .sort();
+      expect(received.length).toBe(N);
+      expect(received).toEqual(expected);
+      expect(res.status).toBe(200);
+    } finally {
+      await new Promise(r => srv.close(r));
+    }
+  });
+
   it("Invalid values for well-known headers name the header, not its index", () => {
     // The HTTPHeaderName fast path must report the header's name (e.g. 'Location'),
     // not its numeric enum value (e.g. '51').