From 6502029bfbe714cae63f48303619d99426c55763 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 11 Sep 2026 03:42:55 +0000 Subject: [PATCH 1/2] http2: send RST_STREAM when a server stream is reset stream.close(code) and stream.destroy() scheduled an RST_STREAM but let the writable side announce a clean end-of-stream first. On a server stream whose request body is complete (any GET) that drives the native stream to CLOSED, where the reset is dropped: the peer read an aborted response as a complete one. On a stream whose request body is still open the reset did go out, behind a spurious END_STREAM. Mark the stream as having a reset pending before closing the writable side. _final then settles without writing END_STREAM, and isFinalWrite no longer lets the last buffered chunk carry it either: with a chunk still queued in the Writable when close() runs, that chunk reaches _write as the final one and would end the stream on its DATA frame. close() and _destroy() each schedule the reset, and the native map-miss fallback writes it again once the first has evicted the stream. Make the submission first-wins on the stream rather than letting one path stand down for the other: close(NO_ERROR) waits for 'finish' to reset, and a destroy() that pre-empts 'finish' must still send it. A server stream that reaches _final without having sent HEADERS has no frame it may write (RFC 9113 8.1); settle the writable instead of emitting DATA ahead of any response. respond(), respondWithFile() and respondWithFD() refuse a closed stream and a stream whose session is already gone, as node does. An inbound RST_STREAM(NO_ERROR) is a clean end of stream: route it to onStreamEnd so 'end' fires before 'close', instead of destroying the stream with the queued EOF still unread. --- src/js/node/http2.ts | 51 ++-- src/runtime/api/bun/h2_frame_parser.rs | 7 + test/js/node/http2/h2-conformance.test.ts | 303 +++++++++++++++++++++- 3 files changed, 343 insertions(+), 18 deletions(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index bd2cb588e233..c7c007a9d91b 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -2038,6 +2038,12 @@ enum StreamState { // callback). Until then no 'error' listener can exist, so stream errors must not be emitted: // node never constructs the JS stream object before a complete header block arrives. Delivered = 1 << 8, // 100000000 = 256 + // close()/destroy() scheduled an RST_STREAM: it replaces the stream's end-of-stream, so + // neither _final nor the last DATA frame may put an END_STREAM on the wire ahead of it. + RstPending = 1 << 9, // 1000000000 = 512 + // The RST_STREAM went to the native side. close() and _destroy() each schedule one, and the + // native map-miss fallback would write it again once the first has evicted the stream. + RstSubmitted = 1 << 10, // 10000000000 = 1024 } // native.writeStream() return-value flag (mirrors WRITE_FLUSHED_WITHOUT_CALLBACK in // h2_frame_parser.rs): the chunk was handed to the socket without queueing and the engine did @@ -2064,7 +2070,8 @@ function isFinalWrite(stream: Http2Stream, pendingLength: number) { return ( (stream._writableState.ending || stream[kEndingWithChunk] === true) && stream.writableLength === pendingLength && - !stream[bunHTTP2WaitForTrailers] + !stream[bunHTTP2WaitForTrailers] && + (stream[bunHTTP2StreamStatus] & StreamState.RstPending) === 0 ); } @@ -2204,8 +2211,9 @@ function markStreamClosed(stream: Http2Stream) { markWritableDone(stream); } } -function rstNextTick(id: number, rstCode: number) { - const session = this as Http2Session; +function rstNextTick(this: Http2Stream, session: Http2Session, id: number, rstCode: number) { + if ((this[bunHTTP2StreamStatus] & StreamState.RstSubmitted) !== 0) return; + this[bunHTTP2StreamStatus] |= StreamState.RstSubmitted; session[bunHTTP2Native]?.rstStream(id, rstCode); } // node streamOnPause/streamOnResume (lib/internal/http2/core.js): the readable's flow state @@ -2225,7 +2233,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) { - setImmediate(rstNextTick.bind(session, this.id, code)); + setImmediate(rstNextTick.bind(this, session, this.id, code)); } function uncorkNT(stream: Http2Stream) { stream.uncork(); @@ -2537,6 +2545,7 @@ class Http2Stream extends Duplex { this.push(null); } const { ending } = this._writableState; + this[bunHTTP2StreamStatus] |= StreamState.RstPending; if (!ending) { // If the writable side of the Http2Stream is still open, emit the // 'aborted' event and set the aborted flag. @@ -2553,9 +2562,9 @@ class Http2Stream extends Duplex { // 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.writableFinished || code) { - setImmediate(rstNextTick.bind(session, this.#id, code)); + setImmediate(rstNextTick.bind(this, session, this.#id, code)); } else { - this.once("finish", rstNextTick.bind(session, this.#id, code)); + this.once("finish", rstNextTick.bind(this, session, this.#id, code)); } // node destroys the stream once both halves have finished; without this a stream closed // while idle never emits 'close'. @@ -2582,6 +2591,7 @@ class Http2Stream extends Duplex { this[kAborted] = true; this.emit("aborted"); } + this[bunHTTP2StreamStatus] |= StreamState.RstPending; // at this state destroyed will be true but we need to close the writable side this._writableState.destroyed = false; this.end(); @@ -2646,7 +2656,7 @@ class Http2Stream extends Duplex { // the deferred rstStream would be a guaranteed no-op host call per request. (rstCode !== 0 || (this[bunHTTP2StreamStatus] & StreamState.NativeClosed) === 0) ) { - setImmediate(rstNextTick.bind(session, this.#id, rstCode)); + setImmediate(rstNextTick.bind(this, session, this.#id, rstCode)); } // Diagnostics channels: published after the stream is closed and destroyed, with the same error @@ -2687,15 +2697,24 @@ class Http2Stream extends Duplex { if (session) { const native = session[bunHTTP2Native]; if (native) { - if (this instanceof ServerHttp2Stream && !this.headersSent && (this.id & 1) === 0) { - // A locally-pushed (even-id) stream ended before respond() (HEAD/endStream pushes): an - // empty DATA frame would precede the response HEADERS on the wire. respond() forces - // endStream for these streams, so END_STREAM rides on the HEADERS frame and the - // onStreamEnd(5) dispatch completes this callback through markWritableDone. - this[bunHTTP2StreamFinal] = callback; + if (this instanceof ServerHttp2Stream && !this.headersSent) { + // RFC 9113 §8.1: a response begins with HEADERS, so DATA here is a connection error. + // A pushed stream (even id) stashes the callback for the respond() that follows; a + // client-initiated stream just settles the writable, with any reset already scheduled. + if ((this.id & 1) === 0) { + this[bunHTTP2StreamFinal] = callback; + } else { + this[bunHTTP2StreamStatus] |= StreamState.FinalCalled | StreamState.WritableClosed; + callback(); + } return; } this[bunHTTP2StreamStatus] |= StreamState.FinalCalled; + if ((this[bunHTTP2StreamStatus] & StreamState.RstPending) !== 0) { + this[bunHTTP2StreamStatus] |= StreamState.WritableClosed; + callback(); + return; + } // When waitForTrailers is active, writing an empty DATA frame with // close=true emits a bare empty DATA frame (flags=0) to the wire // before the trailer/noTrailers path runs, which then emits ANOTHER @@ -3320,7 +3339,7 @@ class ServerHttp2Stream extends Http2Stream { } respondWithFile(path, headers, options) { - if (this.destroyed) { + if (this.destroyed || this.closed || this.session === undefined) { throw $ERR_HTTP2_INVALID_STREAM(); } if (this.headersSent) throw $ERR_HTTP2_HEADERS_SENT(); @@ -3380,7 +3399,7 @@ class ServerHttp2Stream extends Http2Stream { throw err; } } - if (this.destroyed) { + if (this.destroyed || this.closed || this.session === undefined) { throw $ERR_HTTP2_INVALID_STREAM(); } if (this.headersSent) throw $ERR_HTTP2_HEADERS_SENT(); @@ -3507,7 +3526,7 @@ class ServerHttp2Stream extends Http2Stream { session[bunHTTP2Native]?.request(this.id, undefined, headers, sensitiveNames); } respond(headers: any, options?: any) { - if (this.destroyed || this.session === undefined) { + if (this.destroyed || this.closed || this.session === undefined) { throw $ERR_HTTP2_INVALID_STREAM(); } diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index 0047486e23b6..f035da6a8344 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -4194,6 +4194,13 @@ impl crate::api::h2::connection::Sink for H2FrameParser { JSValue::UNDEFINED, JSValue::js_number(old_state as f64), ); + } else if code == ErrorCode::NO_ERROR.0 { + // A NO_ERROR reset is a clean end of stream: 'end' then 'close', no 'error'. + self.dispatch_with_extra( + JSH2FrameParser::Gc::onStreamEnd, + stream_ctx, + JSValue::js_number(StreamState::CLOSED as u8 as f64), + ); } else { self.dispatch_with_extra( JSH2FrameParser::Gc::onStreamError, diff --git a/test/js/node/http2/h2-conformance.test.ts b/test/js/node/http2/h2-conformance.test.ts index 414bf6c3b68b..c5fabb8cd271 100644 --- a/test/js/node/http2/h2-conformance.test.ts +++ b/test/js/node/http2/h2-conformance.test.ts @@ -8,10 +8,12 @@ // WINDOW_UPDATE, frame-size and stream-id rules. HPACK/HEADERS cases live in a sibling file. import { afterAll, beforeAll, describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, gcTick, normalizeBunSnapshot } from "harness"; +import { bunEnv, bunExe, gcTick, normalizeBunSnapshot, tempDir } from "harness"; import { once } from "node:events"; +import fs from "node:fs"; import http2 from "node:http2"; import net from "node:net"; +import path from "node:path"; import { Writable } from "node:stream"; const PREFACE = Buffer.from("PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n", "latin1"); @@ -1289,6 +1291,302 @@ describe("request pseudo-header requirements (RFC 9113 §8.3.1)", () => { }); }); +describe("server stream reset (RFC 9113 §6.4)", () => { + /** A one-shot h2c server plus a raw client that has handshaken and sent one request. */ + async function resetServer(method: "GET" | "POST", onStream: (stream: any) => void) { + const server = http2.createServer(); + server.on("stream", (stream: any) => { + stream.on("error", () => {}); + onStream(stream); + }); + server.listen(0); + await once(server, "listening"); + const c = await RawH2.connect((server.address() as net.AddressInfo).port); + c.sendPreface(); + c.sendEmptySettings(); + // A GET ends the request body, so the server stream is half-closed (remote); a POST leaves it + // open. Closing the local half lands them in different states, so both are covered. + c.sendFrame(FrameType.HEADERS, method === "GET" ? 0x4 | 0x1 : 0x4, 1, requestHeaderBlock(method)); + return { + c, + cleanup() { + c.destroy(); + server.close(); + }, + }; + } + + const endStreamOnStream1 = (f: Frame) => f.type === FrameType.DATA && f.streamId === 1 && (f.flags & 0x1) !== 0; + + test("close(code) mid-body sends RST_STREAM rather than a clean end-of-stream", async () => { + const { c, cleanup } = await resetServer("GET", stream => { + stream.respond({ ":status": 200 }); + stream.write("partial"); + stream.close(ErrorCode.INTERNAL_ERROR); + }); + try { + const rst = await c.waitFor(f => f.type === FrameType.RST_STREAM && f.streamId === 1); + expect(rst.payload.readUInt32BE(0)).toBe(ErrorCode.INTERNAL_ERROR); + // An END_STREAM ahead of the reset tells the peer the aborted response completed. + expect(c.frames.find(endStreamOnStream1)).toBeUndefined(); + } finally { + cleanup(); + } + }); + + test("close(code) while the request body is still open does not also end the stream cleanly", async () => { + const { c, cleanup } = await resetServer("POST", stream => { + stream.respond({ ":status": 200 }); + stream.write("partial"); + stream.close(ErrorCode.INTERNAL_ERROR); + }); + try { + const rst = await c.waitFor(f => f.type === FrameType.RST_STREAM && f.streamId === 1); + expect(rst.payload.readUInt32BE(0)).toBe(ErrorCode.INTERNAL_ERROR); + expect(c.frames.find(endStreamOnStream1)).toBeUndefined(); + } finally { + cleanup(); + } + }); + + test("close(code) before respond() sends RST_STREAM and nothing else", async () => { + const { c, cleanup } = await resetServer("GET", stream => { + stream.close(ErrorCode.REFUSED_STREAM); + }); + try { + const rst = await c.waitFor(f => f.type === FrameType.RST_STREAM && f.streamId === 1); + expect(rst.payload.readUInt32BE(0)).toBe(ErrorCode.REFUSED_STREAM); + // A bare DATA frame with no HEADERS ahead of it is not a response, it is a protocol error. + expect(c.frames.find(f => f.streamId === 1 && f.type !== FrameType.RST_STREAM)).toBeUndefined(); + } finally { + cleanup(); + } + }); + + test.each([ + [ErrorCode.NO_ERROR, "NO_ERROR"], + [ErrorCode.CANCEL, "CANCEL"], + [ErrorCode.INTERNAL_ERROR, "INTERNAL_ERROR"], + ])("close(%i) before respond() writes the reset exactly once", async code => { + const { c, cleanup } = await resetServer("GET", stream => { + stream.close(code); + }); + try { + await c.waitFor(f => f.type === FrameType.RST_STREAM && f.streamId === 1); + // close() and _destroy each schedule an rstNextTick; the second must not reach the wire via + // the native map-miss fallback. A PING round-trip flushes whatever the deferred tick queued. + c.sendFrame(FrameType.PING, 0, 0, Buffer.alloc(8)); + await c.waitFor(f => f.type === FrameType.PING && (f.flags & 0x1) !== 0); + const onStream1 = c.frames.filter(f => f.streamId === 1); + expect(onStream1.map(f => ({ type: f.type, code: f.payload.readUInt32BE(0) }))).toEqual([ + { type: FrameType.RST_STREAM, code }, + ]); + } finally { + cleanup(); + } + }); + + test("close(code) with a chunk still buffered does not end the stream on that chunk", async () => { + // The second write is still queued in the Writable when close() runs, so it reaches _write + // as the writable's last chunk, which would otherwise carry END_STREAM. + const { c, cleanup } = await resetServer("GET", stream => { + stream.respond({ ":status": 200 }); + stream.write("aaaa"); + stream.write("bbbb"); + stream.close(ErrorCode.INTERNAL_ERROR); + }); + try { + await c.waitFor(f => f.type === FrameType.RST_STREAM && f.streamId === 1); + c.sendFrame(FrameType.PING, 0, 0, Buffer.alloc(8)); + await c.waitFor(f => f.type === FrameType.PING && (f.flags & 0x1) !== 0); + const resets = c.frames.filter(f => f.type === FrameType.RST_STREAM && f.streamId === 1); + expect(resets.map(f => f.payload.readUInt32BE(0))).toEqual([ErrorCode.INTERNAL_ERROR]); + expect(c.frames.find(endStreamOnStream1)).toBeUndefined(); + } finally { + cleanup(); + } + }); + + test("destroy() after close(NO_ERROR) still resets a stream whose body cannot finish", async () => { + // close(NO_ERROR) waits for 'finish' before resetting. The body is larger than the flow-control + // window the client never reopens, so 'finish' cannot come and destroy() has to send the reset. + const { c, cleanup } = await resetServer("GET", stream => { + stream.respond({ ":status": 200 }); + stream.write(Buffer.alloc(200_000, 0x61)); + stream.close(ErrorCode.NO_ERROR); + stream.destroy(); + }); + try { + const rst = await c.waitFor(f => f.type === FrameType.RST_STREAM && f.streamId === 1); + expect(rst.payload.readUInt32BE(0)).toBe(ErrorCode.NO_ERROR); + expect(c.frames.find(endStreamOnStream1)).toBeUndefined(); + } finally { + cleanup(); + } + }); + + test("end() without respond() sends nothing on the stream", async () => { + // A response MUST begin with a HEADERS frame: _final on a server stream that never sent one + // has no frame it is allowed to write. node's shutdown is a no-op in nghttp2 here. + const { c, cleanup } = await resetServer("GET", stream => { + stream.end(); + }); + try { + c.sendFrame(FrameType.PING, 0, 0, Buffer.alloc(8)); + await c.waitFor(f => f.type === FrameType.PING && (f.flags & 0x1) !== 0); + expect(c.frames.filter(f => f.streamId === 1)).toEqual([]); + } finally { + cleanup(); + } + }); + + test("destroy() mid-body sends RST_STREAM(NO_ERROR) rather than a clean end-of-stream", async () => { + const { c, cleanup } = await resetServer("GET", stream => { + stream.respond({ ":status": 200 }); + stream.write("partial"); + stream.destroy(); + }); + try { + const rst = await c.waitFor(f => f.type === FrameType.RST_STREAM && f.streamId === 1); + expect(rst.payload.readUInt32BE(0)).toBe(ErrorCode.NO_ERROR); + expect(c.frames.find(endStreamOnStream1)).toBeUndefined(); + } finally { + cleanup(); + } + }); + + // `end(body)` with the body larger than the 65535-byte flow-control window leaves DATA queued, + // so the writable side is ending but `_final` has not run. Resetting from there must still not + // let a clean end-of-stream escape once the queue drains. + test.each([ + ["close", ErrorCode.INTERNAL_ERROR, (stream: any) => stream.close(ErrorCode.INTERNAL_ERROR)], + ["destroy", ErrorCode.NO_ERROR, (stream: any) => stream.destroy()], + ])("%s after end(body) resets the stream with data still queued", async (_name, expected, reset) => { + const { c, cleanup } = await resetServer("GET", stream => { + stream.respond({ ":status": 200 }); + stream.end(Buffer.alloc(200_000, 0x61)); + reset(stream); + }); + try { + // The client never sends WINDOW_UPDATE, so the tail of the body stays queued. + const rst = await c.waitFor(f => f.type === FrameType.RST_STREAM && f.streamId === 1); + expect(rst.payload.readUInt32BE(0)).toBe(expected); + expect(c.frames.find(endStreamOnStream1)).toBeUndefined(); + } finally { + cleanup(); + } + }); + + test("every response entry point refuses a closed stream", async () => { + using dir = tempDir("h2-closed-stream", { "body.txt": "hello" }); + const body = path.join(String(dir), "body.txt"); + const entryPoints: Array<[string, (stream: any) => void]> = [ + ["respond", stream => stream.respond({ ":status": 200 })], + ["respondWithFile", stream => stream.respondWithFile(body, { ":status": 200 })], + [ + "respondWithFD", + stream => { + const fd = fs.openSync(body, "r"); + try { + stream.respondWithFD(fd, { ":status": 200 }); + } finally { + // The guard throws before respondWithFD takes ownership of the descriptor. + fs.closeSync(fd); + } + }, + ], + ["additionalHeaders", stream => stream.additionalHeaders({ ":status": 103 })], + ]; + + const codes: Record = {}; + for (const [name, send] of entryPoints) { + const { c, cleanup } = await resetServer("GET", stream => { + stream.close(ErrorCode.REFUSED_STREAM); + try { + send(stream); + } catch (e: any) { + codes[name] = e.code; + } + }); + try { + await c.waitFor(f => f.type === FrameType.RST_STREAM && f.streamId === 1); + // The reset is all the peer gets: a closed stream emits no HEADERS and no DATA. + expect(c.frames.find(f => f.streamId === 1 && f.type !== FrameType.RST_STREAM)).toBeUndefined(); + } finally { + cleanup(); + } + } + + expect(codes).toEqual({ + respond: "ERR_HTTP2_INVALID_STREAM", + respondWithFile: "ERR_HTTP2_INVALID_STREAM", + respondWithFD: "ERR_HTTP2_INVALID_STREAM", + additionalHeaders: "ERR_HTTP2_INVALID_STREAM", + }); + }); + + test("an inbound RST_STREAM(NO_ERROR) ends the client stream cleanly", async () => { + const raw = await RawH2Server.listen(); + const client = http2.connect(`http://127.0.0.1:${raw.port}`); + client.on("error", () => {}); + try { + const req = client.request({ ":path": "/" }); + const events: string[] = []; + let error: any; + for (const ev of ["end", "close"] as const) req.on(ev, () => events.push(ev)); + req.on("error", (e: any) => (error = e)); + req.end(); + await raw.waitFor(f => f.type === FrameType.HEADERS && f.streamId === 1); + raw.sendFrame(FrameType.SETTINGS, 0, 0); + raw.sendFrame(FrameType.SETTINGS, 0x1, 0); + // The peer resets the stream with NO_ERROR before any response: a clean close, so the + // readable side still ends. Destroying on the reset would swallow 'end' and hang readers. + raw.sendFrame(FrameType.RST_STREAM, 0, 1, Buffer.alloc(4)); + // once() rejects on 'error', which is the point: no error may fire for a NO_ERROR reset. + await once(req, "close"); + expect({ events, code: error?.code, rstCode: req.rstCode }).toEqual({ + events: ["end", "close"], + code: undefined, + rstCode: ErrorCode.NO_ERROR, + }); + } finally { + client.destroy(); + raw.close(); + } + }); + + test("a response the server aborted reaches the client as an error, not a complete 200", async () => { + const server = http2.createServer(); + server.on("stream", (stream: any) => { + stream.on("error", () => {}); + stream.respond({ ":status": 200 }); + stream.write("partial"); + stream.close(ErrorCode.INTERNAL_ERROR); + }); + server.listen(0); + await once(server, "listening"); + const client = http2.connect(`http://127.0.0.1:${(server.address() as net.AddressInfo).port}`); + client.on("error", () => {}); + try { + const req = client.request({ ":path": "/" }); + let error: any; + let body = ""; + req.on("error", (e: any) => (error = e)); + req.on("data", (d: Buffer) => (body += d)); + // `once()` rejects on 'error', and the error here is the thing under test. + await new Promise(resolve => req.on("close", resolve)); + expect({ code: error?.code, rstCode: req.rstCode, body }).toEqual({ + code: "ERR_HTTP2_STREAM_ERROR", + rstCode: ErrorCode.INTERNAL_ERROR, + body: "partial", + }); + } finally { + client.destroy(); + server.close(); + } + }); +}); + // A stream nothing references any more can still survive a bounded number of collections: JSC scans // the machine stack conservatively and honors interior pointers, so a stale word left in a native // frame (seen on x64 as cell+0x84 in the microtask-drain frames; near-deterministic on aarch64) pins @@ -1418,12 +1716,13 @@ describe("inbound stream lifecycle", () => { stderr: "pipe", }); const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // The RST_STREAM(NO_ERROR) closes the stream cleanly, so the later client.destroy() has + // nothing left to cancel: no 'error' precedes 'close'. expect(normalizeBunSnapshot(stdout)).toMatchInlineSnapshot(` "wantTrailers id=1 toString:start toString:end sendTrailers:returned - req error ERR_HTTP2_STREAM_CANCEL req close" `); expect(proc.signalCode).toBeNull(); From fbda71ce62e1307b5eea3530c58f79fd03ef6a4b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 11 Sep 2026 06:05:49 +0000 Subject: [PATCH 2/2] http2: end the stream on HEADERS when respond() follows end() A server stream that reaches _final before any HEADERS went out settles its writable side without writing a frame. A respond() after that sent HEADERS with no END_STREAM, and nothing else would ever end the stream, so the peer got a response that never finished. node supports end() followed by respond(): SubmitResponse sets EMPTY_PAYLOAD when the stream is no longer writable, so END_STREAM rides the HEADERS frame. Do the same. --- src/js/node/http2.ts | 5 ++++- test/js/node/http2/h2-conformance.test.ts | 24 +++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index c7c007a9d91b..57b95793d10a 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -3632,7 +3632,10 @@ class ServerHttp2Stream extends Http2Stream { statusCode === HTTP_STATUS_NO_CONTENT || statusCode === HTTP_STATUS_RESET_CONTENT || statusCode === HTTP_STATUS_NOT_MODIFIED || - this.headRequest === true + this.headRequest === true || + // end() ran before any HEADERS went out, so _final settled the writable without a frame + // and nothing else will end the stream (node: SubmitResponse sets EMPTY_PAYLOAD). + (this[bunHTTP2StreamStatus] & StreamState.WritableClosed) !== 0 ) { // When endStream is true the HEADERS frame itself carries END_STREAM // and the stream moves to HALF_CLOSED_LOCAL inside native request(). diff --git a/test/js/node/http2/h2-conformance.test.ts b/test/js/node/http2/h2-conformance.test.ts index c5fabb8cd271..64de94c491d9 100644 --- a/test/js/node/http2/h2-conformance.test.ts +++ b/test/js/node/http2/h2-conformance.test.ts @@ -1440,6 +1440,30 @@ describe("server stream reset (RFC 9113 §6.4)", () => { } }); + test.each([ + ["in the same tick", (stream: any, respond: () => void) => respond()], + ["on a later turn", (stream: any, respond: () => void) => void stream.once("finish", respond)], + ])("respond() after end() %s ends the stream on its HEADERS frame", async (_when, schedule) => { + // end() first left the stream open with nothing sent, so the HEADERS frame is the only one + // that can still carry END_STREAM. Without it the peer gets a response that never finishes. + const closed = Promise.withResolvers(); + const { c, cleanup } = await resetServer("GET", stream => { + stream.on("close", closed.resolve); + stream.end(); + schedule(stream, () => stream.respond({ ":status": 200 })); + }); + try { + const headers = await c.waitFor(f => f.type === FrameType.HEADERS && f.streamId === 1); + expect(headers.flags & 0x1).toBe(0x1); + await closed.promise; + c.sendFrame(FrameType.PING, 0, 0, Buffer.alloc(8)); + await c.waitFor(f => f.type === FrameType.PING && (f.flags & 0x1) !== 0); + expect(c.frames.filter(f => f.streamId === 1).map(f => f.type)).toEqual([FrameType.HEADERS]); + } finally { + cleanup(); + } + }); + test("destroy() mid-body sends RST_STREAM(NO_ERROR) rather than a clean end-of-stream", async () => { const { c, cleanup } = await resetServer("GET", stream => { stream.respond({ ":status": 200 });