Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/js/node/http2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
13 changes: 10 additions & 3 deletions src/runtime/api/bun/h2_frame_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();

Expand Down
105 changes: 105 additions & 0 deletions test/js/node/http2/h2-conformance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>) {
const sent = Promise.withResolvers<unknown>();
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<unknown>(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.
Expand Down
42 changes: 42 additions & 0 deletions test/js/node/http2/node-http2.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading