diff --git a/src/js/node/http2.ts b/src/js/node/http2.ts index ff7409618401..542d4cb2651f 100644 --- a/src/js/node/http2.ts +++ b/src/js/node/http2.ts @@ -2606,12 +2606,6 @@ class Http2Stream extends Duplex { // node keeps the never-index list visible on sentTrailers (symbol keys are not iterated by // the wire-encoding path, so re-attaching is safe). if (sensitives !== undefined) headers[sensitiveHeaders] = sensitives; - // RFC 9113 §8.1 doesn't explicitly forbid an empty trailer HEADERS frame, - // but strict peer implementations (nghttp2, used by curl and Node) reject - // a zero-length HPACK block as a callback failure. When the user passes an - // empty trailer object (which the compat Http2ServerResponse does - // unconditionally from onStreamTrailersReady), emit an empty DATA frame - // with END_STREAM instead — this matches Node's wire output. // Mark before the native call so a re-entrant sendTrailers() from a header-value // coercion hits ERR_HTTP2_TRAILERS_ALREADY_SENT, but clear it if validation throws // (no frame is written then) so a corrected retry succeeds like node. diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index 1be650bff0d6..1cce6e8f3e5c 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -7718,11 +7718,14 @@ impl H2FrameParser { return Err(global_object.throw(format_args!("Invalid stream id"))); }; // SAFETY: stream is a *mut Stream from self.streams (heap::alloc); valid while the map entry exists - let stream = unsafe { &mut *stream }; + this.send_no_trailers(unsafe { &mut *stream }); + Ok(JSValue::UNDEFINED) + } + /// Empty END_STREAM DATA frame: what node sends in place of a trailer block with no fields. + fn send_no_trailers(&self, stream: &mut Stream) { stream.wait_for_trailers = false; - let _ = this.send_data(stream, b"", true, JSValue::UNDEFINED, false, false); - Ok(JSValue::UNDEFINED) + let _ = self.send_data(stream, b"", true, JSValue::UNDEFINED, false, false); } /// node's strictSingleValueFields option (default true): when disabled, duplicate @@ -8094,6 +8097,10 @@ impl H2FrameParser { } } } + if encoded_headers.is_empty() { + this.send_no_trailers(&mut stream); + return Ok(JSValue::UNDEFINED); + } let encoded_data = encoded_headers.as_slice(); let encoded_size = encoded_data.len(); diff --git a/test/js/node/http2/h2-conformance.test.ts b/test/js/node/http2/h2-conformance.test.ts index 5698cdf5ea6b..bdae64b917e0 100644 --- a/test/js/node/http2/h2-conformance.test.ts +++ b/test/js/node/http2/h2-conformance.test.ts @@ -1008,6 +1008,111 @@ describe("request header and body framing (RFC 9113 §8.1)", () => { }); }); +describe.concurrent("sendTrailers() with a block that encodes to no fields (RFC 9113 §8.1)", () => { + // An empty array value puts no field on the wire (nor does an empty name). When that leaves + // nothing at all, node (Http2Stream::SubmitTrailers) ends the stream with the empty END_STREAM + // DATA frame that sendTrailers({}) also produces, instead of a HEADERS frame carrying a + // zero-length block, which the peer reports as a 'trailers' event with no headers in it. + const END_STREAM = 0x1; + const END_HEADERS = 0x4; + + /** The flags of every HEADERS frame on stream 1, and the frame that ended the stream. */ + function stream1(frames: Frame[]) { + const onStream = frames.filter(f => f.streamId === 1); + return { + headersFlags: onStream.filter(f => f.type === FrameType.HEADERS).map(f => f.flags), + endStream: onStream + .filter(f => (f.flags & END_STREAM) !== 0) + .map(f => ({ type: f.type, flags: f.flags, length: f.length })), + }; + } + + /** Serves one request with waitForTrailers, answering 'wantTrailers' with `trailers`. Resolves + * once the response stream has ended on the wire, with the server stream's sentTrailers. */ + async function serverSends(trailers: Record) { + const sent = Promise.withResolvers(); + const server = http2.createServer(); + server.on("stream", (stream: any) => { + stream.on("error", (err: Error) => sent.reject(err)); + stream.respond({ ":status": 200 }, { waitForTrailers: true }); + stream.on("wantTrailers", () => { + stream.sendTrailers(trailers); + sent.resolve(stream.sentTrailers); + }); + stream.end("body"); + }); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const c = await RawH2.connect((server.address() as net.AddressInfo).port); + try { + c.sendPreface(); + c.sendEmptySettings(); + c.sendFrame(FrameType.HEADERS, END_STREAM | END_HEADERS, 1, requestHeaderBlock("GET")); + await c.waitFor(f => f.streamId === 1 && (f.flags & END_STREAM) !== 0); + return { ...stream1(c.frames), sentTrailers: await sent.promise }; + } finally { + c.destroy(); + server.close(); + } + } + + test.each([ + [{ "x-none": [] }], + // content-type is a single-value field: its bookkeeping runs for the empty array but must + // not count as a field either. + [{ "x-none": [], "content-type": [] }], + [{ "": "x" }], + // {} is short-circuited to noTrailers() in JS before the encoder runs; it pins that the two + // routes put the same frame on the wire. + [{}], + ])("server: %j ends the response with an empty DATA frame and no trailer HEADERS frame", async trailers => { + expect(await serverSends(trailers)).toEqual({ + headersFlags: [END_HEADERS], // the response block; it cannot carry END_STREAM with trailers pending + endStream: [{ type: FrameType.DATA, flags: END_STREAM, length: 0 }], + sentTrailers: trailers, + }); + }); + + test("server: one field next to an empty array still goes out as a trailer HEADERS frame", async () => { + const result = await serverSends({ "x-none": [], "x-sent": "1" }); + expect(result).toEqual({ + headersFlags: [END_HEADERS, END_STREAM | END_HEADERS], + endStream: [{ type: FrameType.HEADERS, flags: END_STREAM | END_HEADERS, length: expect.any(Number) }], + sentTrailers: { "x-none": [], "x-sent": "1" }, + }); + expect(result.endStream[0].length).toBeGreaterThan(0); + }); + + test("client: the same block ends the request with an empty DATA frame and no trailer HEADERS frame", 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({ ":method": "POST", ":path": "/" }, { waitForTrailers: true }); + req.on("error", () => {}); + const sent = new Promise(resolve => { + req.on("wantTrailers", () => { + req.sendTrailers({ "x-none": [] }); + resolve(req.sentTrailers); + }); + }); + req.end(); + await raw.waitFor(f => f.type === FrameType.HEADERS && f.streamId === 1); + raw.sendFrame(FrameType.SETTINGS, 0, 0); // server SETTINGS + raw.sendFrame(FrameType.SETTINGS, 0x1, 0); // ACK the client's + await raw.waitFor(f => f.streamId === 1 && (f.flags & END_STREAM) !== 0); + expect({ ...stream1(raw.frames), sentTrailers: await sent }).toEqual({ + headersFlags: [END_HEADERS], + endStream: [{ type: FrameType.DATA, flags: END_STREAM, length: 0 }], + sentTrailers: { "x-none": [] }, + }); + } finally { + client.destroy(); + raw.close(); + } + }); +}); + describe("request pseudo-header requirements (RFC 9113 §8.3.1)", () => { // HPACK "literal never indexed, new name" (0x10) so the wire shape is exactly what is written // and no client library normalizes it away. diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index 26f4f42a1597..a7b61aceb5f5 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -4720,6 +4720,48 @@ it("end(chunk) on a HALF_CLOSED_REMOTE stream still emits 'finish'", async () => } }); +it("sendTrailers() whose values are all empty arrays ends the stream without a 'trailers' event", async () => { + // Empty arrays put no fields on the wire, so the block encodes to nothing. Node then ends the + // stream with an empty DATA frame (as for sendTrailers({})) and the peer never sees trailers; + // bun used to send a HEADERS frame with an empty block, surfacing as a 'trailers' event with + // no headers. sentTrailers still records what was passed, as in node. + const trailers = { "x-none": [] }; + const server = http2.createServer(); + try { + const serverSide = Promise.withResolvers(); + server.on("stream", stream => { + stream.on("error", serverSide.reject); + stream.respond({ ":status": 200 }, { waitForTrailers: true }); + stream.on("wantTrailers", () => stream.sendTrailers(trailers)); + stream.on("close", () => serverSide.resolve({ sentTrailers: stream.sentTrailers, rstCode: stream.rstCode })); + stream.end("body"); + }); + const port = await new Promise(resolve => server.listen(0, () => resolve(server.address().port))); + + const clientSide = Promise.withResolvers(); + const client = http2.connect(`http://localhost:${port}`); + client.on("error", clientSide.reject); + try { + const req = client.request({ ":path": "/" }); + req.on("error", clientSide.reject); + const trailerEvents = []; + req.on("trailers", (headers, flags, rawHeaders) => trailerEvents.push(rawHeaders)); + let body = ""; + req.setEncoding("utf8"); + req.on("data", chunk => (body += chunk)); + req.on("close", () => clientSide.resolve({ body, trailerEvents, rstCode: req.rstCode })); + req.end(); + + expect(await clientSide.promise).toEqual({ body: "body", trailerEvents: [], rstCode: 0 }); + expect(await serverSide.promise).toEqual({ sentTrailers: trailers, rstCode: 0 }); + } finally { + client.close(); + } + } finally { + server.close(); + } +}); + it("client connects over a user Duplex that already has a 'data' listener", async () => { // A 'data' listener attached before connect() puts the stream in flowing mode, so the // peer's first frames can arrive before the connect callback has run. The preface must