http3: accept a QPACK field section with no field lines; node:quic sends none for empty hints, or for empty trailers after headers

patches/lsqpack/accept-empty-field-section.patch

@@ -0,0 +1,15 @@
+--- a/lsqpack.c
++++ b/lsqpack.c
+@@ -4039,7 +4039,11 @@
+                 read_ctx->hbrc_parse = parse_header_data;
+                 read_ctx->hbrc_parse_ctx_u.data.state
+                                                 = DATA_STATE_NEXT_INSTRUCTION;
+-                if (end - buf)
++                /* bun: a field section can end at its prefix (RFC 9204
++                 * Section 4.5).  LQRHS_NEED here becomes LQRHS_ERROR in
++                 * qdec_read_header(), so let parse_header_data() finish it.
++                 */
++                if (end - buf || read_ctx->hbrc_size == 0)
+                     return parse_header_data(dec, read_ctx, buf, end - buf);
+                 else
+                     return LQRHS_NEED;

scripts/build/deps/lsqpack.ts

@@ -18,9 +18,15 @@ export const lsqpack: Dependency = {
     commit: LSQPACK_COMMIT,
   }),
 
-  // QPACK uses the same RFC 7541 Huffman code as HPACK; share lshpack's
-  // .bss-backed hencs/hdecs instead of carrying a second 768 KB copy.
-  patches: ["patches/lsqpack/bss-huff-tables.patch"],
+  patches: [
+    // QPACK uses the same RFC 7541 Huffman code as HPACK; share lshpack's
+    // .bss-backed hencs/hdecs instead of carrying a second 768 KB copy.
+    "patches/lsqpack/bss-huff-tables.patch",
+    // Decode a field section that has no field lines (RFC 9204 Section 4.5)
+    // instead of failing it, which closes the connection. Not fixed upstream:
+    // https://github.com/litespeedtech/ls-qpack/issues/90#issuecomment-5472689782
+    "patches/lsqpack/accept-empty-field-section.patch",
+  ],
 
   // lsqpack.c is compiled inside the lsquic dep because lsquic feeds it a
   // non-FILE* logger context that only works with LSQPACK_*_LOGGER_HEADER

src/runtime/node/quic/stream.rs

