diff --git a/packages/bun-usockets/src/node_quic_shim.c b/packages/bun-usockets/src/node_quic_shim.c index 05de1583669a..c1d55c09b3e3 100644 --- a/packages/bun-usockets/src/node_quic_shim.c +++ b/packages/bun-usockets/src/node_quic_shim.c @@ -294,19 +294,27 @@ const char *us_nq_hset_pairs(void *hset, size_t *len) { } void us_nq_hset_free(void *hset) { nq_hsi_discard_header_set(hset); } -#define US_NQ_MAX_HEADERS 128 +/* Stack allocation covers node's default maxHeaderPairs; above that we + * spill to the heap. maxHeaderPairs is an inbound decoder limit only + * (node/src/quic/application.cc), so the encoder path imposes no policy cap. */ +#define US_NQ_STACK_HEADERS 128 int us_nq_stream_send_headers(lsquic_stream_t *s, const char *buf, size_t len, int expected, int eos) { - struct lsxpack_header hdrs[US_NQ_MAX_HEADERS]; - int count = 0; + struct lsxpack_header stack_hdrs[US_NQ_STACK_HEADERS]; + struct lsxpack_header *hdrs = stack_hdrs; + int count = 0, rv = -1; size_t i = 0; - if (expected < 0 || expected > US_NQ_MAX_HEADERS) return -1; + if (expected < 0) return -1; + if (expected > US_NQ_STACK_HEADERS) { + hdrs = malloc((size_t) expected * sizeof(*hdrs)); + if (!hdrs) return -1; + } while (i < len) { /* The caller's pair count is authoritative: latin1 encoding of the * NUL-joined buffer maps U+0100-style code points onto the delimiter, * which would otherwise splice extra headers out of one user value * (node/src/node_http_common-inl.h bails the same way on n >= count_). */ - if (count >= expected) return -1; + if (count >= expected) goto done; size_t name_off = i; while (i < len && buf[i]) i++; size_t name_len = i - name_off; @@ -318,19 +326,25 @@ int us_nq_stream_send_headers(lsquic_stream_t *s, const char *buf, size_t len, if (i >= len) break; i++; unsigned char flags = (i < len) ? (unsigned char) buf[i++] : 0; - if (name_len > LSXPACK_MAX_STRLEN || val_len > LSXPACK_MAX_STRLEN) - return -1; - lsxpack_header_set_offset2(&hdrs[count], buf, name_off, name_len, - val_off, val_len); + if (name_len >= LSXPACK_MAX_STRLEN || val_len > LSXPACK_MAX_STRLEN) + goto done; + /* Per-entry base keeps val_offset (= name_len+1) under the setter's + * LSXPACK_MAX_STRLEN assert no matter how large the joined buffer is. */ + lsxpack_header_set_offset2(&hdrs[count], buf + name_off, 0, name_len, + val_off - name_off, val_len); if (flags & 1) { hdrs[count].flags = LSXPACK_NEVER_INDEX; hdrs[count].indexed_type = 2; } count++; } - if (count != expected) return -1; - lsquic_http_headers_t list = { count, hdrs }; - return lsquic_stream_send_headers(s, &list, eos); + if (count == expected) { + lsquic_http_headers_t list = { count, hdrs }; + rv = lsquic_stream_send_headers(s, &list, eos); + } +done: + if (hdrs != stack_hdrs) free(hdrs); + return rv; } static const struct lsquic_stream_if nq_stream_if = { diff --git a/src/runtime/node/quic/stream.rs b/src/runtime/node/quic/stream.rs index efaf487591e2..32e091afc5df 100644 --- a/src/runtime/node/quic/stream.rs +++ b/src/runtime/node/quic/stream.rs @@ -226,13 +226,15 @@ impl QuicStream { let _ = s.set_http_prio(urgency, incremental); } for (bytes, count, eos) in self.pending_headers.with_mut(core::mem::take) { - self.wrote_to_lsquic.set(true); - if s.send_headers(&bytes, count, eos) == 0 && eos { - self.with_state(|st| { - st.fin_sent = 1; - st.write_ended = 1; - }); - s.shutdown(1); + if s.send_headers(&bytes, count, eos) == 0 { + self.wrote_to_lsquic.set(true); + if eos { + self.with_state(|st| { + st.fin_sent = 1; + st.write_ended = 1; + }); + s.shutdown(1); + } } } if uni { diff --git a/test/js/node/quic/quic-stream.test.ts b/test/js/node/quic/quic-stream.test.ts index 1bc9d9e2cdd1..fae192c744d7 100644 --- a/test/js/node/quic/quic-stream.test.ts +++ b/test/js/node/quic/quic-stream.test.ts @@ -178,4 +178,84 @@ describe("HTTP/3 header encoding", () => { expect(seen.length).toBe(1); expect(Object.keys(seen[0])).not.toContain("authorization"); }); + + // `application.maxHeaderPairs` is an inbound decoder limit; the outbound + // encoder path must not impose its own cap. Before the fix, the shim's + // stack buffer rejected any block with >128 field lines, sendHeaders() + // returned false, nothing went on the wire, and the request hung until the + // idle timeout tore down the whole connection. + test("encodes a HEADERS block with more than 128 field lines", async () => { + const CUSTOM = 200; + // Long enough that the NUL-joined encode buffer runs well past 64KB, + // exercising the per-entry lsxpack_header base (not just the count path). + const VAL = Buffer.alloc(400, "a").toString(); + const app = { maxHeaderPairs: 1024, maxHeaderLength: 1 << 20 }; + const serverSaw = Promise.withResolvers>(); + const serverSendOk = Promise.withResolvers(); + await using server = await listen( + async serverSession => { + serverSession.onstream = (stream: any) => { + stream.closed.catch(() => {}); + }; + await serverSession.closed.catch(() => {}); + }, + { + sni: { "*": { keys: [key], certs: [cert] } }, + application: app, + transportParams: { maxIdleTimeout: 1 }, + onheaders(this: any, headers: Record) { + serverSaw.resolve(headers); + const resp: Record = { ":status": "200" }; + for (let i = 0; i < CUSTOM; i++) resp["x-resp-" + i] = String(i); + serverSendOk.resolve(this.sendHeaders(resp)); + this.writer.endSync(); + }, + }, + ); + + // A fresh endpoint so this test's `application` options reach the lsquic + // engine rather than whatever an earlier `connect()` in this file built. + const client = await connect(server.address, { + endpoint: {}, + servername: "localhost", + verifyPeer: "manual", + application: app, + transportParams: { maxIdleTimeout: 1 }, + }); + await client.opened; + + const req: Record = { + ":method": "GET", + ":path": "/", + ":scheme": "https", + ":authority": "localhost", + }; + for (let i = 0; i < CUSTOM; i++) req["x-req-" + i] = VAL; + + const gotHeaders = Promise.withResolvers>(); + const stream = await client.createBidirectionalStream({ + onheaders(headers: Record) { + gotHeaders.resolve(headers); + }, + }); + stream.closed.catch(gotHeaders.reject); + expect(stream.sendHeaders(req, { terminal: true })).toBe(true); + + const seen = await serverSaw.promise; + expect(Object.keys(seen)).toHaveLength(CUSTOM + 4); + expect(seen["x-req-0"]).toBe(VAL); + expect(seen["x-req-" + (CUSTOM - 1)]).toBe(VAL); + // Server direction too: more than 128 response headers. + expect(await serverSendOk.promise).toBe(true); + + const resp = await gotHeaders.promise; + expect(resp[":status"]).toBe("200"); + expect(Object.keys(resp)).toHaveLength(CUSTOM + 1); + expect(resp["x-resp-" + (CUSTOM - 1)]).toBe(String(CUSTOM - 1)); + + for await (const _ of stream) { + } + expect(client.destroyed).toBe(false); + client.close(); + }); });