Skip to content
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
bbd8528
node:http2: reset the stream when respond() exceeds maxSendHeaderBloc…
robobun Sep 19, 2026
63cb733
node:http2: share the oversized header block path with send_trailers
robobun Sep 19, 2026
2d121eb
node:http2: put the last peer stream id in the GOAWAY and send it once
robobun Sep 19, 2026
893286b
node:http2: shorten the comments on the oversized header block path
robobun Sep 19, 2026
94876e7
node:http2: one-line doc comment on reject_oversized_header_block
robobun Sep 19, 2026
4174f9d
node:http2: flush corked frames before destroy() stops accepting writes
robobun Sep 19, 2026
b66f73c
node:http2: shorten the flush comments
robobun Sep 19, 2026
8d04717
node:http2: send the destroy() GOAWAY before the session stops accept…
robobun Sep 19, 2026
c4b7ceb
node:http2: shorten the destroy() GOAWAY comment
robobun Sep 19, 2026
c5731e0
node:http2: destroy() writes only the corked control frames
robobun Sep 19, 2026
d641fed
node:http2: one-line doc comment on flushCorked
robobun Sep 19, 2026
576e877
node:http2: refuse an oversized header block before the HPACK encode …
robobun Sep 19, 2026
0f425e4
node:http2: a reset stream sends no trailers and no END_STREAM
robobun Sep 19, 2026
f63b048
node:http2: keep the client branch of the send limit check as it was
robobun Sep 19, 2026
1d6085f
node:http2: one-line comments on the send limit path
robobun Sep 19, 2026
b022988
node:http2: free a refused client stream and reset an unresponded ser…
robobun Sep 19, 2026
629e14f
node:http2: a refused client request never writes RST_STREAM
robobun Sep 19, 2026
b88692d
node:http2: pin the maxSendHeaderBlockLength threshold in a test
robobun Sep 19, 2026
c2fc937
ci: retrigger
robobun Sep 19, 2026
3bd72aa
node:http2: test that a refused client request is freed safely
robobun Sep 19, 2026
4566950
node:http2: close(code) on a refused client request sends no RST_STREAM
robobun Sep 19, 2026
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
12 changes: 12 additions & 0 deletions src/js/node/http2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -5015,6 +5016,8 @@ 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: 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);
},
Expand Down Expand Up @@ -6564,6 +6567,15 @@ function onErrorSecureServerSession(err, socket) {
function emitFrameErrorEventNT(stream, frameType, errorCode) {
stream.emit("frameError", 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);
Comment thread
robobun marked this conversation as resolved.
Resolved
session.close();
}
class Http2SecureServer extends tls.Server {
timeout = 0;
[kSessions] = new SafeSet();
Expand Down
156 changes: 139 additions & 17 deletions src/runtime/api/bun/h2_frame_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1962,11 +1962,88 @@ impl AbortListener for SignalRef {
}
}

/// Header fields staged so a block can be measured before any of them reaches the HPACK encoder.
#[derive(Default)]
struct HeaderList {
bytes: Vec<u8>,
fields: Vec<HeaderField>,
}

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(())
}

/// 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()
}

/// 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<Item = (&[u8], &[u8], bool)> {
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<HeaderList>,
encoded_headers: &mut Vec<u8>,
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.
///
Expand Down Expand Up @@ -5386,6 +5463,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(
Expand Down Expand Up @@ -5554,6 +5635,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);
}
Comment thread
robobun marked this conversation as resolved.
Resolved

let Some(headers_obj) = headers_arg.get_object() else {
return Err(global_object.throw(format_args!("Expected headers to be an object")));
Expand Down Expand Up @@ -5671,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))
Expand Down Expand Up @@ -6484,6 +6569,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 =
Expand Down Expand Up @@ -6604,7 +6691,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,
Expand Down Expand Up @@ -6774,7 +6862,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,
Expand Down Expand Up @@ -6844,7 +6933,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,
Expand All @@ -6870,7 +6960,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));
Expand Down Expand Up @@ -7071,36 +7160,70 @@ 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);

// 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
// 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
{
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),
);
// 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);
}
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();
stream.free_resources::<false>(this);
Comment thread
robobun marked this conversation as resolved.
Resolved
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));
}

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)
{
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
.get()
Expand Down Expand Up @@ -7290,7 +7413,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))
}

Expand Down
71 changes: 71 additions & 0 deletions test/js/node/http2/h2-conformance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, (server: http2.Http2Server) => 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();
}
});
});
Loading
Loading