@@ -127,9 +127,12 @@ pub(crate) struct QuicStream {
     peer_stop_sending_code: Cell<Option<u64>>,
     wrote_to_lsquic: Cell<bool>,
     headers_received: Cell<bool>,
+    /// lsquic took a header block for this stream.
+    headers_sent: Cell<bool>,
     /// RFC 9218 (urgency, incremental).
     priority: Cell<(u8, bool)>,
-    pending_headers: JsCell<Vec<(Vec<u8>, c_int, bool)>>,
+    /// (nul-joined pairs, pair count, eos, frameless); see `send_headers`.
+    pending_headers: JsCell<Vec<(Vec<u8>, c_int, bool, bool)>>,
     trailers_requested: Cell<bool>,
     blocked_reported: Cell<bool>,
     announce_suppressed: Cell<bool>,
@@ -162,6 +165,7 @@ impl QuicStream {
             peer_stop_sending_code: Cell::new(None),
             wrote_to_lsquic: Cell::new(false),
             headers_received: Cell::new(false),
+            headers_sent: Cell::new(false),
             priority: Cell::new(DEFAULT_PRIORITY),
             pending_headers: JsCell::new(Vec::new()),
             trailers_requested: Cell::new(false),
@@ -245,9 +249,9 @@ impl QuicStream {
         // drain_outbound needs a write event, which nothing else requests
         // unless the JS side already started the writer.
         let mut want_write = false;
-        for (bytes, count, eos) in self.pending_headers.with_mut(core::mem::take) {
+        for (bytes, count, eos, frameless) in self.pending_headers.with_mut(core::mem::take) {
             self.wrote_to_lsquic.set(true);
-            if s.send_headers(&bytes, count, eos) == 0 {
+            if self.put_header_block(s, &bytes, count, eos, frameless) {
                 if eos {
                     self.with_state(|st| {
                         st.fin_sent = 1;
@@ -296,6 +300,30 @@ impl QuicStream {
         unsafe { lsquic::Stream::from_raw(self.raw.get()) }
     }
 
+    /// Sends one header block, or leaves out a `frameless` one. Returns whether the stream took it.
+    fn put_header_block(
+        &self,
+        s: lsquic::Stream,
+        bytes: &[u8],
+        count: c_int,
+        eos: bool,
+        frameless: bool,
+    ) -> bool {
+        // lsquic resets a request stream whose FIN comes before a header block, so `eos` needs one.
+        if frameless
+            && (!eos || self.headers_sent.get())
+            && self.stream_id() & STREAM_ID_UNI_BIT == 0
+            && self.with_state(|s| s.fin_sent == 0 && s.write_ended == 0)
+        {
+            return true;
+        }
+        let taken = s.send_headers(bytes, count, eos) == 0;
+        if taken {
+            self.headers_sent.set(true);
+        }
+        taken
+    }
+
     fn state_mut(&self) -> *mut StreamState {
         self.state.get()
     }
@@ -977,18 +1005,18 @@ impl QuicStream {
         use crate::webcore::encoding::BunStringEncode as _;
         let bytes = bun_core::String::from_js(header_string, global)?
             .encode(crate::node::types::Encoding::Latin1);
-        let is_trailing =
-            kind_arg.coerce_to_i32(global)? as u32 == QUIC_STREAM_HEADERS_KIND_TRAILING;
-        let eos = is_trailing
+        let kind = kind_arg.coerce_to_i32(global)? as u32;
+        let eos = kind == QUIC_STREAM_HEADERS_KIND_TRAILING
             || flags.coerce_to_i32(global)? & (QUIC_STREAM_HEADERS_FLAGS_TERMINAL as i32) != 0;
+        // An unpatched ls-qpack peer closes the connection on a HEADERS frame with no field lines.
+        let frameless = header_count == 0 && kind != QUIC_STREAM_HEADERS_KIND_INITIAL;
         let Some(s) = self.ls() else {
             self.pending_headers
-                .with_mut(|q| q.push((bytes, header_count, eos)));
+                .with_mut(|q| q.push((bytes, header_count, eos, frameless)));
             self.with_state(|s| s.has_outbound = 1);
             return Ok(JSValue::js_boolean(true));
         };
-        let rv = s.send_headers(&bytes, header_count, eos);
-        if rv == 0 {
+        if self.put_header_block(s, &bytes, header_count, eos, frameless) {
             self.wrote_to_lsquic.set(true);
             self.with_state(|s| s.has_outbound = 1);
             if eos {

test/js/bun/http/serve-http3.test.ts

@@ -1925,6 +1925,95 @@ describe("Bun.serve HTTP/3 request validation", () => {
     });
   });
 
+  // A field section with no field lines is valid QPACK (RFC 9204 section 4.5):
+  // the block is only its two-byte prefix. A QPACK decoder failure closes the
+  // connection, so the held response is complete only if the block decoded.
+  // `sendHeaders({})` is what writes the block: the node:quic server in
+  // test/js/node/quic/quic-stream.test.ts reports the same call as empty trailers.
+  test.each([
+    [
+      "after the request headers",
+      (stream: any) => {
+        stream.sendHeaders({ ...requestHeaders("/subject"), ":method": "POST" });
+        stream.sendHeaders({}, { terminal: true });
+      },
+      "200 hello",
+    ],
+    [
+      "after the request body",
+      (stream: any) => {
+        stream.sendHeaders({ ...requestHeaders("/subject"), ":method": "POST" });
+        stream.onwanttrailers = () => stream.sendHeaders({}, { terminal: true });
+        stream.writer.writeSync(new TextEncoder().encode("abc"));
+        stream.writer.endSync();
+      },
+      "200 helloabc",
+    ],
+    // A request with no pseudo-header fields is malformed. Its stream gets a
+    // client error status. Which one is not the subject of this test.
+    [
+      "as the first block of the request",
+      (stream: any) => stream.sendHeaders({}, { terminal: true }),
+      expect.stringMatching(/^4\d\d /),
+    ],
+  ])("decodes a header block with no field lines %s", async (_, send, subject) => {
+    const release = Promise.withResolvers<void>();
+    await using server = Bun.serve({
+      port: 0,
+      tls,
+      http3: true,
+      async fetch(req) {
+        if (new URL(req.url).pathname !== "/held") return new Response("hello" + (await req.text()));
+        return new Response(
+          new ReadableStream({
+            async start(controller) {
+              controller.enqueue("first,");
+              await release.promise;
+              controller.enqueue("second");
+              controller.close();
+            },
+          }),
+        );
+      },
+    });
+
+    await using endpoint = new QuicEndpoint();
+    await using client = await connect(`127.0.0.1:${server.port}`, {
+      endpoint,
+      servername: "localhost",
+      verifyPeer: "manual",
+      transportParams: { maxIdleTimeout: 5 },
+      onerror() {},
+    } as SessionOptions);
+    await client.opened;
+    // "<status> <body>", or the error code when the stream does not end cleanly.
+    const exchange = async (options: Record<string, unknown>, write?: (stream: any) => void) => {
+      let status = "";
+      const stream = await client.createBidirectionalStream({
+        ...options,
+        onheaders(received) {
+          status = received[":status"] as string;
+        },
+      });
+      stream.closed.catch(() => {});
+      write?.(stream);
+      let body = "";
+      try {
+        for await (const batch of stream as AsyncIterable<Uint8Array[]>) {
+          for (const chunk of batch) body += Buffer.from(chunk).toString("latin1");
+        }
+      } catch (error: any) {
+        return `${error.code}`;
+      }
+      return `${status} ${body}`;
+    };
+
+    const held = exchange({ headers: requestHeaders("/held") });
+    const subjectResponse = await exchange({}, send);
+    release.resolve();
+    expect({ subject: subjectResponse, held: await held }).toEqual({ subject, held: "200 first,second" });
+  });
+
   test("requestCert with rejectUnauthorized only serves QUIC clients whose certificate chains to the configured CA", async () => {
     const keysDir = join(import.meta.dir, "..", "..", "node", "test", "fixtures", "keys");
     const pem = (name: string) => readFileSync(join(keysDir, name), "utf8");