Skip to content

node:http2: reject request header blocks that fail RFC 9113 8.3.1 (empty/missing :path, :method, :scheme; CONNECT shape) #34736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cirospaciari merged 2 commits into from
Jul 20, 2026
Merged

node:http2: reject request header blocks that fail RFC 9113 8.3.1 (empty/missing :path, :method, :scheme; CONNECT shape) #34736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d53d0bb
node:http2: reject request header blocks that fail RFC 9113 8.3.1 (em…
robobun Jul 19, 2026
26eb1c8
[autofix.ci] apply automated fixes
autofix-ci[bot] Jul 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
89 changes: 76 additions & 13 deletions src/runtime/api/bun/h2/connection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,18 @@ use super::stream::{self, State};
use super::wire::{self, ErrorCode, FrameHeader, FrameType, SettingId};
use bun_collections::HashMap;

/// Pseudo-header presence bits shared by the per-field decode loop and the RFC 9113 §8.3.1
/// request checks in `finish_header_block` (nghttp2's NGHTTP2_HTTP_FLAG__* equivalents).
mod pseudo {
pub(super) const METHOD: u8 = 1;
pub(super) const SCHEME: u8 = 2;
pub(super) const AUTHORITY: u8 = 4;
pub(super) const PATH: u8 = 8;
pub(super) const STATUS: u8 = 16;
pub(super) const PROTOCOL: u8 = 32;
pub(super) const UNKNOWN: u8 = 64;
}

/// Snapshot of the local-settings values carried by one sent-but-unACKed SETTINGS frame, so an
/// inbound ACK is attributed to the submission it actually acknowledges (RFC 9113 §6.5.3) rather
/// than to the latest submission.
Expand Down Expand Up @@ -235,6 +247,11 @@ pub struct Connection {
header_target: u32,
/// 0 for a normal HEADERS block; the parent stream id when assembling a PUSH_PROMISE block.
header_push_parent: u32,
/// The assembling block opens a new inbound stream (a request block on the server, a
/// PUSH_PROMISE request block on the client). False for a later HEADERS on the same stream
/// (a trailer section), which RFC 9113 §8.1 forbids from carrying pseudo-headers and which
/// must not be held to the request pseudo-header requirements of §8.3.1.
header_is_request: bool,
/// HEADERS arrived on a closed/half-closed-remote stream: the block is still decoded so the
/// connection-scoped HPACK table stays in sync (§4.3), then refused with RST_STREAM
/// (STREAM_CLOSED) instead of being dispatched.
Expand Down Expand Up @@ -286,6 +303,7 @@ impl Connection {
header_flags: 0,
header_target: 0,
header_push_parent: 0,
header_is_request: false,
header_stream_closed: false,
header_stream_refused: false,
terminated: false,
Expand Down Expand Up @@ -986,6 +1004,10 @@ impl Connection {
self.header_flags = hdr.flags;
self.header_target = hdr.stream_id;
self.header_push_parent = 0;
// Only a server receives request blocks via HEADERS; on a client every response block
// looks "new" (the engine only tracks inbound-created streams), so without this gate it
// would be misclassified as a request. PUSH_PROMISE sets the flag itself.
self.header_is_request = self.is_server && is_new;
self.header_stream_closed = stream_closed;
self.header_stream_refused = refused;
if !end_headers {
Expand Down Expand Up @@ -1056,9 +1078,14 @@ impl Connection {
let mut malformed = is_trailer && !self.header_end_stream;
let mut seen_regular = false;
let mut seen_pseudo: u8 = 0;
// Request-block state for the RFC 9113 §8.3.1 checks below (nghttp2's
// nghttp2_http_on_request_headers): only an initial HEADERS block (or a PUSH_PROMISE
// block) is a request; a later HEADERS on the same stream is a trailer section.
let is_request = self.header_is_request;
let mut saw_connect = false;
let mut saw_host = false;
let mut informational = false;
let mut content_length: Option<u64> = None;
let mut connect = false;
while off < block.len() {
match self.hpack.decode(&block[off..]) {
Ok(h) => {
Expand Down Expand Up @@ -1090,13 +1117,13 @@ impl Connection {
malformed = true;
} else if let Some(rest) = name_b.strip_prefix(b":") {
let bit: u8 = match rest {
b"method" => 1,
b"scheme" => 2,
b"authority" => 4,
b"path" => 8,
b"status" => 16,
b"protocol" => 32,
_ => 64,
b"method" => pseudo::METHOD,
b"scheme" => pseudo::SCHEME,
b"authority" => pseudo::AUTHORITY,
b"path" => pseudo::PATH,
b"status" => pseudo::STATUS,
b"protocol" => pseudo::PROTOCOL,
_ => pseudo::UNKNOWN,
};
// 8.3.1: requests never carry :status - a server seeing it inbound is
// a malformed block. (The client direction also constrains pseudo
Expand All @@ -1112,12 +1139,16 @@ impl Connection {
let protocol_disabled = self.is_server
&& rest == b"protocol"
&& self.local_settings.enable_connect_protocol == 0;
// nghttp2 (check_pseudo_header) treats an empty pseudo-header value as
// malformed, so `:path: ""` never counts as a present :path (§8.3.1:
// `:path` "MUST NOT be empty" for http/https).
if seen_regular
|| bit == 64
|| bit == pseudo::UNKNOWN
|| (seen_pseudo & bit) != 0
|| wrong_direction
|| protocol_disabled
|| is_trailer
|| value_b.is_empty()
{
malformed = true;
}
Expand All @@ -1126,13 +1157,14 @@ impl Connection {
}
seen_pseudo |= bit;
if rest == b"method" && value_b == b"CONNECT" {
connect = true;
saw_connect = true;
}
} else {
seen_regular = true;
match name_b {
b"connection" | b"keep-alive" | b"proxy-connection"
| b"transfer-encoding" | b"upgrade" => malformed = true,
b"host" if is_request => saw_host = true,
b"te" => {
// RFC 9110 10.1.4: field values are case-insensitive.
if !value_b.eq_ignore_ascii_case(b"trailers") {
Expand Down Expand Up @@ -1186,9 +1218,25 @@ impl Connection {
sink.on_stream_reset(target, ErrorCode::StreamClosed.as_u32());
return false;
}
// RFC 9113 §8.3.1 (nghttp2_http_on_request_headers): a request block needs exactly one
// non-empty :method, :scheme and :path plus an :authority or Host; plain CONNECT omits
// :scheme/:path and carries :authority; extended CONNECT (:protocol, RFC 8441) requires
// :method CONNECT and :authority. Without this a block with an empty or missing :path
// reaches JS as a request with an empty url (no compliant peer can produce that shape).
if is_request && !rejected && !malformed {
use pseudo::{AUTHORITY, METHOD, PATH, PROTOCOL, SCHEME};
let extended_connect = (seen_pseudo & PROTOCOL) != 0;
malformed = if saw_connect && !extended_connect {
(seen_pseudo & (SCHEME | PATH)) != 0 || (seen_pseudo & AUTHORITY) == 0
} else {
(seen_pseudo & (METHOD | SCHEME | PATH)) != (METHOD | SCHEME | PATH)
|| ((seen_pseudo & AUTHORITY) == 0 && !saw_host)
|| (extended_connect && (!saw_connect || (seen_pseudo & AUTHORITY) == 0))
};
}
if push_parent == 0 && self.is_server && !malformed && !rejected {
if let Some(s) = self.streams.get_mut(&target) {
if !connect && s.content_length.is_none() {
if !saw_connect && s.content_length.is_none() {
s.content_length = content_length;
}
if self.header_end_stream
Expand Down Expand Up @@ -1654,6 +1702,7 @@ impl Connection {
self.header_flags = 0;
self.header_target = promised;
self.header_push_parent = hdr.stream_id;
self.header_is_request = true;
self.header_stream_closed = false;
self.header_stream_refused = false;
if !end_headers {
Expand Down Expand Up @@ -2072,7 +2121,12 @@ mod tests {
let sink = CaptureSink::default();
let mut c = Connection::new(true, Settings::default());
c.preface_received = wire::CONNECTION_PREFACE.len();
let block = encode_block(&[(b":method", b"GET"), (b":path", b"/")]);
let block = encode_block(&[
(b":method", b"GET"),
(b":scheme", b"http"),
(b":path", b"/"),
(b":authority", b"localhost"),
]);
let flags = wire::flags::END_HEADERS | wire::flags::END_STREAM;
let f = frame(FrameType::Headers, flags, 1, &block);
let fed = c.receive(&sink, &f);
Expand Down Expand Up @@ -2104,7 +2158,12 @@ mod tests {
let sink = CaptureSink::default();
let mut c = Connection::new(true, Settings::default());
c.preface_received = wire::CONNECTION_PREFACE.len();
let block = encode_block(&[(b":method", b"POST"), (b":path", b"/")]);
let block = encode_block(&[
(b":method", b"POST"),
(b":scheme", b"http"),
(b":path", b"/"),
(b":authority", b"localhost"),
]);
// HEADERS without END_STREAM -> stream stays open for DATA.
let h = frame(FrameType::Headers, wire::flags::END_HEADERS, 1, &block);
c.receive(&sink, &h);
Expand Down Expand Up @@ -2144,7 +2203,9 @@ mod tests {
let mut client = Connection::new(false, Settings::default());
client.begin_header_block();
assert!(client.encode_header(b":method", b"GET", false));
assert!(client.encode_header(b":scheme", b"http", false));
assert!(client.encode_header(b":path", b"/x", false));
assert!(client.encode_header(b":authority", b"localhost", false));
client.send_header_block(&csink, 1, true);
let wire_bytes = csink.out.borrow().clone();
assert_eq!(
Expand Down Expand Up @@ -2183,7 +2244,9 @@ mod tests {
let mut server = Connection::new(true, Settings::default());
server.begin_header_block();
assert!(server.encode_header(b":method", b"GET", false));
assert!(server.encode_header(b":scheme", b"http", false));
assert!(server.encode_header(b":path", b"/pushed", false));
assert!(server.encode_header(b":authority", b"localhost", false));
server.send_push_promise(&ssink, 1, 2);
let bytes = ssink.out.borrow().clone();
assert_eq!(
Expand Down
Loading
Loading