From bbd8528b67df596228a9ac236458d02a85519d6b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 10:43:43 +0000 Subject: [PATCH 01/21] node:http2: reset the stream when respond() exceeds maxSendHeaderBlockLength The server path of the native request() closed the stream locally with REFUSED_STREAM and wrote nothing. The peer never heard about the failed response and the client request hung. Send RST_STREAM FRAME_SIZE_ERROR and a graceful GOAWAY instead, the same as the trailer path and node. The client path keeps REFUSED_STREAM for a stream that never reached the wire. --- src/runtime/api/bun/h2_frame_parser.rs | 36 ++++++++++++++----- test/js/node/http2/node-http2.test.js | 48 ++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 9 deletions(-) diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index d0e9f4267aee..f769c1775a4c 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -7083,21 +7083,39 @@ impl H2FrameParser { if this.max_send_header_block_length.get() != 0 && encoded_size > this.max_send_header_block_length.get() as usize { - stream.state = StreamState::CLOSED; - stream.rst_code = ErrorCode::REFUSED_STREAM.0; - + let identifier = stream.get_identifier(); + identifier.ensure_still_alive(); this.dispatch_with_2_extra( JSH2FrameParser::Gc::onFrameError, - stream.get_identifier(), + identifier, JSValue::js_number(FrameType::HTTP_FRAME_HEADERS as u8 as f64), JSValue::js_number(ErrorCode::FRAME_SIZE_ERROR.0 as f64), ); - this.dispatch_with_extra( - JSH2FrameParser::Gc::onStreamError, - stream.get_identifier(), - JSValue::js_number(stream.rst_code as f64), - ); + if this.is_server.get() { + // The peer opened this stream, so it must hear that the response + // failed. Node resets it with FRAME_SIZE_ERROR and then closes the + // session (onFrameError in lib/internal/http2/core.js). + let triggering_id = stream.id; + this.end_stream(&mut stream, ErrorCode::FRAME_SIZE_ERROR); + this.send_go_away( + triggering_id, + ErrorCode::NO_ERROR, + b"", + this.last_stream_id.get(), + true, + ); + } else { + // A client stream that never reached the wire is closed locally, + // the way nghttp2 refuses an unsent request. + stream.state = StreamState::CLOSED; + stream.rst_code = ErrorCode::REFUSED_STREAM.0; + this.dispatch_with_extra( + JSH2FrameParser::Gc::onStreamError, + identifier, + JSValue::js_number(stream.rst_code as f64), + ); + } return Ok(JSValue::js_number(stream_id as f64)); } diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index 7cf5c3e7ab3a..da19e2f603a6 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -5796,6 +5796,54 @@ it("delivers the reserved push stream and fails the session when its headers can } }); +it("resets the stream and closes the session when respond() exceeds maxSendHeaderBlockLength", async () => { + // Verified against node v26.3.0: the server stream gets 'frameError' (HEADERS, + // FRAME_SIZE_ERROR), then RST_STREAM FRAME_SIZE_ERROR goes out and the server + // session sends a graceful GOAWAY. The client request fails with that code. + const server = http2.createServer({ maxSendHeaderBlockLength: 200 }); + try { + const serverEvents = []; + const serverStreamClosed = Promise.withResolvers(); + server.on("stream", (stream, headers) => { + stream.on("error", e => serverEvents.push(`error ${e.code} ${e.message}`)); + stream.on("frameError", (type, code) => serverEvents.push(`frameError type=${type} code=${code}`)); + stream.on("close", () => serverStreamClosed.resolve(stream.rstCode)); + stream.respond({ ":status": 200, "x-big": Buffer.alloc(1000, "b").toString() }); + stream.end("big"); + }); + const port = await new Promise(resolve => server.listen(0, () => resolve(server.address().port))); + const client = http2.connect(`http://localhost:${port}`); + client.on("error", () => {}); + const clientGoaway = new Promise(resolve => + client.on("goaway", (code, lastStreamID) => resolve({ code, lastStreamID })), + ); + + const req = client.request({ ":path": "/big" }); + const reqError = new Promise(resolve => req.on("error", resolve)); + const reqClosed = new Promise(resolve => req.on("close", resolve)); + req.on("response", () => { + throw new Error("the client must not receive a response"); + }); + req.resume(); + req.end(); + + const [err, goaway, serverRstCode] = await Promise.all([reqError, clientGoaway, serverStreamClosed.promise]); + await reqClosed; + expect(err.code).toBe("ERR_HTTP2_STREAM_ERROR"); + expect(err.message).toBe("Stream closed with error code NGHTTP2_FRAME_SIZE_ERROR"); + expect(req.rstCode).toBe(http2.constants.NGHTTP2_FRAME_SIZE_ERROR); + expect(goaway).toEqual({ code: http2.constants.NGHTTP2_NO_ERROR, lastStreamID: 1 }); + expect(serverRstCode).toBe(http2.constants.NGHTTP2_FRAME_SIZE_ERROR); + expect(serverEvents).toEqual([ + "frameError type=1 code=6", + "error ERR_HTTP2_STREAM_ERROR Stream closed with error code NGHTTP2_FRAME_SIZE_ERROR", + ]); + client.destroy(); + } finally { + server.close(); + } +}); + it("PerformanceObserver receives http2 session and stream entries", async () => { const entries = []; // Two streams (client+server) + two sessions (client+server): resolve once From 63cb733a7292475802b39d575bc567a83bff2624 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 12:52:11 +0000 Subject: [PATCH 02/21] node:http2: share the oversized header block path with send_trailers --- src/runtime/api/bun/h2_frame_parser.rs | 81 +++++++++++++------------- 1 file changed, 39 insertions(+), 42 deletions(-) diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index f769c1775a4c..4a49763c8c42 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -2186,6 +2186,29 @@ impl H2FrameParser { let _ = self.write(&buffer); } + /// An outbound header block on an open stream is over `maxSendHeaderBlockLength`. + /// Node's `onFrameError` (lib/internal/http2/core.js) emits `frameError`, resets the + /// stream with FRAME_SIZE_ERROR and then closes the session. + fn reject_oversized_header_block(&self, stream: &mut Stream) { + let identifier = stream.get_identifier(); + identifier.ensure_still_alive(); + self.dispatch_with_2_extra( + JSH2FrameParser::Gc::onFrameError, + identifier, + JSValue::js_number(FrameType::HTTP_FRAME_HEADERS as u8 as f64), + JSValue::js_number(ErrorCode::FRAME_SIZE_ERROR.0 as f64), + ); + let triggering_id = stream.id; + self.end_stream(stream, ErrorCode::FRAME_SIZE_ERROR); + self.send_go_away( + triggering_id, + ErrorCode::NO_ERROR, + b"", + self.last_stream_id.get(), + true, + ); + } + pub(crate) fn send_go_away( &self, triggering_stream_id: u32, @@ -5657,23 +5680,7 @@ impl H2FrameParser { // nghttp2 checks maxSendHeaderBlockLength pre-deflation and fires // on_frame_not_send_callback(NGHTTP2_ERR_FRAME_SIZE_ERROR); Node surfaces // 'frameError' + ERR_HTTP2_STREAM_ERROR (test-http2-exceeds-server-trailer-size.js). - let identifier = stream.get_identifier(); - identifier.ensure_still_alive(); - this.dispatch_with_2_extra( - JSH2FrameParser::Gc::onFrameError, - identifier, - JSValue::js_number(FrameType::HTTP_FRAME_HEADERS as u8 as f64), - JSValue::js_number(ErrorCode::FRAME_SIZE_ERROR.0 as f64), - ); - let triggering_id = stream.id; - this.end_stream(&mut stream, ErrorCode::FRAME_SIZE_ERROR); - this.send_go_away( - triggering_id, - ErrorCode::NO_ERROR, - b"", - this.last_stream_id.get(), - true, - ); + this.reject_oversized_header_block(&mut stream); Ok(Some(JSValue::UNDEFINED)) } } @@ -7083,6 +7090,16 @@ impl H2FrameParser { if this.max_send_header_block_length.get() != 0 && encoded_size > this.max_send_header_block_length.get() as usize { + if this.is_server.get() { + // The peer opened this stream and waits for the response. + this.reject_oversized_header_block(&mut stream); + return Ok(JSValue::js_number(stream_id as f64)); + } + + // A client request never reached the wire. nghttp2 closes such a stream + // locally with REFUSED_STREAM and sends nothing. + stream.state = StreamState::CLOSED; + stream.rst_code = ErrorCode::REFUSED_STREAM.0; let identifier = stream.get_identifier(); identifier.ensure_still_alive(); this.dispatch_with_2_extra( @@ -7091,31 +7108,11 @@ impl H2FrameParser { JSValue::js_number(FrameType::HTTP_FRAME_HEADERS as u8 as f64), JSValue::js_number(ErrorCode::FRAME_SIZE_ERROR.0 as f64), ); - - if this.is_server.get() { - // The peer opened this stream, so it must hear that the response - // failed. Node resets it with FRAME_SIZE_ERROR and then closes the - // session (onFrameError in lib/internal/http2/core.js). - let triggering_id = stream.id; - this.end_stream(&mut stream, ErrorCode::FRAME_SIZE_ERROR); - this.send_go_away( - triggering_id, - ErrorCode::NO_ERROR, - b"", - this.last_stream_id.get(), - true, - ); - } else { - // A client stream that never reached the wire is closed locally, - // the way nghttp2 refuses an unsent request. - stream.state = StreamState::CLOSED; - stream.rst_code = ErrorCode::REFUSED_STREAM.0; - this.dispatch_with_extra( - JSH2FrameParser::Gc::onStreamError, - identifier, - JSValue::js_number(stream.rst_code as f64), - ); - } + this.dispatch_with_extra( + JSH2FrameParser::Gc::onStreamError, + identifier, + JSValue::js_number(stream.rst_code as f64), + ); return Ok(JSValue::js_number(stream_id as f64)); } From 2d121eb60ee26e0909df2155aa6f99d7ec3e5ae4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 14:05:48 +0000 Subject: [PATCH 03/21] node:http2: put the last peer stream id in the GOAWAY and send it once The oversized header block path wrote GOAWAY with last_stream_id, which a server push advances past the last peer-initiated id. Use last_peer_stream_id, as the goaway() host function does. The end handler now records that the GOAWAY was sent, so destroy() does not write a second one. --- src/js/node/http2.ts | 4 +++ src/runtime/api/bun/h2_frame_parser.rs | 9 +++++- test/js/node/http2/node-http2.test.js | 38 ++++++++++++++++---------- 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index abfd52512f03..8b9851ecb414 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -4281,6 +4281,8 @@ class ServerHttp2Session extends Http2Session { }, end(self: ServerHttp2Session, errorCode: number, lastStreamId: number, opaqueData: Buffer) { if (!self) return; + // The native side wrote the GOAWAY before this callback. destroy() must not send a second one. + self[kGoawaySent] = true; self.destroy(); }, write(self: ServerHttp2Session, buffer: Buffer) { @@ -5299,6 +5301,8 @@ class ClientHttp2Session extends Http2Session { }, end(self: ClientHttp2Session, errorCode: number, lastStreamId: number, opaqueData: Buffer) { if (!self) return; + // The native side wrote the GOAWAY before this callback. destroy() must not send a second one. + self[kGoawaySent] = true; self.destroy(); }, altsvc(self: ClientHttp2Session, origin: string, value: string, streamId: number) { diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index 4a49763c8c42..6d8cf2f2992d 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -2189,6 +2189,11 @@ impl H2FrameParser { /// An outbound header block on an open stream is over `maxSendHeaderBlockLength`. /// Node's `onFrameError` (lib/internal/http2/core.js) emits `frameError`, resets the /// stream with FRAME_SIZE_ERROR and then closes the session. + /// + /// The refused block was already fed to the HPACK encoder, so the peer's decoder table + /// no longer matches ours and any later header block on this connection fails with + /// COMPRESSION_ERROR. The session therefore ends here (GOAWAY + `onEnd`) instead of + /// a graceful close that would let other streams send more headers. fn reject_oversized_header_block(&self, stream: &mut Stream) { let identifier = stream.get_identifier(); identifier.ensure_still_alive(); @@ -2200,11 +2205,13 @@ impl H2FrameParser { ); let triggering_id = stream.id; self.end_stream(stream, ErrorCode::FRAME_SIZE_ERROR); + // GOAWAY carries the last peer-initiated stream id. A server push advances + // `last_stream_id` but not `last_peer_stream_id`. self.send_go_away( triggering_id, ErrorCode::NO_ERROR, b"", - self.last_stream_id.get(), + self.last_peer_stream_id.get(), true, ); } diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index da19e2f603a6..038ba108c0a6 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -5799,7 +5799,9 @@ it("delivers the reserved push stream and fails the session when its headers can it("resets the stream and closes the session when respond() exceeds maxSendHeaderBlockLength", async () => { // Verified against node v26.3.0: the server stream gets 'frameError' (HEADERS, // FRAME_SIZE_ERROR), then RST_STREAM FRAME_SIZE_ERROR goes out and the server - // session sends a graceful GOAWAY. The client request fails with that code. + // session sends one GOAWAY. The client request fails with that code. The GOAWAY + // carries the last peer-initiated stream id even after a server push allocated a + // higher one. const server = http2.createServer({ maxSendHeaderBlockLength: 200 }); try { const serverEvents = []; @@ -5808,37 +5810,43 @@ it("resets the stream and closes the session when respond() exceeds maxSendHeade stream.on("error", e => serverEvents.push(`error ${e.code} ${e.message}`)); stream.on("frameError", (type, code) => serverEvents.push(`frameError type=${type} code=${code}`)); stream.on("close", () => serverStreamClosed.resolve(stream.rstCode)); + stream.pushStream({ ":path": "/pushed" }, (err, push) => { + if (err) throw err; + push.on("error", () => {}); + }); stream.respond({ ":status": 200, "x-big": Buffer.alloc(1000, "b").toString() }); stream.end("big"); }); const port = await new Promise(resolve => server.listen(0, () => resolve(server.address().port))); const client = http2.connect(`http://localhost:${port}`); client.on("error", () => {}); - const clientGoaway = new Promise(resolve => - client.on("goaway", (code, lastStreamID) => resolve({ code, lastStreamID })), - ); + client.on("stream", push => push.on("error", () => {})); + const goaways = []; + client.on("goaway", (code, lastStreamID) => goaways.push({ code, lastStreamID })); + const clientClosed = new Promise(resolve => client.on("close", resolve)); const req = client.request({ ":path": "/big" }); - const reqError = new Promise(resolve => req.on("error", resolve)); - const reqClosed = new Promise(resolve => req.on("close", resolve)); - req.on("response", () => { - throw new Error("the client must not receive a response"); + const reqResult = new Promise((resolve, reject) => { + let error; + req.on("response", () => reject(new Error("the client must not receive a response"))); + req.on("error", e => (error = e)); + req.on("close", () => resolve({ code: error?.code, message: error?.message, rstCode: req.rstCode })); }); req.resume(); req.end(); - const [err, goaway, serverRstCode] = await Promise.all([reqError, clientGoaway, serverStreamClosed.promise]); - await reqClosed; - expect(err.code).toBe("ERR_HTTP2_STREAM_ERROR"); - expect(err.message).toBe("Stream closed with error code NGHTTP2_FRAME_SIZE_ERROR"); - expect(req.rstCode).toBe(http2.constants.NGHTTP2_FRAME_SIZE_ERROR); - expect(goaway).toEqual({ code: http2.constants.NGHTTP2_NO_ERROR, lastStreamID: 1 }); + const [reqRes, serverRstCode] = await Promise.all([reqResult, serverStreamClosed.promise, clientClosed]); + expect(reqRes).toEqual({ + code: "ERR_HTTP2_STREAM_ERROR", + message: "Stream closed with error code NGHTTP2_FRAME_SIZE_ERROR", + rstCode: http2.constants.NGHTTP2_FRAME_SIZE_ERROR, + }); expect(serverRstCode).toBe(http2.constants.NGHTTP2_FRAME_SIZE_ERROR); + expect(goaways).toEqual([{ code: http2.constants.NGHTTP2_NO_ERROR, lastStreamID: 1 }]); expect(serverEvents).toEqual([ "frameError type=1 code=6", "error ERR_HTTP2_STREAM_ERROR Stream closed with error code NGHTTP2_FRAME_SIZE_ERROR", ]); - client.destroy(); } finally { server.close(); } From 893286b204936112d1ebc9301e89ab1370b5ab16 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 14:07:42 +0000 Subject: [PATCH 04/21] node:http2: shorten the comments on the oversized header block path --- src/runtime/api/bun/h2_frame_parser.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index 6d8cf2f2992d..7fb0afa6b63e 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -2186,14 +2186,9 @@ impl H2FrameParser { let _ = self.write(&buffer); } - /// An outbound header block on an open stream is over `maxSendHeaderBlockLength`. - /// Node's `onFrameError` (lib/internal/http2/core.js) emits `frameError`, resets the - /// stream with FRAME_SIZE_ERROR and then closes the session. - /// - /// The refused block was already fed to the HPACK encoder, so the peer's decoder table - /// no longer matches ours and any later header block on this connection fails with - /// COMPRESSION_ERROR. The session therefore ends here (GOAWAY + `onEnd`) instead of - /// a graceful close that would let other streams send more headers. + /// Node's `onFrameError`: emit `frameError`, reset the stream with FRAME_SIZE_ERROR, + /// then end the session. The refused block is already in the HPACK encoder table, + /// so no later header block on this connection can be decoded by the peer. fn reject_oversized_header_block(&self, stream: &mut Stream) { let identifier = stream.get_identifier(); identifier.ensure_still_alive(); @@ -2205,8 +2200,6 @@ impl H2FrameParser { ); let triggering_id = stream.id; self.end_stream(stream, ErrorCode::FRAME_SIZE_ERROR); - // GOAWAY carries the last peer-initiated stream id. A server push advances - // `last_stream_id` but not `last_peer_stream_id`. self.send_go_away( triggering_id, ErrorCode::NO_ERROR, @@ -7098,13 +7091,11 @@ impl H2FrameParser { && encoded_size > this.max_send_header_block_length.get() as usize { if this.is_server.get() { - // The peer opened this stream and waits for the response. this.reject_oversized_header_block(&mut stream); return Ok(JSValue::js_number(stream_id as f64)); } - // A client request never reached the wire. nghttp2 closes such a stream - // locally with REFUSED_STREAM and sends nothing. + // Client: the request never reached the wire, nghttp2 refuses it locally. stream.state = StreamState::CLOSED; stream.rst_code = ErrorCode::REFUSED_STREAM.0; let identifier = stream.get_identifier(); From 94876e7dbf8a163a7619795978ee3696bd95951f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 14:09:18 +0000 Subject: [PATCH 05/21] node:http2: one-line doc comment on reject_oversized_header_block --- src/runtime/api/bun/h2_frame_parser.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index 7fb0afa6b63e..1838f1d21c59 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -2186,9 +2186,7 @@ impl H2FrameParser { let _ = self.write(&buffer); } - /// Node's `onFrameError`: emit `frameError`, reset the stream with FRAME_SIZE_ERROR, - /// then end the session. The refused block is already in the HPACK encoder table, - /// so no later header block on this connection can be decoded by the peer. + /// Node's `onFrameError`: `frameError`, RST_STREAM FRAME_SIZE_ERROR, then GOAWAY. fn reject_oversized_header_block(&self, stream: &mut Stream) { let identifier = stream.get_identifier(); identifier.ensure_still_alive(); From 4174f9d6350c2e879dd4457dead54d2ce10b15f6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 15:15:18 +0000 Subject: [PATCH 06/21] node:http2: flush corked frames before destroy() stops accepting writes On a JS Duplex transport the write handler drops bytes once the session is no longer connected. The RST_STREAM and GOAWAY that the native side corked for an oversized header block were lost, and the peer hung. --- src/js/node/http2.ts | 6 +++++ test/js/node/http2/node-http2.test.js | 32 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index 8b9851ecb414..6c4e6cb5547a 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -4728,6 +4728,9 @@ class ServerHttp2Session extends Http2Session { const socket = this[bunHTTP2Socket]; if (!this.#connected) return; + // Frames the native side corked (RST_STREAM, GOAWAY) only reach a JS transport while + // the session is connected. + this.#parser?.flush?.(); this.#closed = true; this.#connected = false; if (socket) { @@ -5809,6 +5812,9 @@ class ClientHttp2Session extends Http2Session { // Streams torn down by this destroy surface the same session error (node semantics). this[kSessionDestroyError] = error; } + // Frames the native side corked (RST_STREAM, GOAWAY) only reach a JS transport while + // the session is connected. + this.#parser?.flush?.(); this.#closed = true; this.#connected = false; { diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index 038ba108c0a6..2b30e4be1def 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -5852,6 +5852,38 @@ it("resets the stream and closes the session when respond() exceeds maxSendHeade } }); +it("an oversized respond() over a JS Duplex transport still puts RST_STREAM and GOAWAY on the wire", async () => { + // A session on a user Duplex has no native socket, so the frames the native side corked + // must be flushed before destroy() stops accepting writes. + const [clientSide, serverSide] = duplexPair(); + const server = http2.createServer({ maxSendHeaderBlockLength: 200 }); + server.on("stream", stream => { + stream.on("error", () => {}); + stream.respond({ ":status": 200, "x-big": Buffer.alloc(1000, "b").toString() }); + stream.end("big"); + }); + server.emit("connection", serverSide); + const client = http2.connect("http://localhost", { createConnection: () => clientSide }); + client.on("error", () => {}); + const goaways = []; + client.on("goaway", (code, lastStreamID) => goaways.push({ code, lastStreamID })); + const clientClosed = new Promise(resolve => client.on("close", resolve)); + + const req = client.request({ ":path": "/big" }); + const reqClosed = new Promise((resolve, reject) => { + req.on("response", () => reject(new Error("the client must not receive a response"))); + req.on("error", () => {}); + req.on("close", resolve); + }); + req.resume(); + req.end(); + + await Promise.all([reqClosed, clientClosed]); + expect(req.rstCode).toBe(http2.constants.NGHTTP2_FRAME_SIZE_ERROR); + expect(goaways).toEqual([{ code: http2.constants.NGHTTP2_NO_ERROR, lastStreamID: 1 }]); + server.close(); +}); + it("PerformanceObserver receives http2 session and stream entries", async () => { const entries = []; // Two streams (client+server) + two sessions (client+server): resolve once From b66f73ce7557f345614226ee75c001d4f6b2d8d4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 15:18:03 +0000 Subject: [PATCH 07/21] node:http2: shorten the flush comments --- src/js/node/http2.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index 6c4e6cb5547a..4e6ea5a9db73 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -4728,8 +4728,7 @@ class ServerHttp2Session extends Http2Session { const socket = this[bunHTTP2Socket]; if (!this.#connected) return; - // Frames the native side corked (RST_STREAM, GOAWAY) only reach a JS transport while - // the session is connected. + // Corked frames reach a JS transport only while connected. this.#parser?.flush?.(); this.#closed = true; this.#connected = false; @@ -5812,8 +5811,7 @@ class ClientHttp2Session extends Http2Session { // Streams torn down by this destroy surface the same session error (node semantics). this[kSessionDestroyError] = error; } - // Frames the native side corked (RST_STREAM, GOAWAY) only reach a JS transport while - // the session is connected. + // Corked frames reach a JS transport only while connected. this.#parser?.flush?.(); this.#closed = true; this.#connected = false; From 8d04717d96e21e348f0c91bb564b703f29fd70de Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 16:11:00 +0000 Subject: [PATCH 08/21] node:http2: send the destroy() GOAWAY before the session stops accepting writes On a JS Duplex transport the write handler drops frames once the session is disconnected, so the GOAWAY that destroy() writes never reached the peer. Mark the session closed first, so a GOAWAY the peer answers inside the same synchronous write does not start a second close(). --- src/js/node/http2.ts | 22 +++++++----- test/js/node/http2/node-http2.test.js | 51 +++++++++++++++------------ 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index 4e6ea5a9db73..a64ba861d992 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -4728,17 +4728,17 @@ class ServerHttp2Session extends Http2Session { const socket = this[bunHTTP2Socket]; if (!this.#connected) return; + this.#closed = true; + if (socket && (!this[kGoawaySent] || code)) { + // close() already announced a graceful shutdown - re-sending NO_ERROR would be redundant + // and double-fires the peer's 'goaway' event. An error code is new information, though: + // a destroy(err) after close() must still put the error GOAWAY on the wire. + this.goaway(code || constants.NGHTTP2_NO_ERROR, 0, Buffer.alloc(0)); + } // Corked frames reach a JS transport only while connected. this.#parser?.flush?.(); - this.#closed = true; this.#connected = false; if (socket) { - if (!this[kGoawaySent] || code) { - // close() already announced a graceful shutdown - re-sending NO_ERROR would be redundant - // and double-fires the peer's 'goaway' event. An error code is new information, though: - // a destroy(err) after close() must still put the error GOAWAY on the wire. - this.goaway(code || constants.NGHTTP2_NO_ERROR, 0, Buffer.alloc(0)); - } if (error) { // node's finishSessionClose destroys the socket when the session dies // with an error (a misbehaving peer must observe the connection going @@ -5811,9 +5811,15 @@ class ClientHttp2Session extends Http2Session { // Streams torn down by this destroy surface the same session error (node semantics). this[kSessionDestroyError] = error; } + this.#closed = true; + if (socket && (!this[kGoawaySent] || code)) { + // close() already announced a graceful shutdown - re-sending NO_ERROR would be redundant + // and double-fires the peer's 'goaway' event. An error code is new information, though: + // a destroy(err) after close() must still put the error GOAWAY on the wire. + this.goaway(code || constants.NGHTTP2_NO_ERROR, 0, Buffer.alloc(0)); + } // Corked frames reach a JS transport only while connected. this.#parser?.flush?.(); - this.#closed = true; this.#connected = false; { // Requests still queued (waiting for connect or for a concurrency slot) never reached the diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index 2b30e4be1def..06a76c6b15ec 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -5857,31 +5857,36 @@ it("an oversized respond() over a JS Duplex transport still puts RST_STREAM and // must be flushed before destroy() stops accepting writes. const [clientSide, serverSide] = duplexPair(); const server = http2.createServer({ maxSendHeaderBlockLength: 200 }); - server.on("stream", stream => { - stream.on("error", () => {}); - stream.respond({ ":status": 200, "x-big": Buffer.alloc(1000, "b").toString() }); - stream.end("big"); - }); - server.emit("connection", serverSide); - const client = http2.connect("http://localhost", { createConnection: () => clientSide }); - client.on("error", () => {}); - const goaways = []; - client.on("goaway", (code, lastStreamID) => goaways.push({ code, lastStreamID })); - const clientClosed = new Promise(resolve => client.on("close", resolve)); + let client; + try { + server.on("stream", stream => { + stream.on("error", () => {}); + stream.respond({ ":status": 200, "x-big": Buffer.alloc(1000, "b").toString() }); + stream.end("big"); + }); + server.emit("connection", serverSide); + client = http2.connect("http://localhost", { createConnection: () => clientSide }); + client.on("error", () => {}); + const goaways = []; + client.on("goaway", (code, lastStreamID) => goaways.push({ code, lastStreamID })); + const clientClosed = new Promise(resolve => client.on("close", resolve)); - const req = client.request({ ":path": "/big" }); - const reqClosed = new Promise((resolve, reject) => { - req.on("response", () => reject(new Error("the client must not receive a response"))); - req.on("error", () => {}); - req.on("close", resolve); - }); - req.resume(); - req.end(); + const req = client.request({ ":path": "/big" }); + const reqClosed = new Promise((resolve, reject) => { + req.on("response", () => reject(new Error("the client must not receive a response"))); + req.on("error", () => {}); + req.on("close", resolve); + }); + req.resume(); + req.end(); - await Promise.all([reqClosed, clientClosed]); - expect(req.rstCode).toBe(http2.constants.NGHTTP2_FRAME_SIZE_ERROR); - expect(goaways).toEqual([{ code: http2.constants.NGHTTP2_NO_ERROR, lastStreamID: 1 }]); - server.close(); + await Promise.all([reqClosed, clientClosed]); + expect(req.rstCode).toBe(http2.constants.NGHTTP2_FRAME_SIZE_ERROR); + expect(goaways).toEqual([{ code: http2.constants.NGHTTP2_NO_ERROR, lastStreamID: 1 }]); + } finally { + client?.destroy(); + server.close(); + } }); it("PerformanceObserver receives http2 session and stream entries", async () => { From c4b7ceb506d1a80149b41c7521556d6f6601c6f6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 16:14:37 +0000 Subject: [PATCH 09/21] node:http2: shorten the destroy() GOAWAY comment --- src/js/node/http2.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index a64ba861d992..0d9f0db0757c 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -4730,9 +4730,7 @@ class ServerHttp2Session extends Http2Session { if (!this.#connected) return; this.#closed = true; if (socket && (!this[kGoawaySent] || code)) { - // close() already announced a graceful shutdown - re-sending NO_ERROR would be redundant - // and double-fires the peer's 'goaway' event. An error code is new information, though: - // a destroy(err) after close() must still put the error GOAWAY on the wire. + // close() already sent a NO_ERROR GOAWAY. An error code is new information. this.goaway(code || constants.NGHTTP2_NO_ERROR, 0, Buffer.alloc(0)); } // Corked frames reach a JS transport only while connected. @@ -5813,9 +5811,7 @@ class ClientHttp2Session extends Http2Session { } this.#closed = true; if (socket && (!this[kGoawaySent] || code)) { - // close() already announced a graceful shutdown - re-sending NO_ERROR would be redundant - // and double-fires the peer's 'goaway' event. An error code is new information, though: - // a destroy(err) after close() must still put the error GOAWAY on the wire. + // close() already sent a NO_ERROR GOAWAY. An error code is new information. this.goaway(code || constants.NGHTTP2_NO_ERROR, 0, Buffer.alloc(0)); } // Corked frames reach a JS transport only while connected. From c5731e066572650dc3b35c91ed930377736f8717 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 17:23:51 +0000 Subject: [PATCH 10/21] node:http2: destroy() writes only the corked control frames The full flush() also drains the per-stream DATA queues, so a stream with queued data could finish cleanly while its session is destroyed with an error. The new flushCorked() host method uncorks the GOAWAY and RST_STREAM frames and nothing else. The client destroy() also lost its second GOAWAY send, which the earlier block already covered. --- src/js/node/http2.ts | 10 ++-------- src/runtime/api/bun/h2_frame_parser.rs | 16 ++++++++++++++++ src/runtime/api/h2.classes.ts | 4 ++++ 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index 0d9f0db0757c..a7e3a234b5d6 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -4734,7 +4734,7 @@ class ServerHttp2Session extends Http2Session { this.goaway(code || constants.NGHTTP2_NO_ERROR, 0, Buffer.alloc(0)); } // Corked frames reach a JS transport only while connected. - this.#parser?.flush?.(); + this.#parser?.flushCorked?.(); this.#connected = false; if (socket) { if (error) { @@ -5815,7 +5815,7 @@ class ClientHttp2Session extends Http2Session { this.goaway(code || constants.NGHTTP2_NO_ERROR, 0, Buffer.alloc(0)); } // Corked frames reach a JS transport only while connected. - this.#parser?.flush?.(); + this.#parser?.flushCorked?.(); this.#connected = false; { // Requests still queued (waiting for connect or for a concurrency slot) never reached the @@ -5835,12 +5835,6 @@ class ClientHttp2Session extends Http2Session { } } if (socket) { - if (!this[kGoawaySent] || code) { - // close() already announced a graceful shutdown - re-sending NO_ERROR would be redundant - // and double-fires the peer's 'goaway' event. An error code is new information, though: - // a destroy(err) after close() must still put the error GOAWAY on the wire. - this.goaway(code || constants.NGHTTP2_NO_ERROR, 0, Buffer.alloc(0)); - } if (error) { // See the client session: end first, destroy a tick later (node's // finishSessionClose Windows-ECONNRESET avoidance). diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index 1838f1d21c59..f57685f95bd4 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -6455,6 +6455,22 @@ impl H2FrameParser { Ok(JSValue::js_number(this.flush() as f64)) } + /// Write the corked control frames (GOAWAY, RST_STREAM) to the transport. Unlike + /// `flush()` this does not drain the per-stream DATA queues, so a session that is + /// being destroyed does not complete streams on the way out. + #[bun_jsc::host_fn(method)] + pub(crate) fn flush_corked( + this: &Self, + _global_object: &JSGlobalObject, + _callframe: &CallFrame, + ) -> JsResult { + if this.js_socket_flushing.get() || !this.tx_tracker.get().at_boundary() { + return Ok(JSValue::js_number(0.0)); + } + let _keepalive = this.keepalive(); + Ok(JSValue::js_number(this.uncork() as f64)) + } + #[bun_jsc::host_fn(method)] pub(crate) fn request( this: &Self, diff --git a/src/runtime/api/h2.classes.ts b/src/runtime/api/h2.classes.ts index d612b4ea0ca3..7b12edc6cc35 100644 --- a/src/runtime/api/h2.classes.ts +++ b/src/runtime/api/h2.classes.ts @@ -53,6 +53,10 @@ export default [ fn: "flushFromJS", length: 0, }, + flushCorked: { + fn: "flushCorked", + length: 0, + }, detach: { fn: "detachFromJS", length: 0, From d641fed4cf44835452d51051dbc78deefff1818f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 17:26:09 +0000 Subject: [PATCH 11/21] node:http2: one-line doc comment on flushCorked --- src/runtime/api/bun/h2_frame_parser.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index f57685f95bd4..16fc3e090e0a 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -6455,9 +6455,7 @@ impl H2FrameParser { Ok(JSValue::js_number(this.flush() as f64)) } - /// Write the corked control frames (GOAWAY, RST_STREAM) to the transport. Unlike - /// `flush()` this does not drain the per-stream DATA queues, so a session that is - /// being destroyed does not complete streams on the way out. + /// Uncork the control frames (GOAWAY, RST_STREAM) without draining the stream DATA queues. #[bun_jsc::host_fn(method)] pub(crate) fn flush_corked( this: &Self, From 576e8771eec935d764b233c951db1748f4d7d4ac Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 20:02:52 +0000 Subject: [PATCH 12/21] node:http2: refuse an oversized header block before the HPACK encode and close the session gracefully respond() over maxSendHeaderBlockLength ended the session inside the call. session.destroy() then ended every other open stream with an empty DATA frame and END_STREAM, so a healthy response was cut short without an error. respond() after an oversized additionalHeaders() threw ERR_HTTP2_INVALID_STREAM. request() now stages the fields when the option is set and checks the block on nghttp2's pre-compression bound before the encoder sees it. The peer's HPACK table stays in step, so the server session can close gracefully like node's onFrameError: the stream is reset with FRAME_SIZE_ERROR, then setImmediate runs stream.close(code) and session.close(). A refused 1xx block leaves the stream open for the final response. The destroy() and flushCorked() changes are reverted. The reject path no longer destroys the session, so they are not part of this fix. --- src/js/node/http2.ts | 35 ++-- src/runtime/api/bun/h2_frame_parser.rs | 228 +++++++++++++++++-------- src/runtime/api/h2.classes.ts | 4 - test/js/node/http2/node-http2.test.js | 153 +++++++++++++++-- 4 files changed, 323 insertions(+), 97 deletions(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index a7e3a234b5d6..720ce3433518 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -4018,6 +4018,7 @@ class ServerHttp2Session extends Http2Session { if (!self || typeof stream !== "object") return; // Emit the frameError event with the frame type and error code process.nextTick(emitFrameErrorEventNT, stream, frameType, errorCode); + setImmediate(closeAfterFrameError, self, stream, errorCode); }, aborted(self: ServerHttp2Session, stream: ServerHttp2Stream, error: any, old_state: number) { if (!self || typeof stream !== "object") return; @@ -4281,8 +4282,6 @@ class ServerHttp2Session extends Http2Session { }, end(self: ServerHttp2Session, errorCode: number, lastStreamId: number, opaqueData: Buffer) { if (!self) return; - // The native side wrote the GOAWAY before this callback. destroy() must not send a second one. - self[kGoawaySent] = true; self.destroy(); }, write(self: ServerHttp2Session, buffer: Buffer) { @@ -4729,14 +4728,14 @@ class ServerHttp2Session extends Http2Session { const socket = this[bunHTTP2Socket]; if (!this.#connected) return; this.#closed = true; - if (socket && (!this[kGoawaySent] || code)) { - // close() already sent a NO_ERROR GOAWAY. An error code is new information. - this.goaway(code || constants.NGHTTP2_NO_ERROR, 0, Buffer.alloc(0)); - } - // Corked frames reach a JS transport only while connected. - this.#parser?.flushCorked?.(); this.#connected = false; if (socket) { + if (!this[kGoawaySent] || code) { + // close() already announced a graceful shutdown - re-sending NO_ERROR would be redundant + // and double-fires the peer's 'goaway' event. An error code is new information, though: + // a destroy(err) after close() must still put the error GOAWAY on the wire. + this.goaway(code || constants.NGHTTP2_NO_ERROR, 0, Buffer.alloc(0)); + } if (error) { // node's finishSessionClose destroys the socket when the session dies // with an error (a misbehaving peer must observe the connection going @@ -5301,8 +5300,6 @@ class ClientHttp2Session extends Http2Session { }, end(self: ClientHttp2Session, errorCode: number, lastStreamId: number, opaqueData: Buffer) { if (!self) return; - // The native side wrote the GOAWAY before this callback. destroy() must not send a second one. - self[kGoawaySent] = true; self.destroy(); }, altsvc(self: ClientHttp2Session, origin: string, value: string, streamId: number) { @@ -5810,12 +5807,6 @@ class ClientHttp2Session extends Http2Session { this[kSessionDestroyError] = error; } this.#closed = true; - if (socket && (!this[kGoawaySent] || code)) { - // close() already sent a NO_ERROR GOAWAY. An error code is new information. - this.goaway(code || constants.NGHTTP2_NO_ERROR, 0, Buffer.alloc(0)); - } - // Corked frames reach a JS transport only while connected. - this.#parser?.flushCorked?.(); this.#connected = false; { // Requests still queued (waiting for connect or for a concurrency slot) never reached the @@ -5835,6 +5826,12 @@ class ClientHttp2Session extends Http2Session { } } if (socket) { + if (!this[kGoawaySent] || code) { + // close() already announced a graceful shutdown - re-sending NO_ERROR would be redundant + // and double-fires the peer's 'goaway' event. An error code is new information, though: + // a destroy(err) after close() must still put the error GOAWAY on the wire. + this.goaway(code || constants.NGHTTP2_NO_ERROR, 0, Buffer.alloc(0)); + } if (error) { // See the client session: end first, destroy a tick later (node's // finishSessionClose Windows-ECONNRESET avoidance). @@ -6568,6 +6565,12 @@ function onErrorSecureServerSession(err, socket) { function emitFrameErrorEventNT(stream, frameType, errorCode) { stream.emit("frameError", frameType, errorCode); } +// node's onFrameError: the stream and the session stay usable for the rest of the tick, then the +// session closes gracefully so the other streams finish. Both calls are no-ops once closed. +function closeAfterFrameError(session: ServerHttp2Session, stream: ServerHttp2Stream, code: number) { + stream.close(code); + session.close(); +} class Http2SecureServer extends tls.Server { timeout = 0; [kSessions] = new SafeSet(); diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index 16fc3e090e0a..1003042203cb 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -1962,11 +1962,91 @@ impl AbortListener for SignalRef { } } +/// The fields of one outbound header block, staged so the block can be measured before any +/// field reaches the HPACK encoder. A field that is encoded and never sent desyncs the peer's +/// dynamic table for the rest of the connection. +#[derive(Default)] +struct HeaderList { + bytes: Vec, + fields: Vec, +} + +struct HeaderField { + name_len: usize, + value_len: usize, + never_index: bool, +} + +impl HeaderList { + fn push( + &mut self, + name: &[u8], + value: &[u8], + never_index: bool, + ) -> Result<(), bun_alloc::AllocError> { + self.bytes + .try_reserve(name.len() + value.len()) + .map_err(|_| bun_alloc::AllocError)?; + self.fields + .try_reserve(1) + .map_err(|_| bun_alloc::AllocError)?; + self.bytes.extend_from_slice(name); + self.bytes.extend_from_slice(value); + self.fields.push(HeaderField { + name_len: name.len(), + value_len: value.len(), + never_index, + }); + Ok(()) + } + + /// Same formula as nghttp2_hd_deflate_bound: + /// https://github.com/nodejs/node/blob/v26.3.0/deps/nghttp2/lib/nghttp2_hd.c + fn deflate_bound(&self) -> usize { + 12 + self.fields.len() * 12 + self.bytes.len() + } + + /// A 1xx `:status`: the final response still follows on the same stream. + fn is_informational(&self) -> bool { + self.iter() + .any(|(name, value, _)| name == b":status" && value.len() == 3 && value[0] == b'1') + } + + fn iter(&self) -> impl Iterator { + let mut offset = 0usize; + self.fields.iter().map(move |field| { + let name_end = offset + field.name_len; + let value_end = name_end + field.value_len; + let name = &self.bytes[offset..name_end]; + let value = &self.bytes[name_end..value_end]; + offset = value_end; + (name, value, field.never_index) + }) + } +} + // ────────────────────────────────────────────────────────────────────────── // H2FrameParser impl — core methods // ────────────────────────────────────────────────────────────────────────── impl H2FrameParser { + /// Stages the field when `request()` has to measure the whole block first, else encodes it. + fn encode_or_stage_header( + &self, + staged: &mut Option, + encoded_headers: &mut Vec, + name: &[u8], + value: &[u8], + never_index: bool, + ) -> crate::Result<()> { + match staged { + Some(list) => Ok(list.push(name, value, never_index)?), + None => self + .encode_header_into_list(encoded_headers, name, value, never_index) + .map(|_| ()), + } + } + /// Encodes a single header into the ArrayList, growing if needed. /// Returns the number of bytes written, or error on failure. /// @@ -2186,27 +2266,6 @@ impl H2FrameParser { let _ = self.write(&buffer); } - /// Node's `onFrameError`: `frameError`, RST_STREAM FRAME_SIZE_ERROR, then GOAWAY. - fn reject_oversized_header_block(&self, stream: &mut Stream) { - let identifier = stream.get_identifier(); - identifier.ensure_still_alive(); - self.dispatch_with_2_extra( - JSH2FrameParser::Gc::onFrameError, - identifier, - JSValue::js_number(FrameType::HTTP_FRAME_HEADERS as u8 as f64), - JSValue::js_number(ErrorCode::FRAME_SIZE_ERROR.0 as f64), - ); - let triggering_id = stream.id; - self.end_stream(stream, ErrorCode::FRAME_SIZE_ERROR); - self.send_go_away( - triggering_id, - ErrorCode::NO_ERROR, - b"", - self.last_peer_stream_id.get(), - true, - ); - } - pub(crate) fn send_go_away( &self, triggering_stream_id: u32, @@ -5678,7 +5737,23 @@ impl H2FrameParser { // nghttp2 checks maxSendHeaderBlockLength pre-deflation and fires // on_frame_not_send_callback(NGHTTP2_ERR_FRAME_SIZE_ERROR); Node surfaces // 'frameError' + ERR_HTTP2_STREAM_ERROR (test-http2-exceeds-server-trailer-size.js). - this.reject_oversized_header_block(&mut stream); + let identifier = stream.get_identifier(); + identifier.ensure_still_alive(); + this.dispatch_with_2_extra( + JSH2FrameParser::Gc::onFrameError, + identifier, + JSValue::js_number(FrameType::HTTP_FRAME_HEADERS as u8 as f64), + JSValue::js_number(ErrorCode::FRAME_SIZE_ERROR.0 as f64), + ); + let triggering_id = stream.id; + this.end_stream(&mut stream, ErrorCode::FRAME_SIZE_ERROR); + this.send_go_away( + triggering_id, + ErrorCode::NO_ERROR, + b"", + this.last_stream_id.get(), + true, + ); Ok(Some(JSValue::UNDEFINED)) } } @@ -6455,20 +6530,6 @@ impl H2FrameParser { Ok(JSValue::js_number(this.flush() as f64)) } - /// Uncork the control frames (GOAWAY, RST_STREAM) without draining the stream DATA queues. - #[bun_jsc::host_fn(method)] - pub(crate) fn flush_corked( - this: &Self, - _global_object: &JSGlobalObject, - _callframe: &CallFrame, - ) -> JsResult { - if this.js_socket_flushing.get() || !this.tx_tracker.get().at_boundary() { - return Ok(JSValue::js_number(0.0)); - } - let _keepalive = this.keepalive(); - Ok(JSValue::js_number(this.uncork() as f64)) - } - #[bun_jsc::host_fn(method)] pub(crate) fn request( this: &Self, @@ -6503,6 +6564,8 @@ impl H2FrameParser { if encoded_headers.try_reserve(16384).is_err() { return Err(global_object.throw(format_args!("Failed to allocate header buffer"))); } + // maxSendHeaderBlockLength is checked on the whole block before the encoder sees it. + let mut staged = (this.max_send_header_block_length.get() != 0).then(HeaderList::default); // max header name length for lshpack let mut name_buffer = [0u8; 4096]; let stream_id: u32 = @@ -6623,7 +6686,8 @@ impl H2FrameParser { BStr::new(value) ); - if let Err(err) = this.encode_header_into_list( + if let Err(err) = this.encode_or_stage_header( + &mut staged, &mut encoded_headers, validated_name, value, @@ -6793,7 +6857,8 @@ impl H2FrameParser { BStr::new(value) ); - if let Err(err) = this.encode_header_into_list( + if let Err(err) = this.encode_or_stage_header( + &mut staged, &mut encoded_headers, validated_name, value, @@ -6863,7 +6928,8 @@ impl H2FrameParser { BStr::new(value) ); - if let Err(err) = this.encode_header_into_list( + if let Err(err) = this.encode_or_stage_header( + &mut staged, &mut encoded_headers, validated_name, value, @@ -6889,7 +6955,6 @@ impl H2FrameParser { } } } - let encoded_size = encoded_headers.len(); let Some(stream_ptr) = this.handle_received_stream_id(stream_id) else { return Ok(JSValue::js_number(-1.0)); @@ -7090,41 +7155,71 @@ impl H2FrameParser { } return Ok(JSValue::js_number(stream_id as f64)); } - let mut length: usize = encoded_size; if has_priority { - length += 5; flags |= HeadersFrameFlags::PRIORITY as u8; } - bun_output::scoped_log!(H2FrameParser, "request encoded_size {}", encoded_size); + if let Some(staged) = &staged { + // nghttp2 refuses the block on its pre-compression bound (it always counts the + // priority fields) before it deflates it, so the peer's HPACK table stays in step: + // https://github.com/nodejs/node/blob/v26.3.0/deps/nghttp2/lib/nghttp2_session.c#L2095-L2101 + if staged.deflate_bound() + StreamPriority::BYTE_SIZE + > this.max_send_header_block_length.get() as usize + { + if this.is_server.get() { + let identifier = stream.get_identifier(); + identifier.ensure_still_alive(); + // The JS handler closes the session gracefully, like node's onFrameError. + this.dispatch_with_2_extra( + JSH2FrameParser::Gc::onFrameError, + identifier, + JSValue::js_number(FrameType::HTTP_FRAME_HEADERS as u8 as f64), + JSValue::js_number(ErrorCode::FRAME_SIZE_ERROR.0 as f64), + ); + // Nothing can follow a refused final response: DATA without HEADERS is a + // connection error for the peer. A refused 1xx block leaves the stream open + // for the final response. + if !staged.is_informational() { + this.end_stream(&mut stream, ErrorCode::FRAME_SIZE_ERROR); + } + return Ok(JSValue::js_number(stream_id as f64)); + } - // Check if headers block exceeds maxSendHeaderBlockLength - if this.max_send_header_block_length.get() != 0 - && encoded_size > this.max_send_header_block_length.get() as usize - { - if this.is_server.get() { - this.reject_oversized_header_block(&mut stream); + // Client: the request never reached the wire, nghttp2 refuses it locally. + stream.state = StreamState::CLOSED; + stream.rst_code = ErrorCode::REFUSED_STREAM.0; + + this.dispatch_with_2_extra( + JSH2FrameParser::Gc::onFrameError, + stream.get_identifier(), + JSValue::js_number(FrameType::HTTP_FRAME_HEADERS as u8 as f64), + JSValue::js_number(ErrorCode::FRAME_SIZE_ERROR.0 as f64), + ); + + this.dispatch_with_extra( + JSH2FrameParser::Gc::onStreamError, + stream.get_identifier(), + JSValue::js_number(stream.rst_code as f64), + ); return Ok(JSValue::js_number(stream_id as f64)); } - // Client: the request never reached the wire, nghttp2 refuses it locally. - stream.state = StreamState::CLOSED; - stream.rst_code = ErrorCode::REFUSED_STREAM.0; - let identifier = stream.get_identifier(); - identifier.ensure_still_alive(); - this.dispatch_with_2_extra( - JSH2FrameParser::Gc::onFrameError, - identifier, - JSValue::js_number(FrameType::HTTP_FRAME_HEADERS as u8 as f64), - JSValue::js_number(ErrorCode::FRAME_SIZE_ERROR.0 as f64), - ); - this.dispatch_with_extra( - JSH2FrameParser::Gc::onStreamError, - identifier, - JSValue::js_number(stream.rst_code as f64), - ); - return Ok(JSValue::js_number(stream_id as f64)); + for (name, value, never_index) in staged.iter() { + if let Err(err) = + this.encode_header_into_list(&mut encoded_headers, name, value, never_index) + { + if matches!(err, crate::Error::Alloc(_)) { + return Err( + global_object.throw(format_args!("Failed to allocate header buffer")) + ); + } + this.schedule_header_compression_session_error(); + return Ok(JSValue::js_number(stream_id as f64)); + } + } } + let encoded_size = encoded_headers.len(); + bun_output::scoped_log!(H2FrameParser, "request encoded_size {}", encoded_size); let actual_max_frame_size = this .remote_settings @@ -7315,7 +7410,6 @@ impl H2FrameParser { // TODO: should we make use of this in the future? We validate it. } - let _ = length; Ok(JSValue::js_number(stream_id as f64)) } diff --git a/src/runtime/api/h2.classes.ts b/src/runtime/api/h2.classes.ts index 7b12edc6cc35..d612b4ea0ca3 100644 --- a/src/runtime/api/h2.classes.ts +++ b/src/runtime/api/h2.classes.ts @@ -53,10 +53,6 @@ export default [ fn: "flushFromJS", length: 0, }, - flushCorked: { - fn: "flushCorked", - length: 0, - }, detach: { fn: "detachFromJS", length: 0, diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index 06a76c6b15ec..a10c0341a83b 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -5796,23 +5796,24 @@ it("delivers the reserved push stream and fails the session when its headers can } }); -it("resets the stream and closes the session when respond() exceeds maxSendHeaderBlockLength", async () => { +it("resets the stream and closes the session gracefully when respond() exceeds maxSendHeaderBlockLength", async () => { // Verified against node v26.3.0: the server stream gets 'frameError' (HEADERS, - // FRAME_SIZE_ERROR), then RST_STREAM FRAME_SIZE_ERROR goes out and the server - // session sends one GOAWAY. The client request fails with that code. The GOAWAY - // carries the last peer-initiated stream id even after a server push allocated a - // higher one. + // FRAME_SIZE_ERROR), RST_STREAM FRAME_SIZE_ERROR goes out, and the session closes with one + // GOAWAY NO_ERROR. The pushed stream responds after the refused block and is still + // delivered: the refused block never reached the HPACK encoder. const server = http2.createServer({ maxSendHeaderBlockLength: 200 }); try { const serverEvents = []; const serverStreamClosed = Promise.withResolvers(); - server.on("stream", (stream, headers) => { + server.on("stream", stream => { stream.on("error", e => serverEvents.push(`error ${e.code} ${e.message}`)); stream.on("frameError", (type, code) => serverEvents.push(`frameError type=${type} code=${code}`)); stream.on("close", () => serverStreamClosed.resolve(stream.rstCode)); stream.pushStream({ ":path": "/pushed" }, (err, push) => { if (err) throw err; push.on("error", () => {}); + push.respond({ ":status": 200, "x-pushed": "yes" }); + push.end("pushed"); }); stream.respond({ ":status": 200, "x-big": Buffer.alloc(1000, "b").toString() }); stream.end("big"); @@ -5820,10 +5821,19 @@ it("resets the stream and closes the session when respond() exceeds maxSendHeade const port = await new Promise(resolve => server.listen(0, () => resolve(server.address().port))); const client = http2.connect(`http://localhost:${port}`); client.on("error", () => {}); - client.on("stream", push => push.on("error", () => {})); const goaways = []; client.on("goaway", (code, lastStreamID) => goaways.push({ code, lastStreamID })); const clientClosed = new Promise(resolve => client.on("close", resolve)); + const pushResult = new Promise(resolve => { + client.on("stream", push => { + const result = { pushed: undefined, body: "", error: undefined, rstCode: undefined }; + push.setEncoding("utf8"); + push.on("push", headers => (result.pushed = headers["x-pushed"])); + push.on("data", chunk => (result.body += chunk)); + push.on("error", e => (result.error = e.code)); + push.on("close", () => resolve({ ...result, rstCode: push.rstCode })); + }); + }); const req = client.request({ ":path": "/big" }); const reqResult = new Promise((resolve, reject) => { @@ -5835,12 +5845,18 @@ it("resets the stream and closes the session when respond() exceeds maxSendHeade req.resume(); req.end(); - const [reqRes, serverRstCode] = await Promise.all([reqResult, serverStreamClosed.promise, clientClosed]); + const [reqRes, pushRes, serverRstCode] = await Promise.all([ + reqResult, + pushResult, + serverStreamClosed.promise, + clientClosed, + ]); expect(reqRes).toEqual({ code: "ERR_HTTP2_STREAM_ERROR", message: "Stream closed with error code NGHTTP2_FRAME_SIZE_ERROR", rstCode: http2.constants.NGHTTP2_FRAME_SIZE_ERROR, }); + expect(pushRes).toEqual({ pushed: "yes", body: "pushed", error: undefined, rstCode: 0 }); expect(serverRstCode).toBe(http2.constants.NGHTTP2_FRAME_SIZE_ERROR); expect(goaways).toEqual([{ code: http2.constants.NGHTTP2_NO_ERROR, lastStreamID: 1 }]); expect(serverEvents).toEqual([ @@ -5852,9 +5868,126 @@ it("resets the stream and closes the session when respond() exceeds maxSendHeade } }); +it("streams in flight complete when another response exceeds maxSendHeaderBlockLength", async () => { + // Verified against node v26.3.0: the session closes gracefully, so /slow receives the rest of + // its body after the GOAWAY. The /late response is a header block that follows the refused + // one, and it reuses an indexed field, so it only decodes if the HPACK tables are in step. + const HALF = Buffer.alloc(1000, "a").toString(); + const server = http2.createServer({ maxSendHeaderBlockLength: 200 }); + try { + let late; + server.on("stream", (stream, headers) => { + stream.on("error", () => {}); + const path = headers[":path"]; + if (path === "/slow") { + stream.respond({ ":status": 200, "x-marker": "indexed" }); + stream.write(HALF); + // The client ends this request once it has seen the GOAWAY. + stream.on("end", () => { + stream.end(HALF); + late.respond({ ":status": 200, "x-marker": "indexed" }); + late.end("late"); + }); + stream.resume(); + } else if (path === "/late") { + late = stream; + } else { + stream.respond({ ":status": 200, "x-big": Buffer.alloc(1000, "b").toString() }); + stream.end("big"); + } + }); + const port = await new Promise(resolve => server.listen(0, () => resolve(server.address().port))); + const client = http2.connect(`http://localhost:${port}`); + client.on("error", () => {}); + const goaway = new Promise(resolve => client.once("goaway", resolve)); + const clientClosed = new Promise(resolve => client.on("close", resolve)); + const collect = req => + new Promise(resolve => { + const result = { status: undefined, marker: undefined, bytes: 0, ended: false, error: undefined }; + req.on("response", headers => { + result.status = headers[":status"]; + result.marker = headers["x-marker"]; + }); + req.on("data", chunk => (result.bytes += chunk.length)); + req.on("end", () => (result.ended = true)); + req.on("error", e => (result.error = e.code)); + req.on("close", () => resolve({ ...result, rstCode: req.rstCode })); + }); + + const slow = client.request({ ":path": "/slow", ":method": "POST" }); + const slowResult = collect(slow); + const firstHalf = new Promise(resolve => slow.once("data", resolve)); + const lateResult = collect(client.request({ ":path": "/late" })); + await firstHalf; + const bigResult = collect(client.request({ ":path": "/big" })); + await goaway; + slow.end(); + + const [slowRes, lateRes, bigRes] = await Promise.all([slowResult, lateResult, bigResult, clientClosed]); + expect({ slow: slowRes, late: lateRes, big: bigRes }).toEqual({ + slow: { status: 200, marker: "indexed", bytes: 2000, ended: true, error: undefined, rstCode: 0 }, + late: { status: 200, marker: "indexed", bytes: 4, ended: true, error: undefined, rstCode: 0 }, + big: { + status: undefined, + marker: undefined, + bytes: 0, + ended: false, + error: "ERR_HTTP2_STREAM_ERROR", + rstCode: http2.constants.NGHTTP2_FRAME_SIZE_ERROR, + }, + }); + } finally { + server.close(); + } +}); + +it("respond() delivers the response after additionalHeaders() exceeds maxSendHeaderBlockLength", async () => { + // Verified against node v26.3.0: the 1xx block is dropped with 'frameError', the stream stays + // usable for the rest of the tick, and the session closes gracefully afterwards. + const server = http2.createServer({ maxSendHeaderBlockLength: 200 }); + try { + const serverEvents = []; + const serverStreamClosed = Promise.withResolvers(); + server.on("stream", stream => { + stream.on("error", e => serverEvents.push(`error ${e.code} ${e.message}`)); + stream.on("frameError", (type, code) => serverEvents.push(`frameError type=${type} code=${code}`)); + stream.on("close", () => serverStreamClosed.resolve(stream.rstCode)); + stream.additionalHeaders({ ":status": 103, "x-big": Buffer.alloc(1000, "b").toString() }); + stream.respond({ ":status": 200 }); + stream.end("body"); + }); + const port = await new Promise(resolve => server.listen(0, () => resolve(server.address().port))); + const client = http2.connect(`http://localhost:${port}`); + client.on("error", () => {}); + const goaways = []; + client.on("goaway", (code, lastStreamID) => goaways.push({ code, lastStreamID })); + const clientClosed = new Promise(resolve => client.on("close", resolve)); + + const req = client.request({ ":path": "/" }); + const reqResult = new Promise(resolve => { + const result = { informational: [], status: undefined, body: "", error: undefined }; + req.setEncoding("utf8"); + req.on("headers", headers => result.informational.push(headers[":status"])); + req.on("response", headers => (result.status = headers[":status"])); + req.on("data", chunk => (result.body += chunk)); + req.on("error", e => (result.error = e.code)); + req.on("close", () => resolve({ ...result, rstCode: req.rstCode })); + }); + req.end(); + + const [reqRes, serverRstCode] = await Promise.all([reqResult, serverStreamClosed.promise, clientClosed]); + expect(reqRes).toEqual({ informational: [], status: 200, body: "body", error: undefined, rstCode: 0 }); + expect(serverRstCode).toBe(http2.constants.NGHTTP2_NO_ERROR); + expect(goaways).toEqual([{ code: http2.constants.NGHTTP2_NO_ERROR, lastStreamID: 1 }]); + expect(serverEvents).toEqual(["frameError type=1 code=6"]); + } finally { + server.close(); + } +}); + it("an oversized respond() over a JS Duplex transport still puts RST_STREAM and GOAWAY on the wire", async () => { - // A session on a user Duplex has no native socket, so the frames the native side corked - // must be flushed before destroy() stops accepting writes. + // A session on a user Duplex has no native socket: the RST_STREAM is corked by the native + // side and must still reach the transport. const [clientSide, serverSide] = duplexPair(); const server = http2.createServer({ maxSendHeaderBlockLength: 200 }); let client; From 0f425e492e608205a537ca13f2a1f4df631844e3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 20:45:04 +0000 Subject: [PATCH 13/21] node:http2: a reset stream sends no trailers and no END_STREAM After respond() is refused the handler keeps using the stream for the rest of the tick. The compat API ended it with an empty DATA frame, and sendTrailers() wrote a HEADERS frame, both after the RST_STREAM. noTrailers() and sendTrailers() now return early for a closed stream, like writeStream(). Tests: wire-level frames for the core and compat API, and a client request over the limit that must leave the HPACK encoder in step. --- src/runtime/api/bun/h2_frame_parser.rs | 8 +++ test/js/node/http2/h2-conformance.test.ts | 71 +++++++++++++++++++++++ test/js/node/http2/node-http2.test.js | 57 +++++++++++++++++- 3 files changed, 133 insertions(+), 3 deletions(-) diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index 1003042203cb..b976ca92086e 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -5466,6 +5466,10 @@ impl H2FrameParser { }; // SAFETY: stream is a *mut Stream from self.streams (heap::alloc); valid while the map entry exists let stream = unsafe { &mut *stream }; + // JS can still end a stream in the tick that reset it. A reset stream sends nothing. + if stream.state == StreamState::CLOSED { + return Ok(JSValue::UNDEFINED); + } stream.wait_for_trailers = false; let _ = this.send_data( @@ -5634,6 +5638,10 @@ impl H2FrameParser { // The header/sensitive-object getters and value coercions below can run user JS // while `stream` is borrowed. let mut stream = this.enter_stream_dispatch(stream_ptr); + // JS can still send trailers in the tick that reset the stream. A reset stream sends nothing. + if stream.state == StreamState::CLOSED { + return Ok(JSValue::UNDEFINED); + } let Some(headers_obj) = headers_arg.get_object() else { return Err(global_object.throw(format_args!("Expected headers to be an object"))); diff --git a/test/js/node/http2/h2-conformance.test.ts b/test/js/node/http2/h2-conformance.test.ts index 414bf6c3b68b..0cf9e30e2bcd 100644 --- a/test/js/node/http2/h2-conformance.test.ts +++ b/test/js/node/http2/h2-conformance.test.ts @@ -1928,3 +1928,74 @@ describe("stream release after a queued END_STREAM", () => { } }); }); + +describe.concurrent("a response over maxSendHeaderBlockLength (nghttp2 on_frame_not_send_callback)", () => { + // Verified against node v26.3.0 with the same raw client: the stream only carries + // RST_STREAM FRAME_SIZE_ERROR, and the session closes with GOAWAY NO_ERROR (node sends that + // GOAWAY twice). The handler keeps using the stream for the rest of the tick, and none of + // that may reach the wire. + const BIG = Buffer.alloc(300, "b").toString(); + const handlers: Record void> = { + "respond() then end()": server => + server.on("stream", stream => { + stream.on("error", () => {}); + stream.respond({ ":status": 200, "x-big": BIG }); + stream.end("ok"); + }), + "respond() with waitForTrailers, then sendTrailers()": server => + server.on("stream", stream => { + stream.on("error", () => {}); + stream.respond({ ":status": 200, "x-big": BIG }, { waitForTrailers: true }); + stream.on("wantTrailers", () => stream.sendTrailers({ "x-trailer": "1" })); + stream.end("ok"); + }), + "the compat API": server => + server.on("request", (req, res) => { + res.on("error", () => {}); + res.setHeader("x-big", BIG); + res.end("ok"); + }), + "the compat API with trailers": server => + server.on("request", (req, res) => { + res.on("error", () => {}); + res.setHeader("x-big", BIG); + res.addTrailers({ "x-trailer": "1" }); + res.end("ok"); + }), + }; + + test.each(Object.keys(handlers))("%s", async name => { + const server = http2.createServer({ maxSendHeaderBlockLength: 100 }); + handlers[name](server); + server.listen(0); + await once(server, "listening"); + const c = await RawH2.connect((server.address() as net.AddressInfo).port); + try { + const closed = once(c.socket, "close"); + c.sendPreface(); + c.sendEmptySettings(); + c.sendSettingsAck(); + c.sendFrame(FrameType.HEADERS, 0x5, 1, requestHeaderBlock("GET")); + await closed; + const frames = c.frames + .filter(f => f.streamId === 1 || f.type === FrameType.GOAWAY) + .map(f => ({ + type: f.type, + streamId: f.streamId, + code: + f.type === FrameType.GOAWAY + ? goawayErrorCode(f) + : f.type === FrameType.RST_STREAM + ? f.payload.readUInt32BE(0) + : undefined, + })); + expect(frames).toEqual([ + { type: FrameType.RST_STREAM, streamId: 1, code: ErrorCode.FRAME_SIZE_ERROR }, + { type: FrameType.GOAWAY, streamId: 0, code: ErrorCode.NO_ERROR }, + ]); + } finally { + c.destroy(); + server.close(); + } + }); +}); diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index a10c0341a83b..9ae489e6eb98 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -5798,9 +5798,9 @@ it("delivers the reserved push stream and fails the session when its headers can it("resets the stream and closes the session gracefully when respond() exceeds maxSendHeaderBlockLength", async () => { // Verified against node v26.3.0: the server stream gets 'frameError' (HEADERS, - // FRAME_SIZE_ERROR), RST_STREAM FRAME_SIZE_ERROR goes out, and the session closes with one - // GOAWAY NO_ERROR. The pushed stream responds after the refused block and is still - // delivered: the refused block never reached the HPACK encoder. + // FRAME_SIZE_ERROR), RST_STREAM FRAME_SIZE_ERROR goes out, and the session closes with + // GOAWAY NO_ERROR (node sends that GOAWAY twice). The pushed stream responds after the + // refused block and is still delivered: the refused block never reached the HPACK encoder. const server = http2.createServer({ maxSendHeaderBlockLength: 200 }); try { const serverEvents = []; @@ -5985,6 +5985,57 @@ it("respond() delivers the response after additionalHeaders() exceeds maxSendHea } }); +it("a client request over maxSendHeaderBlockLength leaves the session usable", async () => { + // Verified against node v26.3.0: the request is refused locally with 'frameError' and + // REFUSED_STREAM. The next request reuses an indexed field, so the server only decodes it if + // the refused block never reached the client's HPACK encoder. + const server = http2.createServer(); + try { + server.on("stream", (stream, headers) => { + stream.respond({ ":status": 200, "x-echo": String(headers["x-marker"]) }); + stream.end(); + }); + const port = await new Promise(resolve => server.listen(0, () => resolve(server.address().port))); + const client = http2.connect(`http://localhost:${port}`, { maxSendHeaderBlockLength: 300 }); + const sessionErrors = []; + client.on("error", e => sessionErrors.push(e.message)); + const send = headers => + new Promise(resolve => { + const result = { frameError: undefined, echo: undefined, error: undefined }; + const req = client.request(headers); + req.on("frameError", (type, code) => (result.frameError = [type, code])); + req.on("response", responseHeaders => (result.echo = responseHeaders["x-echo"])); + req.on("error", e => (result.error = e.message)); + req.on("close", () => resolve({ ...result, rstCode: req.rstCode })); + req.resume(); + req.end(); + }); + + const results = [ + await send({ ":path": "/a", "x-marker": "indexed" }), + await send({ ":path": "/b", "x-marker": "indexed", "x-big": Buffer.alloc(400, "b").toString() }), + await send({ ":path": "/c", "x-marker": "indexed" }), + ]; + client.destroy(); + expect({ results, sessionErrors }).toEqual({ + results: [ + { frameError: undefined, echo: "indexed", error: undefined, rstCode: 0 }, + { + // frame type 1 is HEADERS + frameError: [1, http2.constants.NGHTTP2_FRAME_SIZE_ERROR], + echo: undefined, + error: "Stream closed with error code NGHTTP2_REFUSED_STREAM", + rstCode: http2.constants.NGHTTP2_REFUSED_STREAM, + }, + { frameError: undefined, echo: "indexed", error: undefined, rstCode: 0 }, + ], + sessionErrors: [], + }); + } finally { + server.close(); + } +}); + it("an oversized respond() over a JS Duplex transport still puts RST_STREAM and GOAWAY on the wire", async () => { // A session on a user Duplex has no native socket: the RST_STREAM is corked by the native // side and must still reach the transport. From f63b048572484bedfa9ec5b5fb7eb410346acbd9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 21:00:47 +0000 Subject: [PATCH 14/21] node:http2: keep the client branch of the send limit check as it was --- src/runtime/api/bun/h2_frame_parser.rs | 75 +++++++++++++------------- 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index b976ca92086e..7853c4687664 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -2001,7 +2001,7 @@ impl HeaderList { } /// Same formula as nghttp2_hd_deflate_bound: - /// https://github.com/nodejs/node/blob/v26.3.0/deps/nghttp2/lib/nghttp2_hd.c + /// https://github.com/nodejs/node/blob/v26.3.0/deps/nghttp2/lib/nghttp2_hd.c#L1578-L1603 fn deflate_bound(&self) -> usize { 12 + self.fields.len() * 12 + self.bytes.len() } @@ -7167,51 +7167,52 @@ impl H2FrameParser { flags |= HeadersFrameFlags::PRIORITY as u8; } - if let Some(staged) = &staged { - // nghttp2 refuses the block on its pre-compression bound (it always counts the - // priority fields) before it deflates it, so the peer's HPACK table stays in step: - // https://github.com/nodejs/node/blob/v26.3.0/deps/nghttp2/lib/nghttp2_session.c#L2095-L2101 - if staged.deflate_bound() + StreamPriority::BYTE_SIZE + // nghttp2 refuses the block on its pre-compression bound (it always counts the priority + // fields) before it deflates it, so the peer's HPACK table stays in step: + // https://github.com/nodejs/node/blob/v26.3.0/deps/nghttp2/lib/nghttp2_session.c#L2095-L2101 + if let Some(staged) = &staged + && staged.deflate_bound() + StreamPriority::BYTE_SIZE > this.max_send_header_block_length.get() as usize - { - if this.is_server.get() { - let identifier = stream.get_identifier(); - identifier.ensure_still_alive(); - // The JS handler closes the session gracefully, like node's onFrameError. - this.dispatch_with_2_extra( - JSH2FrameParser::Gc::onFrameError, - identifier, - JSValue::js_number(FrameType::HTTP_FRAME_HEADERS as u8 as f64), - JSValue::js_number(ErrorCode::FRAME_SIZE_ERROR.0 as f64), - ); - // Nothing can follow a refused final response: DATA without HEADERS is a - // connection error for the peer. A refused 1xx block leaves the stream open - // for the final response. - if !staged.is_informational() { - this.end_stream(&mut stream, ErrorCode::FRAME_SIZE_ERROR); - } - return Ok(JSValue::js_number(stream_id as f64)); - } - - // Client: the request never reached the wire, nghttp2 refuses it locally. - stream.state = StreamState::CLOSED; - stream.rst_code = ErrorCode::REFUSED_STREAM.0; - + { + if this.is_server.get() { + let identifier = stream.get_identifier(); + identifier.ensure_still_alive(); + // The JS handler closes the session gracefully, like node's onFrameError. this.dispatch_with_2_extra( JSH2FrameParser::Gc::onFrameError, - stream.get_identifier(), + identifier, JSValue::js_number(FrameType::HTTP_FRAME_HEADERS as u8 as f64), JSValue::js_number(ErrorCode::FRAME_SIZE_ERROR.0 as f64), ); - - this.dispatch_with_extra( - JSH2FrameParser::Gc::onStreamError, - stream.get_identifier(), - JSValue::js_number(stream.rst_code as f64), - ); + // Nothing can follow a refused final response: DATA without HEADERS is a + // connection error for the peer. A refused 1xx block leaves the stream open for + // the final response. + if !staged.is_informational() { + this.end_stream(&mut stream, ErrorCode::FRAME_SIZE_ERROR); + } return Ok(JSValue::js_number(stream_id as f64)); } + // Client: the request never reached the wire, nghttp2 refuses it locally. + stream.state = StreamState::CLOSED; + stream.rst_code = ErrorCode::REFUSED_STREAM.0; + + this.dispatch_with_2_extra( + JSH2FrameParser::Gc::onFrameError, + stream.get_identifier(), + JSValue::js_number(FrameType::HTTP_FRAME_HEADERS as u8 as f64), + JSValue::js_number(ErrorCode::FRAME_SIZE_ERROR.0 as f64), + ); + + this.dispatch_with_extra( + JSH2FrameParser::Gc::onStreamError, + stream.get_identifier(), + JSValue::js_number(stream.rst_code as f64), + ); + return Ok(JSValue::js_number(stream_id as f64)); + } + + if let Some(staged) = &staged { for (name, value, never_index) in staged.iter() { if let Err(err) = this.encode_header_into_list(&mut encoded_headers, name, value, never_index) From 1d6085f23818db2e3dd4c73fc147a9ffbe3cfacc Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 21:57:32 +0000 Subject: [PATCH 15/21] node:http2: one-line comments on the send limit path --- src/js/node/http2.ts | 3 +-- src/runtime/api/bun/h2_frame_parser.rs | 15 ++++----------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index 720ce3433518..010c5689a6f5 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -6565,8 +6565,7 @@ function onErrorSecureServerSession(err, socket) { function emitFrameErrorEventNT(stream, frameType, errorCode) { stream.emit("frameError", frameType, errorCode); } -// node's onFrameError: the stream and the session stay usable for the rest of the tick, then the -// session closes gracefully so the other streams finish. Both calls are no-ops once closed. +// node's onFrameError (lib/internal/http2/core.js#L661-L681 at v26.3.0). Both calls are no-ops once closed. function closeAfterFrameError(session: ServerHttp2Session, stream: ServerHttp2Stream, code: number) { stream.close(code); session.close(); diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index 7853c4687664..f3ba07bcfcbf 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -1962,9 +1962,7 @@ impl AbortListener for SignalRef { } } -/// The fields of one outbound header block, staged so the block can be measured before any -/// field reaches the HPACK encoder. A field that is encoded and never sent desyncs the peer's -/// dynamic table for the rest of the connection. +/// Header fields staged so a block can be measured before any of them reaches the HPACK encoder. #[derive(Default)] struct HeaderList { bytes: Vec, @@ -2000,8 +1998,7 @@ impl HeaderList { Ok(()) } - /// Same formula as nghttp2_hd_deflate_bound: - /// https://github.com/nodejs/node/blob/v26.3.0/deps/nghttp2/lib/nghttp2_hd.c#L1578-L1603 + /// nghttp2_hd_deflate_bound: https://github.com/nodejs/node/blob/v26.3.0/deps/nghttp2/lib/nghttp2_hd.c#L1578-L1603 fn deflate_bound(&self) -> usize { 12 + self.fields.len() * 12 + self.bytes.len() } @@ -7167,9 +7164,7 @@ impl H2FrameParser { flags |= HeadersFrameFlags::PRIORITY as u8; } - // nghttp2 refuses the block on its pre-compression bound (it always counts the priority - // fields) before it deflates it, so the peer's HPACK table stays in step: - // https://github.com/nodejs/node/blob/v26.3.0/deps/nghttp2/lib/nghttp2_session.c#L2095-L2101 + // Like nghttp2, priority bytes always counted: https://github.com/nodejs/node/blob/v26.3.0/deps/nghttp2/lib/nghttp2_session.c#L2095-L2101 if let Some(staged) = &staged && staged.deflate_bound() + StreamPriority::BYTE_SIZE > this.max_send_header_block_length.get() as usize @@ -7184,9 +7179,7 @@ impl H2FrameParser { JSValue::js_number(FrameType::HTTP_FRAME_HEADERS as u8 as f64), JSValue::js_number(ErrorCode::FRAME_SIZE_ERROR.0 as f64), ); - // Nothing can follow a refused final response: DATA without HEADERS is a - // connection error for the peer. A refused 1xx block leaves the stream open for - // the final response. + // DATA cannot follow a refused final response. A refused 1xx block keeps the stream open. if !staged.is_informational() { this.end_stream(&mut stream, ErrorCode::FRAME_SIZE_ERROR); } From b0229882645eb11ee10e386e8e9384dc1f257abf Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 22:33:59 +0000 Subject: [PATCH 16/21] node:http2: free a refused client stream and reset an unresponded server stream before close() A client request refused for maxSendHeaderBlockLength set the stream to CLOSED but kept its map entry and JS context until the session ended. Free them the way end_stream() does. The deferred close after a refused 1xx block ended the writable of a stream that had sent no HEADERS, which wrote an empty DATA END_STREAM before any response. Reset the stream first, so close() sends nothing. The trailer path's GOAWAY now carries the last peer-initiated stream id. --- src/js/node/http2.ts | 4 ++ src/runtime/api/bun/h2_frame_parser.rs | 11 +++--- test/js/node/http2/node-http2.test.js | 55 ++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 5 deletions(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index 010c5689a6f5..a23c01d87f7d 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -6567,6 +6567,10 @@ function emitFrameErrorEventNT(stream, frameType, errorCode) { } // node's onFrameError (lib/internal/http2/core.js#L661-L681 at v26.3.0). Both calls are no-ops once closed. function closeAfterFrameError(session: ServerHttp2Session, stream: ServerHttp2Stream, code: number) { + if (!stream.destroyed && !stream.headersSent) { + // No response HEADERS went out, so the writable must not end with a DATA frame. Reset first. + session[bunHTTP2Native]?.rstStream(stream.id, code); + } stream.close(code); session.close(); } diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index f3ba07bcfcbf..b18e4ad827b1 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -5756,7 +5756,7 @@ impl H2FrameParser { triggering_id, ErrorCode::NO_ERROR, b"", - this.last_stream_id.get(), + this.last_peer_stream_id.get(), true, ); Ok(Some(JSValue::UNDEFINED)) @@ -7189,17 +7189,18 @@ impl H2FrameParser { // Client: the request never reached the wire, nghttp2 refuses it locally. stream.state = StreamState::CLOSED; stream.rst_code = ErrorCode::REFUSED_STREAM.0; - + let identifier = stream.get_identifier(); + identifier.ensure_still_alive(); + stream.free_resources::(this); this.dispatch_with_2_extra( JSH2FrameParser::Gc::onFrameError, - stream.get_identifier(), + identifier, JSValue::js_number(FrameType::HTTP_FRAME_HEADERS as u8 as f64), JSValue::js_number(ErrorCode::FRAME_SIZE_ERROR.0 as f64), ); - this.dispatch_with_extra( JSH2FrameParser::Gc::onStreamError, - stream.get_identifier(), + identifier, JSValue::js_number(stream.rst_code as f64), ); return Ok(JSValue::js_number(stream_id as f64)); diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index 9ae489e6eb98..09632e583ee9 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -5985,6 +5985,61 @@ it("respond() delivers the response after additionalHeaders() exceeds maxSendHea } }); +it("a refused 1xx block followed by a late respond() resets the stream without a DATA frame", async () => { + // Verified against node v26.3.0: the deferred close resets the stream with FRAME_SIZE_ERROR, + // the late respond() throws ERR_HTTP2_INVALID_STREAM, and the client session stays healthy. + // An empty DATA END_STREAM before any HEADERS would be a protocol error for the peer. + const server = http2.createServer({ maxSendHeaderBlockLength: 200 }); + try { + const serverEvents = []; + const serverStreamClosed = Promise.withResolvers(); + server.on("stream", async stream => { + stream.on("error", e => serverEvents.push(`error ${e.code} ${e.message}`)); + stream.on("frameError", (type, code) => serverEvents.push(`frameError type=${type} code=${code}`)); + stream.on("close", () => serverStreamClosed.resolve(stream.rstCode)); + stream.additionalHeaders({ ":status": 103, "x-big": Buffer.alloc(1000, "b").toString() }); + await new Promise(resolve => setImmediate(() => setImmediate(resolve))); + try { + stream.respond({ ":status": 200 }); + serverEvents.push("respond ok"); + } catch (e) { + serverEvents.push(`respond threw ${e.code}`); + } + }); + const port = await new Promise(resolve => server.listen(0, () => resolve(server.address().port))); + const client = http2.connect(`http://localhost:${port}`); + const sessionErrors = []; + client.on("error", e => sessionErrors.push(e.code)); + const clientClosed = new Promise(resolve => client.on("close", resolve)); + + const req = client.request({ ":path": "/" }); + const reqResult = new Promise(resolve => { + const result = { response: false, error: undefined }; + req.on("response", () => (result.response = true)); + req.on("error", e => (result.error = e.message)); + req.on("close", () => resolve({ ...result, rstCode: req.rstCode })); + }); + req.resume(); + req.end(); + + const [reqRes, serverRstCode] = await Promise.all([reqResult, serverStreamClosed.promise, clientClosed]); + expect(reqRes).toEqual({ + response: false, + error: "Stream closed with error code NGHTTP2_FRAME_SIZE_ERROR", + rstCode: http2.constants.NGHTTP2_FRAME_SIZE_ERROR, + }); + expect(serverRstCode).toBe(http2.constants.NGHTTP2_FRAME_SIZE_ERROR); + expect(sessionErrors).toEqual([]); + expect(serverEvents).toEqual([ + "frameError type=1 code=6", + "error ERR_HTTP2_STREAM_ERROR Stream closed with error code NGHTTP2_FRAME_SIZE_ERROR", + "respond threw ERR_HTTP2_INVALID_STREAM", + ]); + } finally { + server.close(); + } +}); + it("a client request over maxSendHeaderBlockLength leaves the session usable", async () => { // Verified against node v26.3.0: the request is refused locally with 'frameError' and // REFUSED_STREAM. The next request reuses an indexed field, so the server only decodes it if From 629e14fbb8a22d3fc16073591ef9bf336ddde211 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 23:03:52 +0000 Subject: [PATCH 17/21] node:http2: a refused client request never writes RST_STREAM Its HEADERS never reached the wire. Once the freed entry is evicted, the deferred rstStream() from _destroy would write a raw RST_STREAM for an id the peer considers idle. Mark the stream as never announced, as the queued-request path does. --- src/js/node/http2.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index a23c01d87f7d..9773ea1f5e48 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -5016,6 +5016,9 @@ class ClientHttp2Session extends Http2Session { frameError: withStreamFrame( (self: ClientHttp2Session, stream: ClientHttp2Stream, frameType: number, errorCode: number) => { if (!self || typeof stream !== "object") return; + // The refused HEADERS never reached the wire, so the teardown must not write RST_STREAM + // for an id the peer considers idle. + stream[kNeverAnnounced] = true; // Emit the frameError event with the frame type and error code process.nextTick(emitFrameErrorEventNT, stream, frameType, errorCode); }, From b88692d511129ca89881dbf1ca15dee2d15beca5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 23:11:47 +0000 Subject: [PATCH 18/21] node:http2: pin the maxSendHeaderBlockLength threshold in a test --- src/js/node/http2.ts | 3 +-- test/js/node/http2/node-http2.test.js | 34 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index 9773ea1f5e48..8614511b9b65 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -5016,8 +5016,7 @@ class ClientHttp2Session extends Http2Session { frameError: withStreamFrame( (self: ClientHttp2Session, stream: ClientHttp2Stream, frameType: number, errorCode: number) => { if (!self || typeof stream !== "object") return; - // The refused HEADERS never reached the wire, so the teardown must not write RST_STREAM - // for an id the peer considers idle. + // The refused HEADERS never reached the wire: no RST_STREAM for an idle id. stream[kNeverAnnounced] = true; // Emit the frameError event with the frame type and error code process.nextTick(emitFrameErrorEventNT, stream, frameType, errorCode); diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index 09632e583ee9..47ceae69e950 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -6040,6 +6040,40 @@ it("a refused 1xx block followed by a late respond() resets the stream without a } }); +it("maxSendHeaderBlockLength is compared against nghttp2's pre-compression bound", async () => { + // respond({ ":status": "200" }) stages one field of 10 bytes: the bound is 12 + 12 + 10 = 34, + // plus 5 priority bytes. Verified against node v26.3.0: 39 delivers the response, 38 refuses it. + const probe = async limit => { + const server = http2.createServer({ maxSendHeaderBlockLength: limit }); + try { + server.on("stream", stream => { + stream.on("error", () => {}); + stream.respond({ ":status": 200 }, { sendDate: false, endStream: true }); + }); + const port = await new Promise(resolve => server.listen(0, () => resolve(server.address().port))); + const client = http2.connect(`http://localhost:${port}`); + client.on("error", () => {}); + try { + const req = client.request({ ":path": "/" }); + return await new Promise(resolve => { + let status; + req.on("response", headers => (status = headers[":status"])); + req.on("error", () => {}); + req.on("close", () => resolve({ status, rstCode: req.rstCode })); + req.resume(); + req.end(); + }); + } finally { + client.destroy(); + } + } finally { + server.close(); + } + }; + expect(await probe(39)).toEqual({ status: 200, rstCode: 0 }); + expect(await probe(38)).toEqual({ status: undefined, rstCode: http2.constants.NGHTTP2_FRAME_SIZE_ERROR }); +}); + it("a client request over maxSendHeaderBlockLength leaves the session usable", async () => { // Verified against node v26.3.0: the request is refused locally with 'frameError' and // REFUSED_STREAM. The next request reuses an indexed field, so the server only decodes it if From c2fc93721ee9a531351e0b988aebafd6fef07d84 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 23:25:37 +0000 Subject: [PATCH 19/21] ci: retrigger From 3bd72aac4df76c398145a212daf7b8cc45de6d94 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 23:26:31 +0000 Subject: [PATCH 20/21] node:http2: test that a refused client request is freed safely The client branch of request() frees the refused stream before it calls into JS. The test closes, writes to, destroys and aborts such streams, with forced GCs, before and after the entry is evicted. The ASan lane turns a stale access into a crash. --- test/js/node/http2/node-http2.test.js | 75 +++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index 47ceae69e950..b76d3a203246 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -6125,6 +6125,81 @@ it("a client request over maxSendHeaderBlockLength leaves the session usable", a } }); +it("a refused client request is freed safely: close(), a body, destroy() and a late abort do not touch it", async () => { + // The native side frees the refused stream before it calls into JS, and the next inbound + // frames evict its entry. Nothing the caller does afterwards may reach the freed stream + // (ASan builds turn a stale access into a crash). + const server = http2.createServer(); + try { + server.on("stream", stream => { + stream.respond({ ":status": 200 }); + stream.end(); + }); + const port = await new Promise(resolve => server.listen(0, () => resolve(server.address().port))); + const client = http2.connect(`http://localhost:${port}`, { maxSendHeaderBlockLength: 300 }); + const sessionErrors = []; + client.on("error", e => sessionErrors.push(e.message)); + await new Promise(resolve => client.once("connect", resolve)); + const big = Buffer.alloc(400, "b").toString(); + const settle = req => + new Promise(resolve => { + const result = { status: undefined, error: undefined }; + req.on("response", headers => (result.status = headers[":status"])); + req.on("error", e => (result.error = e.message)); + req.on("close", () => resolve({ ...result, rstCode: req.rstCode })); + req.resume(); + }); + + const closed = client.request({ ":path": "/closed", "x-big": big }); + const closedResult = settle(closed); + closed.close(); + Bun.gc(true); + const controller = new AbortController(); + const aborted = client.request({ ":path": "/aborted", "x-big": big }, { signal: controller.signal }); + const abortedResult = settle(aborted); + const posted = client.request({ ":path": "/posted", ":method": "POST", "x-big": big }); + const postedResult = settle(posted); + posted.write("body"); + posted.end("more"); + const destroyed = client.request({ ":path": "/destroyed", "x-big": big }); + const destroyedResult = settle(destroyed); + destroyed.on("frameError", () => { + Bun.gc(true); + destroyed.destroy(); + }); + const refused = await Promise.all([closedResult, abortedResult, postedResult, destroyedResult]); + + // A response is inbound traffic, which evicts the freed entries. + const first = client.request({ ":path": "/first" }); + const firstResult = await settle(first); + controller.abort(); + for (const req of [closed, aborted, posted, destroyed]) { + req.close(); + req.destroy(); + } + Bun.gc(true); + const second = client.request({ ":path": "/second" }); + const secondResult = await settle(second); + client.destroy(); + + const refusedError = "Stream closed with error code NGHTTP2_REFUSED_STREAM"; + const rstCode = http2.constants.NGHTTP2_REFUSED_STREAM; + expect({ refused, firstResult, secondResult, sessionErrors }).toEqual({ + refused: [ + { status: undefined, error: refusedError, rstCode }, + { status: undefined, error: refusedError, rstCode }, + { status: undefined, error: refusedError, rstCode }, + { status: undefined, error: undefined, rstCode }, + ], + firstResult: { status: 200, error: undefined, rstCode: 0 }, + secondResult: { status: 200, error: undefined, rstCode: 0 }, + sessionErrors: [], + }); + } finally { + server.close(); + } +}); + it("an oversized respond() over a JS Duplex transport still puts RST_STREAM and GOAWAY on the wire", async () => { // A session on a user Duplex has no native socket: the RST_STREAM is corked by the native // side and must still reach the transport. From 4566950307d45a468f57009b3b1f0c6792e23216 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 23:40:08 +0000 Subject: [PATCH 21/21] node:http2: close(code) on a refused client request sends no RST_STREAM The never-announced check lived in _destroy only. close() with a code, and the ready-deferred reset of a queued request, still scheduled rstStream() for an id the peer never saw. --- src/js/node/http2.ts | 3 +++ test/js/node/http2/node-http2.test.js | 20 ++++++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index 8614511b9b65..046578bfbb33 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -2215,6 +2215,7 @@ function streamOnResume(this: Http2Stream) { // A close() on a stream that has not been submitted yet (no id): the RST_STREAM has to follow the // HEADERS frame, which is sent when the queued request becomes ready (node's finishCloseStream). function sendRstOnReady(this: Http2Stream, session: Http2Session, code: number) { + if (this[kNeverAnnounced]) return; setImmediate(rstNextTick.bind(session, this.id, code)); } function uncorkNT(stream: Http2Stream) { @@ -2542,6 +2543,8 @@ class Http2Stream extends Duplex { // No id yet (the HEADERS frame is still queued behind connect/concurrency limits): the // RST_STREAM has to be sent after the HEADERS frame, once the id is assigned. this.once("ready", sendRstOnReady.bind(this, session, code)); + } else if (this[kNeverAnnounced]) { + // The peer never saw this id: nothing to reset. } else if (this.writableFinished || code) { setImmediate(rstNextTick.bind(session, this.#id, code)); } else { diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index b76d3a203246..83dd93b34f41 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -6167,13 +6167,27 @@ it("a refused client request is freed safely: close(), a body, destroy() and a l Bun.gc(true); destroyed.destroy(); }); - const refused = await Promise.all([closedResult, abortedResult, postedResult, destroyedResult]); + // close(code) must not put RST_STREAM on the wire for an id the peer never saw. + const cancelled = client.request({ ":path": "/cancelled", "x-big": big }); + const cancelledResult = settle(cancelled); + cancelled.close(http2.constants.NGHTTP2_CANCEL); + const late = client.request({ ":path": "/late", "x-big": big }); + const lateResult = settle(late); + late.on("frameError", () => late.close(http2.constants.NGHTTP2_INTERNAL_ERROR)); + const refused = await Promise.all([ + closedResult, + abortedResult, + postedResult, + destroyedResult, + cancelledResult, + lateResult, + ]); // A response is inbound traffic, which evicts the freed entries. const first = client.request({ ":path": "/first" }); const firstResult = await settle(first); controller.abort(); - for (const req of [closed, aborted, posted, destroyed]) { + for (const req of [closed, aborted, posted, destroyed, cancelled, late]) { req.close(); req.destroy(); } @@ -6190,6 +6204,8 @@ it("a refused client request is freed safely: close(), a body, destroy() and a l { status: undefined, error: refusedError, rstCode }, { status: undefined, error: refusedError, rstCode }, { status: undefined, error: undefined, rstCode }, + { status: undefined, error: refusedError, rstCode }, + { status: undefined, error: refusedError, rstCode }, ], firstResult: { status: 200, error: undefined, rstCode: 0 }, secondResult: { status: 200, error: undefined, rstCode: 0 },