http2: give HPACK DecodeResult an honest lifetime and copy headers out at the parser boundary

src/http/lshpack.rs

@@ -29,11 +29,12 @@ pub struct HPACK {
     self_: *mut c_void,
 }
 
-pub struct DecodeResult {
-    // TODO: lifetime — name/value point into an FFI thread_local shared buffer,
-    // valid only until the next decode/encode call. Consider `DecodeResult<'a>`.
-    pub name: &'static [u8],
-    pub value: &'static [u8],
+/// One decoded header. `name`/`value` point into a thread-local scratch
+/// buffer shared by every `HPACK` instance on the thread; copy the bytes out
+/// before the next decode/encode through any instance.
+pub struct DecodeResult<'a> {
+    pub name: &'a [u8],
+    pub value: &'a [u8],
     pub never_index: bool,
     pub well_know: u16,
     /// offset of the next header position in src
@@ -54,8 +55,9 @@ bun_core::named_error_set!(HpackError);
 impl HPACK {
     pub const LSHPACK_MAX_HEADER_SIZE: usize = 65536;
 
-    /// DecodeResult name and value uses a thread_local shared buffer and should be copy/cloned before the next decode/encode call
-    pub fn decode(&mut self, src: &[u8]) -> Result<DecodeResult, HpackError> {
+    /// `DecodeResult` name/value point into a thread_local shared buffer;
+    /// copy them out before the next decode/encode call.
+    pub fn decode(&mut self, src: &[u8]) -> Result<DecodeResult<'_>, HpackError> {
         let mut header = lshpack_header::default();
         // SAFETY: genuine FFI — only the `(src.as_ptr(), src.len())` pair carries
         // an obligation here (in-bounds read), discharged by `src: &[u8]`. The

src/runtime/api/bun/h2_frame_parser.rs

@@ -2144,7 +2144,72 @@
     }
 }
 
-type HeaderValue = lshpack::DecodeResult;
+/// Owned copy of one decoded HPACK header. `lshpack::DecodeResult` borrows a
+/// thread-local scratch buffer that the next decode/encode reuses (e.g.
+/// `send_go_away` → `encode` while a decoded header is still in use), so the
+/// bytes are copied out at the decode boundary. Name and value share one
+/// buffer, split at `name_len`; small headers are stored inline.
+pub(crate) struct HeaderValue {
+    data: HeaderValueData,
+    name_len: usize,
+    never_index: bool,
+    well_know: u16,
+    /// offset of the next header position in src
+    next: usize,
+}
+
+/// Inline capacity (name + value combined); typical headers avoid a heap allocation.
+const HEADER_VALUE_INLINE_CAP: usize = 160;
+
+enum HeaderValueData {
+    Inline {
+        buf: [u8; HEADER_VALUE_INLINE_CAP],
+        len: usize,
+    },
+    Heap(Vec<u8>),
+}
+
+impl HeaderValue {
+    fn new(name: &[u8], value: &[u8], never_index: bool, well_know: u16, next: usize) -> Self {
+        let total = name.len() + value.len();
+        let data = if total <= HEADER_VALUE_INLINE_CAP {
+            let mut buf = [0u8; HEADER_VALUE_INLINE_CAP];
+            buf[..name.len()].copy_from_slice(name);
+            buf[name.len()..total].copy_from_slice(value);
+            HeaderValueData::Inline { buf, len: total }
+        } else {
+            let mut data = Vec::with_capacity(total);
+            data.extend_from_slice(name);
+            data.extend_from_slice(value);
+            HeaderValueData::Heap(data)
+        };
+        Self {
+            data,
+            name_len: name.len(),
+            never_index,
+            well_know,
+            next,
+        }
+    }
+
+    #[inline]
+    fn bytes(&self) -> &[u8] {
+        match &self.data {
+            HeaderValueData::Inline { buf, len } => &buf[..*len],
+            HeaderValueData::Heap(data) => data,
+        }
+    }
+
+    #[inline]
+    fn name(&self) -> &[u8] {
+        &self.bytes()[..self.name_len]
+    }
+
+    #[inline]
+    fn value(&self) -> &[u8] {
+        &self.bytes()[self.name_len..]
+    }
+}
 
 // ──────────────────────────────────────────────────────────────────────────
 // H2FrameParser impl — core methods
@@ -2190,7 +2255,15 @@
     pub(crate) fn decode(&self, src_buffer: &[u8]) -> Result<HeaderValue, bun_core::Error> {
         self.hpack.with_mut(|hpack| {
             if let Some(hpack) = hpack.as_mut() {
-                return hpack.decode(src_buffer).map_err(bun_core::Error::from);
+                let decoded = hpack.decode(src_buffer).map_err(bun_core::Error::from)?;
+                // Copy out of the decoder's scratch buffer (see `HeaderValue`).
+                return Ok(HeaderValue::new(
+                    decoded.name,
+                    decoded.value,
+                    decoded.never_index,
+                    decoded.well_know,
+                    decoded.next,
+                ));
             }
             Err(bun_core::err!("UnableToDecode"))
         })
@@ -3329,10 +3402,10 @@
             bun_output::scoped_log!(
                 H2FrameParser,
                 "header {} {}",
-                BStr::new(header.name),
-                BStr::new(header.value)
+                BStr::new(header.name()),
+                BStr::new(header.value())
             );
-            if self.is_server.get() && header.name == b":status" {
+            if self.is_server.get() && header.name() == b":status" {
                 self.send_go_away(
                     stream_id,
                     ErrorCode::PROTOCOL_ERROR,
@@ -3346,7 +3419,7 @@
             // RFC 7540 Section 6.5.2: Calculate header list size
             // Size = name length + value length + HPACK entry overhead per header
             stream.header_block_size +=
-                header.name.len() + header.value.len() + HPACK_ENTRY_OVERHEAD;
+                header.name().len() + header.value().len() + HPACK_ENTRY_OVERHEAD;
             stream.header_block_count += 1;
 
             // Check against maxHeaderListSize / maxHeaderListPairs.
@@ -3359,16 +3432,16 @@
                 continue;
             }
 
-            let is_pseudo_header = header.name.first() == Some(&b':');
+            let is_pseudo_header = header.name().first() == Some(&b':');
             if is_pseudo_header {
                 if seen_regular_header {
                     malformed = true;
                 }
             } else {
                 seen_regular_header = true;
             }
-            if is_pseudo_header || header.name == b"content-length" {
-                if let Some(idx) = single_value_headers_index_of(header.name) {
+            if is_pseudo_header || header.name() == b"content-length" {
+                if let Some(idx) = single_value_headers_index_of(header.name()) {
                     if single_value_headers[idx] {
                         malformed = true;
                     }
@@ -3377,14 +3450,14 @@
             }
 
             if malformed
-                || is_malformed_field_name(header.name)
-                || is_malformed_field_value(header.value)
-                || is_forbidden_connection_specific_header(header.name, header.value)
+                || is_malformed_field_name(header.name())
+                || is_malformed_field_value(header.value())
+                || is_forbidden_connection_specific_header(header.name(), header.value())
                 || (is_pseudo_header
                     && !if self.is_server.get() {
-                        is_valid_request_pseudo_header(header.name)
+                        is_valid_request_pseudo_header(header.name())
                     } else {
-                        is_valid_response_pseudo_header(header.name)
+                        is_valid_response_pseudo_header(header.name())
                     })
             {
                 malformed = true;
@@ -3394,7 +3467,7 @@
                 headers.push(&global_object, js_header_name)?;
                 headers.push(
                     &global_object,
-                    bun_jsc::bun_string_jsc::create_utf8_for_js(&global_object, header.value)?,
+                    bun_jsc::bun_string_jsc::create_utf8_for_js(&global_object, header.value())?,
                 )?;
                 if header.never_index {
                     if sensitive_headers.is_undefined() {
@@ -3405,9 +3478,9 @@
                 }
             } else {
                 let js_header_name =
-                    bun_jsc::bun_string_jsc::create_utf8_for_js(&global_object, header.name)?;
+                    bun_jsc::bun_string_jsc::create_utf8_for_js(&global_object, header.name())?;
                 let js_header_value =
-                    bun_jsc::bun_string_jsc::create_utf8_for_js(&global_object, header.value)?;
+                    bun_jsc::bun_string_jsc::create_utf8_for_js(&global_object, header.value())?;
 
                 if header.never_index {
                     if sensitive_headers.is_undefined() {

test/js/node/http2/node-http2.test.js

@@ -2711,3 +2711,142 @@ it("http2 client keeps parsing a socket chunk whose ArrayBuffer is transferred b
   expect(stdout).toContain('PINGS:["4141414141414141","4242424242424242"]');
   expect(exitCode).toBe(0);
 });
+
+it("http2 server sends GOAWAY PROTOCOL_ERROR when a request header block contains :status", async () => {
+  // A response pseudo-header in a request triggers a GOAWAY (an encode)
+  // while the freshly decoded header is still in use.
+  const delivered = [];
+  const server = http2.createServer();
+  server.on("stream", (stream, headers) => {
+    delivered.push(headers);
+    stream.respond({ ":status": 200 });
+    stream.end("ok");
+  });
+  const { promise: listening, resolve: onListening } = Promise.withResolvers();
+  server.listen(0, "127.0.0.1", onListening);
+  await listening;
+  const port = server.address().port;
+
+  const frames = [];
+  const { promise: exchanged, resolve: onExchanged, reject: onSocketError } = Promise.withResolvers();
+  const socket = net.connect(port, "127.0.0.1", () => {
+    socket.write(http2utils.kClientMagic);
+    socket.write(new http2utils.SettingsFrame(false).data);
+    // HEADERS on stream 1 with END_HEADERS | END_STREAM. 0x88 is the fully
+    // indexed static-table entry ":status: 200".
+    socket.write(new http2utils.HeadersFrame(1, Buffer.from([0x88]), 0, true, true).data);
+  });
+  socket.on("error", onSocketError);
+  let received = Buffer.alloc(0);
+  socket.on("data", chunk => {
+    received = Buffer.concat([received, chunk]);
+    while (received.length >= 9) {
+      const length = received.readUIntBE(0, 3);
+      if (received.length < 9 + length) break;
+      const frame = {
+        type: received[3],
+        streamId: received.readUInt32BE(5) & 0x7fffffff,
+        payload: Buffer.from(received.subarray(9, 9 + length)),
+      };
+      received = received.subarray(9 + length);
+      frames.push(frame);
+      if (frame.type === 7) {
+        onExchanged();
+        return;
+      }
+    }
+  });
+  socket.on("close", () => onExchanged());
+
+  try {
+    await exchanged;
+    const goaway = frames.find(f => f.type === 7);
+    expect(goaway).toBeDefined();
+    // GOAWAY payload: last stream id (4 bytes), error code (4 bytes), debug data.
+    expect(goaway.payload.readUInt32BE(4)).toBe(http2.constants.NGHTTP2_PROTOCOL_ERROR);
+    expect(goaway.payload.subarray(8).toString()).toContain(":status");
+    // The malformed request never reaches the application.
+    expect(delivered).toEqual([]);
+  } finally {
+    socket.destroy();
+    server.close();
+  }
+});
+
+it("http2 decoded header bytes stay intact across interleaved decode/encode and GC", async () => {
+  // Server and client share one thread, so each request/response interleaves
+  // HPACK decodes and encodes on the same thread-local scratch buffer.
+  await using proc = Bun.spawn({
+    cmd: [
+      bunExe(),
+      "-e",
+      `
+      const http2 = require("node:http2");
+      const assert = require("node:assert");
+
+      const server = http2.createServer();
+      server.on("stream", (stream, headers) => {
+        const res = { ":status": 200 };
+        for (const name in headers) {
+          if (name.startsWith("x-")) res["e-" + name] = headers[name];
+        }
+        res[http2.sensitiveHeaders] = ["e-x-secret"];
+        stream.respond(res);
+        stream.end("done");
+      });
+      server.listen(0, "127.0.0.1", async () => {
+        try {
+          const port = server.address().port;
+          const client = http2.connect("http://127.0.0.1:" + port);
+          client.on("error", err => {
+            console.error(err);
+            process.exit(1);
+          });
+          const pSuffix = Buffer.alloc(64, "p").toString();
+          const qSuffix = Buffer.alloc(48, "q").toString();
+          for (let i = 0; i < 40; i++) {
+            const reqHeaders = { ":path": "/" + i };
+            for (let j = 0; j < 12; j++) {
+              // Fixed values are HPACK dynamic-table indexed after round 0;
+              // varying values are literal every round.
+              reqHeaders["x-fixed-" + j] = "value-" + j + "-" + pSuffix;
+              reqHeaders["x-vary-" + j] = i + "-" + j + "-" + qSuffix;
+            }
+            reqHeaders["x-secret"] = "hunter2-" + i;
+            reqHeaders[http2.sensitiveHeaders] = ["x-secret"];
+            const resHeaders = await new Promise((resolve, reject) => {
+              const req = client.request(reqHeaders);
+              req.on("response", resolve);
+              req.on("error", reject);
+              req.resume();
+              req.end();
+            });
+            assert.strictEqual(resHeaders[":status"], 200);
+            for (let j = 0; j < 12; j++) {
+              assert.strictEqual(resHeaders["e-x-fixed-" + j], reqHeaders["x-fixed-" + j]);
+              assert.strictEqual(resHeaders["e-x-vary-" + j], reqHeaders["x-vary-" + j]);
+            }
+            assert.strictEqual(resHeaders["e-x-secret"], "hunter2-" + i);
+            assert.ok(resHeaders[http2.sensitiveHeaders].includes("e-x-secret"));
+            if (i % 8 === 0) Bun.gc(true);
+          }
+          client.close();
+          server.close();
+          console.log("OK");
+        } catch (err) {
+          console.error(err);
+          process.exit(1);
+        }
+      });
+      `,
+    ],
+    env: bunEnv,
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+  expect(stdout, `subprocess stderr:\n${stderr}`).toContain("OK");
+  expect(exitCode).toBe(0);
+});