From 2bafe8ecb95581bca461800671aec14e527f33de Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 18 Oct 2026 15:46:48 +0000 Subject: [PATCH 1/2] node:quic: destroy the stream when its pendingTrailers cannot be sent [kTrailers] takes the text of Node v26.10.0 (nodejs/node#65863). A throw from sendTrailers() no longer leaves the native callback, and a false answer is no longer dropped: both destroy the stream, so closed rejects and the response ends. --- src/js/internal/quic/quic.ts | 13 ++- test/js/node/quic/quic-stream.test.ts | 110 ++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 1 deletion(-) diff --git a/src/js/internal/quic/quic.ts b/src/js/internal/quic/quic.ts index dc474e25e4f3..eac7ab58868a 100644 --- a/src/js/internal/quic/quic.ts +++ b/src/js/internal/quic/quic.ts @@ -2609,7 +2609,18 @@ class QuicStream { const inner = this.#inner; if (inner.pendingTrailers) { - this.sendTrailers(inner.pendingTrailers); + // Node v26.10.0: https://github.com/nodejs/node/blob/v26.10.0/lib/internal/quic/quic.js#L2794-L2807 + let sent; + try { + sent = this.sendTrailers(inner.pendingTrailers); + } catch (error) { + this.destroy(error); + return; + } + if (!sent) { + this.destroy(ERR_QUIC_STREAM_ABORTED("Failed to submit trailing headers")); + return; + } inner.pendingTrailers = undefined; } else { const { onwanttrailers } = inner; diff --git a/test/js/node/quic/quic-stream.test.ts b/test/js/node/quic/quic-stream.test.ts index fe3a7b08b396..da8ffd8f405b 100644 --- a/test/js/node/quic/quic-stream.test.ts +++ b/test/js/node/quic/quic-stream.test.ts @@ -284,6 +284,116 @@ describe("HTTP/3 header blocks that follow another header block", () => { }); }); +// A stream sends `pendingTrailers` itself when the body is out, so the caller +// has no answer to read. Node v26.10.0 destroys the stream when the block +// cannot be sent (nodejs/node#65863). +describe("pendingTrailers that cannot be sent", () => { + const encoder = new TextEncoder(); + const request = { ":method": "POST", ":scheme": "https", ":authority": "localhost" }; + // Longer than a test: an idle timeout must not be what ends the session. + const transportParams = { maxIdleTimeout: 60 }; + + // One response with a body and `trailers` as its pendingTrailers. Gives how + // `closed` settled for the server stream, and the body that the client read. + async function respondWith(trailers: Record) { + let closed = "open"; + const server = await listen( + async serverSession => { + serverSession.onstream = (stream: any) => { + stream.closed.catch(() => {}); + }; + await serverSession.closed.catch(() => {}); + }, + { + sni: { "*": { keys: [key], certs: [cert] } }, + transportParams, + onheaders(this: any, headers: Record) { + if (headers[":path"] !== "/trailers") { + this.sendHeaders({ ":status": "200" }, { terminal: true }); + return; + } + this.closed.then( + () => (closed = "resolved"), + (err: any) => (closed = err.code), + ); + this.sendHeaders({ ":status": "200" }); + this.pendingTrailers = trailers; + this.writer.writeSync(encoder.encode("body")); + this.writer.endSync(); + }, + // A stream is asked for its trailers only when this is set. + onwanttrailers() {}, + }, + ); + let client: any; + // Not a graceful close. That waits for every stream, and a stream stays + // open when its trailers are never sent. + const dispose = async () => { + client?.destroy(); + await server.destroy(); + }; + try { + client = await connect(server.address!, { + servername: "localhost", + verifyPeer: "manual", + transportParams, + } as SessionOptions); + await client.opened; + + const stream = await client.createBidirectionalStream({ + headers: { ...request, ":path": "/trailers" }, + onheaders() {}, + }); + stream.closed.catch(() => {}); + const arrived = Promise.withResolvers(); + const body = (async () => { + let text = ""; + for await (const batch of stream) { + for (const chunk of batch) text += Buffer.from(chunk).toString("latin1"); + if (text === "body") arrived.resolve(); + } + arrived.resolve(); + return text; + })(); + body.catch(arrived.reject); + await arrived.promise; + + // The server asks the stream for its trailers when the body leaves, which + // is before it gets this request. So the answer comes after that. + const answered = Promise.withResolvers(); + const later = await client.createBidirectionalStream({ + headers: { ...request, ":path": "/" }, + onheaders() { + answered.resolve(); + }, + }); + later.closed.then(() => answered.reject(new Error("the stream closed with no answer")), answered.reject); + await answered.promise; + return { closed, body, [Symbol.asyncDispose]: dispose }; + } catch (err) { + await dispose(); + throw err; + } + } + + test("a block that sendTrailers() throws for destroys the stream with the error", async () => { + await using response = await respondWith({ connection: "close" }); + // destroy() still ends a response that the stream wrote and ended. + expect({ closed: response.closed, body: await response.body }).toEqual({ + closed: "ERR_HTTP2_INVALID_CONNECTION_HEADERS", + body: "body", + }); + }); + + test("a block that sendTrailers() answers false for destroys the stream", async () => { + // Latin-1 on the wire makes a NUL of U+0100. The block then has more + // pairs than it declares, and the native side refuses it. + await using response = await respondWith({ "x-bad": "a\u0100b" }); + response.body.catch(() => {}); + expect(response.closed).toBe("ERR_QUIC_STREAM_ABORTED"); + }); +}); + describe("verifyClient", () => { test("a server requiring a client certificate never surfaces streams from a client that presented none", async () => { let announcedStreams = 0; From e7121bd3b1d155a74a6fb05e4c6973c5c72cb6f9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 18 Oct 2026 17:16:51 +0000 Subject: [PATCH 2/2] node:quic: test a pendingTrailers block that is sent A new test sets a valid pendingTrailers block. It checks that the client gets the trailers and the body, and that the server stream closes with no error. The test for a block that the native side refuses now waits for the client's read to end. The helper no longer fails when that read ends with an error, because the native side decides how it ends. --- test/js/node/quic/quic-stream.test.ts | 65 +++++++++++++++++++-------- 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/test/js/node/quic/quic-stream.test.ts b/test/js/node/quic/quic-stream.test.ts index da8ffd8f405b..eb1946dc857c 100644 --- a/test/js/node/quic/quic-stream.test.ts +++ b/test/js/node/quic/quic-stream.test.ts @@ -287,16 +287,21 @@ describe("HTTP/3 header blocks that follow another header block", () => { // A stream sends `pendingTrailers` itself when the body is out, so the caller // has no answer to read. Node v26.10.0 destroys the stream when the block // cannot be sent (nodejs/node#65863). -describe("pendingTrailers that cannot be sent", () => { +describe("pendingTrailers", () => { const encoder = new TextEncoder(); const request = { ":method": "POST", ":scheme": "https", ":authority": "localhost" }; // Longer than a test: an idle timeout must not be what ends the session. const transportParams = { maxIdleTimeout: 60 }; // One response with a body and `trailers` as its pendingTrailers. Gives how - // `closed` settled for the server stream, and the body that the client read. + // `closed` settles for the server stream, and what the client gets. async function respondWith(trailers: Record) { let closed = "open"; + const settled = Promise.withResolvers(); + const settle = (how: string) => { + closed = how; + settled.resolve(how); + }; const server = await listen( async serverSession => { serverSession.onstream = (stream: any) => { @@ -307,14 +312,14 @@ describe("pendingTrailers that cannot be sent", () => { { sni: { "*": { keys: [key], certs: [cert] } }, transportParams, - onheaders(this: any, headers: Record) { + onheaders(this: any, headers: Record) { if (headers[":path"] !== "/trailers") { this.sendHeaders({ ":status": "200" }, { terminal: true }); return; } this.closed.then( - () => (closed = "resolved"), - (err: any) => (closed = err.code), + () => settle("resolved"), + (err: any) => settle(err.code), ); this.sendHeaders({ ":status": "200" }); this.pendingTrailers = trailers; @@ -340,22 +345,32 @@ describe("pendingTrailers that cannot be sent", () => { } as SessionOptions); await client.opened; + const clientTrailers = Promise.withResolvers>(); const stream = await client.createBidirectionalStream({ headers: { ...request, ":path": "/trailers" }, onheaders() {}, + ontrailers: clientTrailers.resolve, }); - stream.closed.catch(() => {}); + // A stream gets its trailers before it closes. + stream.closed.then( + () => clientTrailers.reject(new Error("the stream closed with no trailers")), + clientTrailers.reject, + ); + clientTrailers.promise.catch(() => {}); const arrived = Promise.withResolvers(); const body = (async () => { let text = ""; - for await (const batch of stream) { - for (const chunk of batch) text += Buffer.from(chunk).toString("latin1"); - if (text === "body") arrived.resolve(); + try { + for await (const batch of stream) { + for (const chunk of batch) text += Buffer.from(chunk).toString("latin1"); + if (text === "body") arrived.resolve(); + } + return text; + } finally { + arrived.resolve(); } - arrived.resolve(); - return text; })(); - body.catch(arrived.reject); + body.catch(() => {}); await arrived.promise; // The server asks the stream for its trailers when the body leaves, which @@ -369,28 +384,42 @@ describe("pendingTrailers that cannot be sent", () => { }); later.closed.then(() => answered.reject(new Error("the stream closed with no answer")), answered.reject); await answered.promise; - return { closed, body, [Symbol.asyncDispose]: dispose }; + return { + closed, + settled: settled.promise, + body, + trailers: clientTrailers.promise, + [Symbol.asyncDispose]: dispose, + }; } catch (err) { await dispose(); throw err; } } + test("a block that is sent reaches the client, and the stream closes with no error", async () => { + await using response = await respondWith({ "x-checksum": "abc123" }); + expect({ + trailers: { ...(await response.trailers) }, + body: await response.body, + closed: await response.settled, + }).toEqual({ trailers: { "x-checksum": "abc123" }, body: "body", closed: "resolved" }); + }); + test("a block that sendTrailers() throws for destroys the stream with the error", async () => { await using response = await respondWith({ connection: "close" }); + expect(response.closed).toBe("ERR_HTTP2_INVALID_CONNECTION_HEADERS"); // destroy() still ends a response that the stream wrote and ended. - expect({ closed: response.closed, body: await response.body }).toEqual({ - closed: "ERR_HTTP2_INVALID_CONNECTION_HEADERS", - body: "body", - }); + expect(await response.body).toBe("body"); }); test("a block that sendTrailers() answers false for destroys the stream", async () => { // Latin-1 on the wire makes a NUL of U+0100. The block then has more // pairs than it declares, and the native side refuses it. await using response = await respondWith({ "x-bad": "a\u0100b" }); - response.body.catch(() => {}); expect(response.closed).toBe("ERR_QUIC_STREAM_ABORTED"); + // The client's read ends. The native side decides how. + await response.body.catch(() => {}); }); });