From 6e0431d9bc1093ccee981e14eead4555ef6983fe Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 13:04:59 +0000 Subject: [PATCH 1/4] node:http2: respond() reads only the options a response has ServerHttp2Stream#respond() passed the caller's options object to the native HEADERS writer. ClientHttp2Session.request() uses the same writer, so it read parent, weight, exclusive, silent and signal from the object. An out-of-range parent or weight closed the stream with no frame on the wire, and the client request never completed. A value of the wrong type threw after the header block was in the HPACK encoder, and the next response on the session failed to decode. respond() now copies its options and reads endStream, waitForTrailers and sendDate from the copy by truthiness, as node does. It gives the native writer a new object that holds endStream, waitForTrailers and paddingStrategy. Options that are not an object throw ERR_INVALID_ARG_TYPE, as in node. respondWithFile() and respondWithFD() go through respond(). They now check the options the same way and do not read endStream, as in node. Before, endStream: true sent the HEADERS frame with END_STREAM and no file. --- src/js/node/http2.ts | 56 +++--- test/js/node/http2/node-http2.test.js | 257 +++++++++++++++++++++++++- 2 files changed, 286 insertions(+), 27 deletions(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index abfd52512f03..4d155260ed7e 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -3315,6 +3315,7 @@ class ServerHttp2Stream extends Http2Stream { throw $ERR_HTTP2_INVALID_STREAM(); } if (this.headersSent) throw $ERR_HTTP2_HEADERS_SENT(); + assertIsObject(options, "options"); if ($isArray(headers)) { // node rejects the raw-array form here (only respond() accepts it) - same @@ -3333,7 +3334,8 @@ class ServerHttp2Stream extends Http2Stream { headers[HTTP2_HEADER_STATUS] = 200; } const statusCode = headers[HTTP2_HEADER_STATUS]; - options = { ...options }; + // node's file responders never read endStream: the file is the payload. + options = { ...options, endStream: false }; // Payload/DATA frames are not permitted in these cases if ( @@ -3375,6 +3377,7 @@ class ServerHttp2Stream extends Http2Stream { throw $ERR_HTTP2_INVALID_STREAM(); } if (this.headersSent) throw $ERR_HTTP2_HEADERS_SENT(); + assertIsObject(options, "options"); if ($isArray(headers)) { // node rejects the raw-array form here (only respond() accepts it) - same @@ -3403,7 +3406,8 @@ class ServerHttp2Stream extends Http2Stream { ) { throw $ERR_HTTP2_PAYLOAD_FORBIDDEN(statusCode); } - options = { ...options }; + // node's file responders never read endStream: the file is the payload. + options = { ...options, endStream: false }; if (options.offset !== undefined && typeof options.offset !== "number") { throw $ERR_INVALID_ARG_VALUE("options.offset", options.offset); } @@ -3508,6 +3512,15 @@ class ServerHttp2Stream extends Http2Stream { if (this.sentTrailers) { throw $ERR_HTTP2_TRAILERS_ALREADY_SENT(); } + assertIsObject(options, "options"); + // node's respond() (lib/internal/http2/core.js) copies its options, so only own enumerable + // keys count, then reads endStream, waitForTrailers and sendDate from the copy, each by + // truthiness. It ignores every other option. paddingStrategy is a bun extension. + options = { ...options }; + const sendDate = options.sendDate; + const paddingStrategy = options.paddingStrategy; + let endStream = !!options.endStream; + let waitForTrailers = !!options.waitForTrailers; // Raw (flat [name, value, ...] array) headers form: the pairs are encoded // on the wire in their given order; a default :status is prepended and a @@ -3541,8 +3554,7 @@ class ServerHttp2Stream extends Http2Stream { statusCode = 200; headers.unshift(HTTP2_HEADER_STATUS, statusCode); } - const sendDateOption = options?.sendDate; - if (!isDateSet && (sendDateOption == null || sendDateOption)) { + if (!isDateSet && (sendDate == null || sendDate)) { headers.push(HTTP2_HEADER_DATE, utcDate()); } rawHeadersList = headers; @@ -3598,7 +3610,6 @@ class ServerHttp2Stream extends Http2Stream { if (statusCode < 100 || statusCode > 599) { throw $ERR_HTTP2_STATUS_INVALID(statusCode); } - let endStream = !!options?.endStream; if ( endStream || statusCode === HTTP_STATUS_NO_CONTENT || @@ -3611,14 +3622,12 @@ class ServerHttp2Stream extends Http2Stream { // If waitForTrailers is ALSO true the native layer dispatches // onWantTrailers immediately after, whose JS handler calls // noTrailers → sendData("", true) and emits a spurious DATA frame on - // the already-half-closed stream (RFC 9113 §5.1 violation). Strip - // waitForTrailers here so the native never fires that path; the JS - // guard further down (`options?.waitForTrailers && !endStream`) only - // covers the `_final` side and runs AFTER the native call. - options = { ...options, endStream: true, waitForTrailers: false }; + // the already-half-closed stream (RFC 9113 §5.1 violation). Drop + // waitForTrailers here so the native never fires that path, and so + // `_final` never drives the wantTrailers path on a half-closed stream. endStream = true; + waitForTrailers = false; } - const sendDate = options?.sendDate; if (rawHeadersList === null && (sendDate == null || sendDate)) { const current_date = headers["date"]; if (current_date == null) { @@ -3627,21 +3636,16 @@ class ServerHttp2Stream extends Http2Stream { } const wireHeaders = rawHeadersList !== null ? rawHeadersList : headers; - if (typeof options === "undefined") { - session[bunHTTP2Native]?.request(this.id, undefined, wireHeaders, sensitiveNames); - } else { - session[bunHTTP2Native]?.request(this.id, undefined, wireHeaders, sensitiveNames, options); - // Only track waitForTrailers when the HEADERS frame above did NOT end - // the stream. Status codes 204/205/304 and HEAD requests force - // endStream=true earlier in this method, which means the native - // request() already wrote END_STREAM on the HEADERS frame — driving - // the wantTrailers path from `_final` on such a stream would call - // `noTrailers`/`emit("wantTrailers")` on an already-half-closed - // stream and corrupt state. Use optional chaining: `options` may be - // `null` here (typeof null === "object" enters this else branch). - if (options?.waitForTrailers && !endStream) { - this[bunHTTP2WaitForTrailers] = true; - } + // The native HEADERS writer is shared with ClientHttp2Session.request(). It reads the + // request-only options (parent, weight, exclusive, silent, signal) from the object it is + // given, and resets the stream or throws on a value it rejects. + session[bunHTTP2Native]?.request(this.id, undefined, wireHeaders, sensitiveNames, { + endStream, + waitForTrailers, + paddingStrategy, + }); + if (waitForTrailers) { + this[bunHTTP2WaitForTrailers] = true; } this.headersSent = true; if (onServerStreamFinishChannel.hasSubscribers) { diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index 7cf5c3e7ab3a..17d87fadc35a 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -1,5 +1,5 @@ import { jscDescribe } from "bun:jsc"; -import { bunEnv, bunExe, isASAN, isCI, isDebug, nodeExe } from "harness"; +import { bunEnv, bunExe, isASAN, isCI, isDebug, nodeExe, tempDir } from "harness"; import { createTest } from "node-harness"; import { AsyncLocalStorage } from "node:async_hooks"; import dc from "node:diagnostics_channel"; @@ -5024,6 +5024,261 @@ it("http2 stream.respond accepts raw-headers arrays; respondWithFD/respondWithFi server.close(); } }); + +// node's respond() reads endStream, waitForTrailers and sendDate from its options, each by +// truthiness, and nothing else (lib/internal/http2/core.js). parent, weight, exclusive, silent +// and signal are options of ClientHttp2Session.request(): a response that is given them goes out +// as if they were absent. respondWithFile() and respondWithFD() do not read endStream either. +describe("http2 response options", () => { + // Runs `respond(stream)` for each request on a fresh server and sends two requests, one after + // the other, over one client session. Resolves with what the client received for each, or with + // the first failure either side saw. The second response is decoded against the HPACK table + // the first one left behind, so it only arrives intact when the first respond() left the + // encoder in step with the wire. + async function requestTwice(respond) { + const { promise: failed, resolve: fail } = Promise.withResolvers(); + const server = http2.createServer(); + server.on("sessionError", err => fail(`server session error ${err.code}`)); + server.on("stream", stream => { + stream.on("error", err => fail(`server stream error ${err.code}`)); + try { + respond(stream); + } catch (err) { + fail(`threw ${err.code}`); + } + }); + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + const client = http2.connect(`http://127.0.0.1:${server.address().port}`); + client.on("error", err => fail(`client session error ${err.code}`)); + const get = () => + new Promise(resolve => { + const req = client.request({ ":path": "/" }); + const received = []; + req.setEncoding("utf8"); + req.on("error", err => fail(`client stream error ${err.code}`)); + req.on("response", headers => received.push(headers[":status"])); + req.on("data", chunk => received.push(chunk)); + req.on("trailers", headers => received.push(`x-trailer=${headers["x-trailer"]}`)); + req.on("end", () => resolve(received.join(" "))); + req.end(); + }); + try { + return await Promise.race([failed, (async () => [await get(), await get()])()]); + } finally { + client.destroy(); + server.close(); + } + } + + // Every shape at once: { name: result of requestTwice() }. + async function requestTwiceForEach(shapes, respond) { + const names = Object.keys(shapes); + const results = await Promise.all(names.map(name => requestTwice(stream => respond(stream, shapes[name])))); + return Object.fromEntries(names.map((name, i) => [name, results[i]])); + } + + const everyShape = (shapes, result) => Object.fromEntries(Object.keys(shapes).map(name => [name, result])); + + const requestOnlyOptions = { + "parent: -1": { parent: -1 }, + "parent: 0": { parent: 0 }, + "parent: 'x'": { parent: "x" }, + "weight: 0": { weight: 0 }, + "weight: 300": { weight: 300 }, + "weight: 'x'": { weight: "x" }, + "exclusive: 1": { exclusive: 1 }, + "silent: 1": { silent: 1 }, + "signal: {}": { signal: {} }, + "signal: aborted": { signal: AbortSignal.abort() }, + }; + + it("respond() ignores the options of request()", async () => { + const results = await requestTwiceForEach(requestOnlyOptions, (stream, options) => { + stream.respond({ ":status": 200 }, options); + stream.end("ok"); + }); + expect(results).toEqual(everyShape(requestOnlyOptions, ["200 ok", "200 ok"])); + }); + + it("respond() reads endStream and waitForTrailers by truthiness", async () => { + const shapes = { + "endStream: 0": { endStream: 0 }, + "endStream: ''": { endStream: "" }, + "endStream: 1": { endStream: 1 }, + "waitForTrailers: 0": { waitForTrailers: 0 }, + "waitForTrailers: 1": { waitForTrailers: 1 }, + "endStream: 1, waitForTrailers: 1": { endStream: 1, waitForTrailers: 1 }, + // node reads a copy of the options, so a key that is not own and enumerable does not count. + "inherited endStream: true": Object.create({ endStream: true }), + }; + const results = await requestTwiceForEach(shapes, (stream, options) => { + stream.on("wantTrailers", () => stream.sendTrailers({ "x-trailer": "sent" })); + stream.respond({ ":status": 200 }, options); + if (!stream.writableEnded) stream.end("ok"); + }); + expect(results).toEqual({ + "endStream: 0": ["200 ok", "200 ok"], + "endStream: ''": ["200 ok", "200 ok"], + "endStream: 1": ["200", "200"], + "waitForTrailers: 0": ["200 ok", "200 ok"], + "waitForTrailers: 1": ["200 ok x-trailer=sent", "200 ok x-trailer=sent"], + "endStream: 1, waitForTrailers: 1": ["200", "200"], + "inherited endStream: true": ["200 ok", "200 ok"], + }); + }); + + it("respondWithFile() and respondWithFD() ignore them too, and endStream", async () => { + using dir = tempDir("http2-response-options", { "body.txt": "file body" }); + const file = path.join(String(dir), "body.txt"); + const shapes = { + "weight: 0": { weight: 0 }, + "silent: 1": { silent: 1 }, + "endStream: true": { endStream: true }, + }; + const expected = everyShape(shapes, ["200 file body", "200 file body"]); + + const withFile = await requestTwiceForEach(shapes, (stream, options) => { + stream.respondWithFile(file, { ":status": 200 }, options); + }); + expect(withFile).toEqual(expected); + + // respondWithFD() leaves the descriptor to its caller. + const fds = []; + try { + const withFD = await requestTwiceForEach(shapes, (stream, options) => { + const fd = fs.openSync(file, "r"); + fds.push(fd); + stream.respondWithFD(fd, { ":status": 200 }, options); + }); + expect(withFD).toEqual(expected); + } finally { + for (const fd of fds) fs.closeSync(fd); + } + }); + + it("a response throws ERR_INVALID_ARG_TYPE for options that are not an object", async () => { + const notObjects = { + "null": null, + "5": 5, + "'str'": "str", + "true": true, + "[]": [], + }; + const received = { + "null": "Received null", + "5": "Received type number (5)", + "'str'": "Received type string ('str')", + "true": "Received type boolean (true)", + "[]": "Received an instance of Array", + }; + const thrown = {}; + const responses = await requestTwice(stream => { + for (const [name, options] of Object.entries(notObjects)) { + for (const [method, call] of [ + ["respond", () => stream.respond({ ":status": 200 }, options)], + ["respondWithFile", () => stream.respondWithFile(import.meta.path, { ":status": 200 }, options)], + ["respondWithFD", () => stream.respondWithFD(0, { ":status": 200 }, options)], + ]) { + try { + call(); + thrown[`${method}(headers, ${name})`] = "did not throw"; + } catch (err) { + thrown[`${method}(headers, ${name})`] = `${err.name} ${err.code}: ${err.message}`; + } + } + } + // A rejected call sends nothing, so the stream can still answer. + stream.respond({ ":status": 200 }); + stream.end("ok"); + }); + + const expected = {}; + for (const name of Object.keys(notObjects)) { + for (const method of ["respond", "respondWithFile", "respondWithFD"]) { + expected[`${method}(headers, ${name})`] = + `TypeError ERR_INVALID_ARG_TYPE: The "options" argument must be of type object. ${received[name]}`; + } + } + expect({ thrown, responses }).toEqual({ thrown: expected, responses: ["200 ok", "200 ok"] }); + }); + + it("a response HEADERS frame never carries a priority field", async () => { + const flagNames = flags => + Object.entries({ END_STREAM: 0x1, END_HEADERS: 0x4, PADDED: 0x8, PRIORITY: 0x20 }) + .filter(([, bit]) => (flags & bit) !== 0) + .map(([name]) => name) + .join(" | "); + + // Sends one GET on stream 1 from a raw socket and resolves with the flags of the HEADERS frame + // the server answers it with. + async function responseHeadersFlags(options) { + const { promise, resolve, reject } = Promise.withResolvers(); + const server = http2.createServer(); + server.on("stream", stream => { + stream.on("error", reject); + try { + stream.respond({ ":status": 200 }, options); + if (!stream.writableEnded) stream.end("ok"); + } catch (err) { + reject(err); + } + }); + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + const socket = net.connect(server.address().port, "127.0.0.1", () => { + socket.write(http2utils.kClientMagic); + socket.write(new http2utils.SettingsFrame(false).data); + // :method GET, :scheme http, :path /, :authority "x" (literal, name index 1), with + // END_HEADERS | END_STREAM. + const block = Buffer.from([0x82, 0x86, 0x84, 0x01, 0x01, 0x78]); + socket.write(new http2utils.HeadersFrame(1, block, 0, true, true).data); + }); + socket.on("error", reject); + socket.on("close", () => reject(new Error("the socket closed before the response HEADERS frame"))); + let buffered = Buffer.alloc(0); + socket.on("data", chunk => { + buffered = Buffer.concat([buffered, chunk]); + while (buffered.length >= 9) { + const length = buffered.readUIntBE(0, 3); + if (buffered.length < 9 + length) break; + const type = buffered[3]; + const flags = buffered[4]; + const streamId = buffered.readUInt32BE(5) & 0x7fffffff; + buffered = buffered.subarray(9 + length); + if (streamId !== 1) continue; + if (type === 1) return resolve(flagNames(flags)); + if (type === 3) return reject(new Error("the server reset the stream")); + } + }); + try { + return await promise; + } finally { + socket.destroy(); + server.close(); + } + } + + const shapes = { + "no options": undefined, + "weight: 16": { weight: 16 }, + "exclusive: true": { exclusive: true }, + "parent: 1": { parent: 1 }, + "endStream: 1": { endStream: 1 }, + // Not a node option: bun lets a single response override the session's paddingStrategy. + "paddingStrategy: PADDING_STRATEGY_MAX": { paddingStrategy: http2.constants.PADDING_STRATEGY_MAX }, + }; + const names = Object.keys(shapes); + const flags = await Promise.all(names.map(name => responseHeadersFlags(shapes[name]))); + expect(Object.fromEntries(names.map((name, i) => [name, flags[i]]))).toEqual({ + "no options": "END_HEADERS", + "weight: 16": "END_HEADERS", + "exclusive: true": "END_HEADERS", + "parent: 1": "END_HEADERS", + "endStream: 1": "END_STREAM | END_HEADERS", + "paddingStrategy: PADDING_STRATEGY_MAX": "END_HEADERS | PADDED", + }); + }); +}); + it("http2 client.request() on a destroyed or closed session uses the right error codes", async () => { // Node: destroyed session -> ERR_HTTP2_INVALID_SESSION, // closed (GOAWAY-pending) session -> ERR_HTTP2_GOAWAY_SESSION. From db3a37992aebfde3aeaeef48be7ec44b86a03194 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 18:21:10 +0000 Subject: [PATCH 2/4] node:http2: keep statCheck from changing the options of a file response respondWithFile() and respondWithFD() gave respond() the same options object that they give to statCheck as its third argument. A statCheck that set endStream on it made respond() put END_STREAM on the HEADERS frame, next to a content-length, and no file was sent. doSendFileFD() now reads waitForTrailers, sendDate and paddingStrategy before statCheck runs and gives respond() an object with only those. node does the same: it computes the stream options before statCheck and never reads endStream for a file response. A statCheck can no longer turn waitForTrailers on or sendDate off, as in node. This replaces the endStream: false that the two methods put on their options copy. That key also showed up in the statCheck argument. --- src/js/node/http2.ts | 19 ++++++++++++------- test/js/node/http2/node-http2.test.js | 8 ++++++++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index 4d155260ed7e..4dab100e56b0 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -2957,6 +2957,13 @@ function tryClose(fd) { function doSendFileFD(options, fd, headers, err, stat) { const onError = options.onError; const ownsFd = this[kOwnsFd] === true; + // node's file responders read waitForTrailers and sendDate before statCheck runs, and never read + // endStream: the file is the payload. statCheck is given `options` and can write to it. + const respondOptions = { + waitForTrailers: options.waitForTrailers, + sendDate: options.sendDate, + paddingStrategy: options.paddingStrategy, + }; if (err) { if (ownsFd && err.code !== "EBADF") { tryClose(fd); @@ -2964,7 +2971,7 @@ function doSendFileFD(options, fd, headers, err, stat) { if (onError) onError(err); else { - this.respond(headers, options); + this.respond(headers, respondOptions); this.destroy(streamErrorFromCode(NGHTTP2_INTERNAL_ERROR)); } return; @@ -2983,7 +2990,7 @@ function doSendFileFD(options, fd, headers, err, stat) { if (ownsFd) tryClose(fd); if (onError) onError(err); else { - this.respond(headers, options); + this.respond(headers, respondOptions); this.destroy(err); } return; @@ -3040,7 +3047,7 @@ function doSendFileFD(options, fd, headers, err, stat) { headers[HTTP2_HEADER_CONTENT_LENGTH] = statOptions.length; } try { - this.respond(headers, options); + this.respond(headers, respondOptions); } catch (err) { // respond() rejected the headers (e.g. a request pseudo-header in the response): the fd opened // for the file never reaches a read stream, so close it here before the stream is destroyed. @@ -3334,8 +3341,7 @@ class ServerHttp2Stream extends Http2Stream { headers[HTTP2_HEADER_STATUS] = 200; } const statusCode = headers[HTTP2_HEADER_STATUS]; - // node's file responders never read endStream: the file is the payload. - options = { ...options, endStream: false }; + options = { ...options }; // Payload/DATA frames are not permitted in these cases if ( @@ -3406,8 +3412,7 @@ class ServerHttp2Stream extends Http2Stream { ) { throw $ERR_HTTP2_PAYLOAD_FORBIDDEN(statusCode); } - // node's file responders never read endStream: the file is the payload. - options = { ...options, endStream: false }; + options = { ...options }; if (options.offset !== undefined && typeof options.offset !== "number") { throw $ERR_INVALID_ARG_VALUE("options.offset", options.offset); } diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index 17d87fadc35a..5d87a5a1fd40 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -5134,6 +5134,14 @@ describe("http2 response options", () => { "weight: 0": { weight: 0 }, "silent: 1": { silent: 1 }, "endStream: true": { endStream: true }, + // bun gives statCheck the options as a third argument. node gives it { offset, length } or + // nothing, and reads its stream options before statCheck runs. + "statCheck sets endStream and weight": { + statCheck(stat, headers, options) { + options.endStream = true; + options.weight = 0; + }, + }, }; const expected = everyShape(shapes, ["200 file body", "200 file body"]); From 5db8c46f6d601a7385e94f9b9363f04b8eb8908a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 19:05:52 +0000 Subject: [PATCH 3/4] node:http2: cut the new comments in respond() to one line each Each comment keeps the fact that the code cannot show: the options copy is deliberate, paddingStrategy is a bun extension, the native writer is shared with request(), and statCheck can write to the options it gets. The paragraph from #29075 keeps its text. Only the sentence that named a guard this branch removed is new. --- src/js/node/http2.ts | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index 4dab100e56b0..3c636a3bba67 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -2957,8 +2957,7 @@ function tryClose(fd) { function doSendFileFD(options, fd, headers, err, stat) { const onError = options.onError; const ownsFd = this[kOwnsFd] === true; - // node's file responders read waitForTrailers and sendDate before statCheck runs, and never read - // endStream: the file is the payload. statCheck is given `options` and can write to it. + // statCheck is handed `options` and may write to it. node reads these first and never reads endStream. const respondOptions = { waitForTrailers: options.waitForTrailers, sendDate: options.sendDate, @@ -3518,9 +3517,7 @@ class ServerHttp2Stream extends Http2Stream { throw $ERR_HTTP2_TRAILERS_ALREADY_SENT(); } assertIsObject(options, "options"); - // node's respond() (lib/internal/http2/core.js) copies its options, so only own enumerable - // keys count, then reads endStream, waitForTrailers and sendDate from the copy, each by - // truthiness. It ignores every other option. paddingStrategy is a bun extension. + // Like node, read a copy so only own enumerable keys count. paddingStrategy is a bun extension. options = { ...options }; const sendDate = options.sendDate; const paddingStrategy = options.paddingStrategy; @@ -3627,9 +3624,8 @@ class ServerHttp2Stream extends Http2Stream { // If waitForTrailers is ALSO true the native layer dispatches // onWantTrailers immediately after, whose JS handler calls // noTrailers → sendData("", true) and emits a spurious DATA frame on - // the already-half-closed stream (RFC 9113 §5.1 violation). Drop - // waitForTrailers here so the native never fires that path, and so - // `_final` never drives the wantTrailers path on a half-closed stream. + // the already-half-closed stream (RFC 9113 §5.1 violation). Strip + // waitForTrailers so neither the native layer nor `_final` takes the wantTrailers path. endStream = true; waitForTrailers = false; } @@ -3641,9 +3637,7 @@ class ServerHttp2Stream extends Http2Stream { } const wireHeaders = rawHeadersList !== null ? rawHeadersList : headers; - // The native HEADERS writer is shared with ClientHttp2Session.request(). It reads the - // request-only options (parent, weight, exclusive, silent, signal) from the object it is - // given, and resets the stream or throws on a value it rejects. + // The native writer is shared with request() and acts on parent, weight, exclusive, silent and signal. session[bunHTTP2Native]?.request(this.id, undefined, wireHeaders, sensitiveNames, { endStream, waitForTrailers, From 25f7f4323c7cb49461cd4e0aeb6067cdcef2c69f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 19:59:03 +0000 Subject: [PATCH 4/4] node:http2: check and copy the options of pushStream() like node pushStream() accepted options that are not an object, and it read endStream from the caller's object, so an inherited endStream ended the pushed writable. node throws ERR_INVALID_ARG_TYPE for the first and sends the push body for the second. pushStream() now calls assertIsObject() and reads endStream from a copy, as respond(), respondWithFile() and respondWithFD() do on this branch. The check sits after the push-allowed and nested-push checks, so those errors still win, as in node. --- src/js/node/http2.ts | 5 +++- test/js/node/http2/node-http2.test.js | 43 +++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index 3c636a3bba67..2771fa4b7d98 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -3226,6 +3226,9 @@ class ServerHttp2Stream extends Http2Stream { if (!this.pushAllowed) { throw $ERR_HTTP2_PUSH_DISABLED(); } + assertIsObject(options, "options"); + // Like node, read a copy so only own enumerable keys count. + options = { ...options }; const session = this[bunHTTP2Session]; const parser = session?.[bunHTTP2Native]; if (!parser) { @@ -3309,7 +3312,7 @@ class ServerHttp2Stream extends Http2Stream { if (headers[HTTP2_HEADER_METHOD] === HTTP2_METHOD_HEAD) { pushedStream[kHeadRequest] = true; pushedStream.end(); - } else if (options?.endStream) { + } else if (options.endStream) { pushedStream.end(); } } diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index 5d87a5a1fd40..55a1ae1d05e2 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -5164,7 +5164,7 @@ describe("http2 response options", () => { } }); - it("a response throws ERR_INVALID_ARG_TYPE for options that are not an object", async () => { + it("respond(), respondWithFile(), respondWithFD() and pushStream() throw for options that are not an object", async () => { const notObjects = { "null": null, "5": 5, @@ -5186,6 +5186,7 @@ describe("http2 response options", () => { ["respond", () => stream.respond({ ":status": 200 }, options)], ["respondWithFile", () => stream.respondWithFile(import.meta.path, { ":status": 200 }, options)], ["respondWithFD", () => stream.respondWithFD(0, { ":status": 200 }, options)], + ["pushStream", () => stream.pushStream({ ":path": "/pushed" }, options, () => {})], ]) { try { call(); @@ -5202,7 +5203,7 @@ describe("http2 response options", () => { const expected = {}; for (const name of Object.keys(notObjects)) { - for (const method of ["respond", "respondWithFile", "respondWithFD"]) { + for (const method of ["respond", "respondWithFile", "respondWithFD", "pushStream"]) { expected[`${method}(headers, ${name})`] = `TypeError ERR_INVALID_ARG_TYPE: The "options" argument must be of type object. ${received[name]}`; } @@ -5210,6 +5211,44 @@ describe("http2 response options", () => { expect({ thrown, responses }).toEqual({ thrown: expected, responses: ["200 ok", "200 ok"] }); }); + it("pushStream() does not count an inherited endStream", async () => { + const { promise: pushed, resolve } = Promise.withResolvers(); + const server = http2.createServer(); + server.on("stream", stream => { + stream.on("error", err => resolve(`server stream error ${err.code}`)); + stream.pushStream({ ":path": "/pushed" }, Object.create({ endStream: true }), (err, push) => { + if (err) return resolve(`push error ${err.code}`); + push.on("error", err => resolve(`pushed stream error ${err.code}`)); + // pushStream() ends the pushed writable when it reads a truthy endStream. + if (push.writableEnded) return resolve("the pushed writable was already ended"); + push.respond({ ":status": 200 }); + push.end("pushed body"); + }); + stream.respond({ ":status": 200 }); + stream.end("ok"); + }); + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + const client = http2.connect(`http://127.0.0.1:${server.address().port}`); + try { + client.on("error", err => resolve(`client session error ${err.code}`)); + client.on("stream", push => { + const chunks = []; + push.setEncoding("utf8"); + push.on("error", err => resolve(`client push error ${err.code}`)); + push.on("data", chunk => chunks.push(chunk)); + push.on("end", () => resolve(chunks.join(""))); + }); + const req = client.request({ ":path": "/" }); + req.on("error", err => resolve(`client stream error ${err.code}`)); + req.resume(); + req.end(); + expect(await pushed).toBe("pushed body"); + } finally { + client.destroy(); + server.close(); + } + }); + it("a response HEADERS frame never carries a priority field", async () => { const flagNames = flags => Object.entries({ END_STREAM: 0x1, END_HEADERS: 0x4, PADDED: 0x8, PRIORITY: 0x20 })