From aac5ef8c250a7e49e97c187669443779faf78635 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 18 Sep 2026 01:26:43 +0000 Subject: [PATCH 1/5] node:http2: apply SETTINGS_HEADER_TABLE_SIZE to the HPACK encoder and decoder Both HPACK tables took the local headerTableSize when the session was created and never changed. The outbound encoder now starts at the 4096 default, follows every SETTINGS_HEADER_TABLE_SIZE entry the peer sends (capped at 4096, node's default maxDeflateDynamicTableSize), and opens the next header block with the RFC 7541 6.3 size update: the smallest size since the last block first, then the final size. The update stays pending until a block that carries it is written. The decoder now starts at the 4096 default and takes the local value when the peer ACKs the SETTINGS frame that carried it. --- src/http/lshpack.rs | 8 + src/jsc/bindings/c-bindings.cpp | 5 + src/runtime/api/bun/h2/connection.rs | 28 +- src/runtime/api/bun/h2/hpack.rs | 132 ++++--- src/runtime/api/bun/h2_frame_parser.rs | 50 ++- test/js/node/http2/h2-conformance.test.ts | 442 +++++++++++++++++++++- 6 files changed, 595 insertions(+), 70 deletions(-) diff --git a/src/http/lshpack.rs b/src/http/lshpack.rs index d6af265cef42..da9f1cbc6f2e 100644 --- a/src/http/lshpack.rs +++ b/src/http/lshpack.rs @@ -126,6 +126,13 @@ impl HPACK { lshpack_wrapper_enc_set_max_capacity(self, max_capacity as c_uint); } + /// Adjust the decoder's dynamic-table capacity after init: the limit on the + /// peer's RFC 7541 §6.3 Dynamic Table Size Updates, and the table's size + /// until the peer sends one. Evicts entries to fit. + pub fn set_decoder_max_capacity(&mut self, max_capacity: u32) { + lshpack_wrapper_dec_set_max_capacity(self, max_capacity as c_uint); + } + // Raw `*mut HPACK` teardown is subsumed by the // safe [`HpackHandle`] RAII wrapper below — every owner holds an // `HpackHandle`, so the raw destructor is private to `HpackHandle::drop`. @@ -208,6 +215,7 @@ unsafe extern "C" { // Only precondition is a valid non-null `*HPACK`; `&mut HPACK` (ABI-identical // thin pointer) discharges it at the type level, so this is `safe fn`. safe fn lshpack_wrapper_enc_set_max_capacity(self_: &mut HPACK, max_capacity: c_uint); + safe fn lshpack_wrapper_dec_set_max_capacity(self_: &mut HPACK, max_capacity: c_uint); // Frees `self_` (lshpack_{enc,dec}_cleanup + mi_free) — ownership transfer, // so this keeps its raw-pointer signature and caller-side safety obligation. fn lshpack_wrapper_deinit(self_: *mut HPACK); diff --git a/src/jsc/bindings/c-bindings.cpp b/src/jsc/bindings/c-bindings.cpp index f9a6ec70194c..76f9ebb3136a 100644 --- a/src/jsc/bindings/c-bindings.cpp +++ b/src/jsc/bindings/c-bindings.cpp @@ -844,6 +844,11 @@ void lshpack_wrapper_enc_set_max_capacity(lshpack_wrapper* self, unsigned max_ca lshpack_enc_set_max_capacity(&self->enc, max_capacity); } +void lshpack_wrapper_dec_set_max_capacity(lshpack_wrapper* self, unsigned max_capacity) +{ + lshpack_dec_set_max_capacity(&self->dec, max_capacity); +} + void lshpack_wrapper_deinit(lshpack_wrapper* self) { lshpack_dec_cleanup(&self->dec); diff --git a/src/runtime/api/bun/h2/connection.rs b/src/runtime/api/bun/h2/connection.rs index a6ad19360952..66030e6cf3cf 100644 --- a/src/runtime/api/bun/h2/connection.rs +++ b/src/runtime/api/bun/h2/connection.rs @@ -254,6 +254,10 @@ pub(crate) trait Sink { } /// A SETTINGS entry with an id outside the standard registry (node's remoteCustomSettings). fn on_remote_custom_setting(&self, _id: u16, _value: u32) {} + /// Transition shim while the outbound path still flows through the embedder's legacy encoder: + /// one SETTINGS_HEADER_TABLE_SIZE entry from the peer, for every entry in wire order, so that + /// encoder can follow it (`hpack::Coder::set_peer_header_table_size`). + fn on_remote_header_table_size(&self, _size: u32) {} /// One decoded header field. `name`/`value` alias a shared buffer — copy before returning. fn on_header(&self, _stream_id: u32, _name: &[u8], _value: &[u8], _never_index: bool) {} /// The header block for `stream_id` is complete. `end_stream` = the HEADERS carried END_STREAM. @@ -408,7 +412,7 @@ impl Connection { max_settings: 32, send_window: SendWindow::new(wire::DEFAULT_WINDOW_SIZE), recv_window: RecvWindow::new(wire::DEFAULT_WINDOW_SIZE), - hpack: hpack::Coder::new(local.header_table_size), + hpack: hpack::Coder::new(), streams: HashMap::new(), header_block_in_flight: None, header_block: Vec::new(), @@ -793,6 +797,8 @@ impl Connection { // The peer has acknowledged this submission: header-list enforcement may now use the // limit it carried. self.enforced_max_header_list_size = acked.settings.max_header_list_size; + self.hpack + .set_acked_header_table_size(acked.settings.header_table_size); sink.on_local_settings(&acked.settings); return false; } @@ -811,7 +817,6 @@ impl Connection { self.send_go_away(sink, code, b"SETTINGS value out of range"); return true; } - let old_table = self.remote_settings.header_table_size; let old_initial_window = self.remote_settings.initial_window_size; let mut i = 0; while i + 6 <= payload.len() { @@ -837,16 +842,17 @@ impl Connection { return true; } self.remote_settings.apply(sid, value); + // The peer's HEADER_TABLE_SIZE governs OUR encoder. Every entry counts, not only + // the last one: the peer's decoder evicts at each (RFC 7541 §4.2). + if sid == SettingId::HeaderTableSize { + self.hpack.set_peer_header_table_size(value); + sink.on_remote_header_table_size(value); + } } else { sink.on_remote_custom_setting(id, value); } i += 6; } - // The peer's HEADER_TABLE_SIZE governs OUR encoder; queue a 6.3 size update. - if self.remote_settings.header_table_size != old_table { - self.hpack - .queue_encoder_capacity(self.remote_settings.header_table_size); - } // 6.9.2: a change to SETTINGS_INITIAL_WINDOW_SIZE adjusts every non-closed stream's send // window by the delta (the connection window is not affected). if self.remote_settings.initial_window_size != old_initial_window { @@ -2010,9 +2016,7 @@ impl Connection { /// Begin a new outbound header block. Emits any pending §6.3 dynamic-table size update first. pub(crate) fn begin_header_block(&mut self) { self.enc_buf.clear(); - let mut tmp = [0u8; hpack::MAX_SIZE_UPDATE_BYTES]; - let n = self.hpack.take_pending_size_update(&mut tmp, 0); - self.enc_buf.extend_from_slice(&tmp[..n]); + self.hpack.write_pending_size_update(&mut self.enc_buf); } /// HPACK-encode one header field into the current block. Returns false on encode failure. @@ -2073,6 +2077,7 @@ impl Connection { ); off += len; } + self.hpack.size_update_sent(); self.enc_buf = block; self.enc_buf.clear(); @@ -2199,6 +2204,7 @@ impl Connection { ); o += len; } + self.hpack.size_update_sent(); self.enc_buf = block; self.enc_buf.clear(); @@ -2295,7 +2301,7 @@ mod tests { /// Encode a header block with a standalone coder (mirrors a real peer's encoder). fn encode_block(pairs: &[(&[u8], &[u8])]) -> Vec { - let mut coder = hpack::Coder::new(4096); + let mut coder = hpack::Coder::new(); let mut buf = vec![0u8; 4096]; let mut off = 0usize; for (name, value) in pairs { diff --git a/src/runtime/api/bun/h2/hpack.rs b/src/runtime/api/bun/h2/hpack.rs index ac84c8048116..8edc6bb80404 100644 --- a/src/runtime/api/bun/h2/hpack.rs +++ b/src/runtime/api/bun/h2/hpack.rs @@ -1,53 +1,96 @@ //! HPACK coder (RFC 7541) over the lshpack binding — the only reused piece in the rewrite. //! -//! Centralizes the dynamic-table-size-update handling: when the peer changes -//! SETTINGS_HEADER_TABLE_SIZE, the encoder capacity is lowered AND a §6.3 Dynamic Table Size -//! Update opcode is emitted at the start of the next header block so the peer's decoder evicts in -//! lockstep. Decode results alias a shared buffer and MUST be copied before the next call -//! (see lshpack.rs). +//! Centralizes the dynamic-table-size handling. Each table belongs to the side that decodes with +//! it: the encoder's table follows the PEER's SETTINGS_HEADER_TABLE_SIZE, and every change is +//! announced with a §6.3 Dynamic Table Size Update at the start of the next header block so the +//! peer's decoder evicts in lockstep. The decoder's table follows OUR setting, from the moment +//! the peer acknowledges it. Decode results alias a shared buffer and MUST be copied before the +//! next call (see lshpack.rs). #![allow(dead_code)] use bun_http::lshpack::{DecodeResult, HpackError, HpackHandle}; -/// RFC 7541 §6.3: a Dynamic Table Size Update integer never needs more than 6 bytes for a u32. -pub(crate) const MAX_SIZE_UPDATE_BYTES: usize = 6; +/// RFC 9113 §6.5.2: SETTINGS_HEADER_TABLE_SIZE in both directions until a SETTINGS frame says +/// otherwise. +pub(crate) const DEFAULT_HEADER_TABLE_SIZE: u32 = 4096; + +/// Upper bound of the encoder's dynamic table, whatever the peer allows: a peer that advertises a +/// huge table must not make this side retain that much header history. Node's default +/// `maxDeflateDynamicTableSize`. +pub(crate) const MAX_ENCODER_TABLE_SIZE: u32 = 4096; pub(crate) struct Coder { hpack: HpackHandle, + /// Capacity of the encoder's dynamic table. enc_capacity: u32, - /// A capacity change requested by the peer's SETTINGS_HEADER_TABLE_SIZE, applied + announced at - /// the start of the next encoded header block. `None` = nothing pending. - pending_enc_capacity: Option, + /// `Some` while the peer's decoder has not been told about a capacity change: the smallest + /// capacity the encoder had since the last header block that reached the transport. The + /// peer's decoder evicts at each of its SETTINGS, so RFC 7541 §4.2 wants that minimum + /// signaled before the final value. + unannounced_min: Option, + /// Limit of the decoder's dynamic table: our SETTINGS_HEADER_TABLE_SIZE that the peer + /// acknowledged last. + dec_capacity: u32, } impl Coder { - pub(crate) fn new(max_capacity: u32) -> Self { + /// Both tables start at the protocol default. The encoder moves with + /// [`Self::set_peer_header_table_size`], the decoder with + /// [`Self::set_acked_header_table_size`]. + pub(crate) fn new() -> Self { Coder { - hpack: HpackHandle::new(max_capacity), - enc_capacity: max_capacity, - pending_enc_capacity: None, + hpack: HpackHandle::new(DEFAULT_HEADER_TABLE_SIZE), + enc_capacity: DEFAULT_HEADER_TABLE_SIZE, + unannounced_min: None, + dec_capacity: DEFAULT_HEADER_TABLE_SIZE, + } + } + + /// The peer acknowledged a SETTINGS frame of ours that carried this + /// SETTINGS_HEADER_TABLE_SIZE. Until the ACK the peer's encoder still works against the + /// previous value, so the decoder must not move earlier. From the ACK on, the value bounds + /// the peer's §6.3 size updates. + pub(crate) fn set_acked_header_table_size(&mut self, size: u32) { + if size == self.dec_capacity { + return; } + self.hpack.set_decoder_max_capacity(size); + self.dec_capacity = size; } - /// Schedule an encoder capacity change from a received SETTINGS_HEADER_TABLE_SIZE. Applied - /// lazily so the §6.3 size-update opcode is emitted inside the next header block. - pub(crate) fn queue_encoder_capacity(&mut self, capacity: u32) { - if capacity == self.enc_capacity && self.pending_enc_capacity.is_none() { + /// One SETTINGS_HEADER_TABLE_SIZE entry from the peer; call it for every entry, in wire + /// order. Evicts right away, like the peer's decoder did when it sent the value. + pub(crate) fn set_peer_header_table_size(&mut self, size: u32) { + let capacity = size.min(MAX_ENCODER_TABLE_SIZE); + if capacity == self.enc_capacity && self.unannounced_min.is_none() { return; } - self.pending_enc_capacity = Some(capacity); + self.hpack.set_encoder_max_capacity(capacity); + self.enc_capacity = capacity; + self.unannounced_min = Some(match self.unannounced_min { + Some(min) => min.min(capacity), + None => capacity, + }); } - /// If a capacity change is pending, apply it and write the §6.3 size-update opcode into `dst` at - /// `offset`. Returns bytes written (0 if none). `dst[offset..]` needs >= MAX_SIZE_UPDATE_BYTES. - pub(crate) fn take_pending_size_update(&mut self, dst: &mut [u8], offset: usize) -> usize { - let Some(cap) = self.pending_enc_capacity.take() else { - return 0; + /// Call at the start of every outbound header block, before its first field: appends the + /// §6.3 size update(s) the peer's decoder still has to see. They stay pending until + /// [`Self::size_update_sent`], so a block that is built but never sent does not lose them. + pub(crate) fn write_pending_size_update(&self, block: &mut Vec) { + let Some(min) = self.unannounced_min else { + return; }; - self.hpack.set_encoder_max_capacity(cap); - self.enc_capacity = cap; - write_table_size_update(dst, offset, cap) + if min < self.enc_capacity { + write_table_size_update(block, min); + } + write_table_size_update(block, self.enc_capacity); + } + + /// The header block opened with [`Self::write_pending_size_update`] was handed to the + /// transport. + pub(crate) fn size_update_sent(&mut self) { + self.unannounced_min = None; } #[inline] @@ -69,25 +112,19 @@ impl Coder { } } -/// RFC 7541 §5.1 + §6.3: encode `value` as a 5-bit-prefix integer with the `001` pattern (0x20) -/// into `dst[offset..]`. Returns bytes written. -fn write_table_size_update(dst: &mut [u8], offset: usize, value: u32) -> usize { - let mut i = offset; +/// RFC 7541 §5.1 + §6.3: append `value` as a 5-bit-prefix integer with the `001` pattern (0x20). +fn write_table_size_update(block: &mut Vec, value: u32) { if value < 31 { - dst[i] = 0x20 | value as u8; - return 1; + block.push(0x20 | value as u8); + return; } - dst[i] = 0x20 | 31; - i += 1; + block.push(0x20 | 31); let mut rest = value - 31; while rest >= 128 { - dst[i] = (rest as u8) | 0x80; - i += 1; + block.push((rest as u8) | 0x80); rest >>= 7; } - dst[i] = rest as u8; - i += 1; - i - offset + block.push(rest as u8); } #[cfg(test)] @@ -96,17 +133,16 @@ mod tests { #[test] fn size_update_small() { - let mut buf = [0u8; 6]; - assert_eq!(write_table_size_update(&mut buf, 0, 30), 1); - assert_eq!(buf[0], 0x20 | 30); + let mut block = Vec::new(); + write_table_size_update(&mut block, 30); + assert_eq!(block, [0x20 | 30]); } #[test] fn size_update_large() { - let mut buf = [0u8; 6]; - // 4096 = 31 + 4065; 4065 = 0b111_1110_0001 -> 0xE1, 0x1F - let n = write_table_size_update(&mut buf, 0, 4096); - assert_eq!(buf[0], 0x3f); // 0x20 | 31 - assert!(n >= 2); + let mut block = Vec::new(); + // 4096 = 31 + 4065; 4065 = 0b11111_1100001 -> 0xE1, 0x1F + write_table_size_update(&mut block, 4096); + assert_eq!(block, [0x3f, 0xe1, 0x1f]); } } diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index b9ba74915f95..f56d28a4e62c 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -18,7 +18,6 @@ use crate::webcore::AutoFlusher; use bstr::BStr; use bun_collections::{ByteVecExt, HashMap as BunHashMap, HiveArrayFallback, VecExt}; use bun_core::strings; -use bun_http::lshpack; use bun_jsc::AbortSignal; use bun_jsc::ErrorCode as JscErrorCode; use bun_jsc::StringJsc as _; @@ -1111,7 +1110,8 @@ pub(crate) struct H2FrameParser { streams: JsCell>, - hpack: JsCell>, + /// The outbound HPACK encoder. The inbound decoder is the engine's own `Coder`. + hpack: JsCell>, has_nonnative_backpressure: Cell, /// True while flush() has bytes out in an onWrite dispatch to a JS-backed socket. @@ -1997,6 +1997,27 @@ impl H2FrameParser { }) } + /// Opens an outbound header block: the peer's decoder learns about a changed dynamic table + /// size here and nowhere else (RFC 7541 §4.2). Pair with `header_block_sent`. + fn begin_header_block(&self, encoded_headers: &mut Vec) { + self.hpack.with_mut(|hpack| { + if let Some(hpack) = hpack.as_mut() { + hpack.write_pending_size_update(encoded_headers); + } + }); + } + + /// The block opened with `begin_header_block` was written. A block that is dropped before + /// this (a header that fails validation, an option that throws) keeps the size update pending + /// for the next one. + fn header_block_sent(&self) { + self.hpack.with_mut(|hpack| { + if let Some(hpack) = hpack.as_mut() { + hpack.size_update_sent(); + } + }); + } + /// Serialize the SETTINGS entries that go on the wire: only the standard parameters JS set /// explicitly plus any custom settings (node never serializes defaults — a session created /// with default options sends an empty SETTINGS frame). Returns the payload length. @@ -3865,6 +3886,14 @@ impl crate::api::h2::connection::Sink for H2FrameParser { }); } + fn on_remote_header_table_size(&self, size: u32) { + self.hpack.with_mut(|hpack| { + if let Some(hpack) = hpack.as_mut() { + hpack.set_peer_header_table_size(size); + } + }); + } + fn on_remote_settings(&self, settings: &crate::api::h2::settings::Settings) { // Bridge: the legacy outbound (frame sizing, window init) reads the remote_settings Cell. let fp = FullSettingsPayload { @@ -5560,6 +5589,7 @@ impl H2FrameParser { if encoded_headers.try_reserve(16384).is_err() { return Err(global_object.throw(format_args!("Failed to allocate header buffer"))); } + self.begin_header_block(&mut encoded_headers); // max header name length for lshpack let mut name_buffer = [0u8; 4096]; @@ -5824,6 +5854,7 @@ impl H2FrameParser { offset += chunk_size; } } + self.header_block_sent(); let identifier = stream.get_identifier(); identifier.ensure_still_alive(); if stream.state == StreamState::HALF_CLOSED_REMOTE { @@ -6022,6 +6053,7 @@ impl H2FrameParser { let mut name_buffer = [0u8; 4096]; let mut encoded_headers: Vec = Vec::new(); + self.begin_header_block(&mut encoded_headers); let mut single_value_headers = [false; SINGLE_VALUE_HEADERS_LEN]; // A PUSH_PROMISE carries a REQUEST, so request pseudo-headers are valid even on the server. @@ -6236,6 +6268,7 @@ impl H2FrameParser { offset += chunk; } } + self.header_block_sent(); let _ = self.flush(); Ok(JSValue::js_number(promised_id as f64)) @@ -6427,6 +6460,7 @@ impl H2FrameParser { if encoded_headers.try_reserve(16384).is_err() { return Err(global_object.throw(format_args!("Failed to allocate header buffer"))); } + self.begin_header_block(&mut encoded_headers); // max header name length for lshpack let mut name_buffer = [0u8; 4096]; let stream_id: u32 = if stream_id.is_number() { @@ -7188,6 +7222,7 @@ impl H2FrameParser { offset += chunk_size; } } + self.header_block_sent(); if end_stream { stream.end_after_headers = true; @@ -7590,13 +7625,10 @@ impl H2FrameParser { unsafe { bun_jsc::AbortHandle::arm_owner(this, context) }; } - // Note: `HPACK::init` returns a C-allocated wrapper that must be - // torn down via `lshpack_wrapper_deinit` (runs `lshpack_{enc,dec}_cleanup` - // before freeing). Wrapping it in `heap::take` and letting `Box` drop - // would `mi_free` the struct but leak the encoder/decoder internals. - this_ref.hpack.set(Some(lshpack::HpackHandle::new( - this_ref.local_settings.get().header_table_size, - ))); + // Our own headerTableSize sizes the engine's decoder. This encoder follows the peer's. + this_ref + .hpack + .set(Some(crate::api::h2::hpack::Coder::new())); if is_server { let _ = this_ref.set_settings(this_ref.local_settings.get()); } else { diff --git a/test/js/node/http2/h2-conformance.test.ts b/test/js/node/http2/h2-conformance.test.ts index 7d347a0e636a..d4932cebacb8 100644 --- a/test/js/node/http2/h2-conformance.test.ts +++ b/test/js/node/http2/h2-conformance.test.ts @@ -968,10 +968,24 @@ class RawH2Server { } } -/** HPACK string literal: 7-bit length prefix, no Huffman coding. */ +/** RFC 7541 §5.1 prefixed integer. `pattern` holds the opcode bits above the `prefixBits`-bit prefix. */ +function hpackInt(value: number, prefixBits: number, pattern: number): Buffer { + const max = (1 << prefixBits) - 1; + if (value < max) return Buffer.from([pattern | value]); + const bytes = [pattern | max]; + let rest = value - max; + while (rest >= 128) { + bytes.push((rest % 128) | 0x80); + rest = Math.floor(rest / 128); + } + bytes.push(rest); + return Buffer.from(bytes); +} + +/** HPACK string literal: 7-bit prefixed length, no Huffman coding. */ function hpackLiteral(str: string): Buffer { const bytes = Buffer.from(str, "latin1"); - return Buffer.concat([Buffer.from([bytes.length]), bytes]); + return Buffer.concat([hpackInt(bytes.length, 7, 0x00), bytes]); } describe("push stream states (checklist §5.1, RFC 9113 §6.4/§8.4)", () => { @@ -1141,6 +1155,430 @@ describe("SETTINGS ack ordering (RFC 9113 §6.5.3)", () => { }); }); +/** + * What an HPACK header block (RFC 7541 §6) does with the dynamic table: the §6.3 size updates it + * carries, and every index it resolves there (the static table ends at 61). + */ +function inspectHeaderBlock(block: Buffer): { sizeUpdates: number[]; dynamicIndexes: number[] } { + const sizeUpdates: number[] = []; + const dynamicIndexes: number[] = []; + let pos = 0; + // §5.1 prefixed integer. + const readInt = (prefixBits: number) => { + const max = (1 << prefixBits) - 1; + let value = block[pos++] & max; + if (value === max) { + let shift = 0; + let byte: number; + do { + byte = block[pos++]; + value += (byte & 0x7f) * 2 ** shift; + shift += 7; + } while (byte & 0x80); + } + return value; + }; + // §5.2 string literal. The Huffman flag sits above the 7-bit length prefix. + const skipString = () => { + const length = readInt(7); + pos += length; + }; + const literalField = (prefixBits: number) => { + const nameIndex = readInt(prefixBits); + if (nameIndex > 61) dynamicIndexes.push(nameIndex); + if (nameIndex === 0) skipString(); + skipString(); + }; + while (pos < block.length) { + const first = block[pos]; + if (first & 0x80) { + const index = readInt(7); // §6.1 indexed field + if (index > 61) dynamicIndexes.push(index); + } else if (first & 0x40) { + literalField(6); // §6.2.1 literal with incremental indexing + } else if (first & 0x20) { + sizeUpdates.push(readInt(5)); // §6.3 dynamic table size update + } else { + literalField(4); // §6.2.2 and §6.2.3 literal without indexing, never indexed + } + } + return { sizeUpdates, dynamicIndexes }; +} + +// Each HPACK dynamic table belongs to the side that decodes with it (RFC 7541 §4.2). The peer's +// SETTINGS_HEADER_TABLE_SIZE bounds our encoder, and the block after a change has to open with a +// §6.3 size update. Our own value bounds our decoder, from the moment the peer ACKs it. nginx +// sends 0 to every h2 upstream and fails a response that uses a dynamic index (#19152). nghttp2 +// (node, curl) answers a missing size update with GOAWAY(COMPRESSION_ERROR). +describe("SETTINGS_HEADER_TABLE_SIZE (RFC 7541 §4.2)", () => { + /** A SETTINGS payload with one HEADER_TABLE_SIZE (0x1) entry per value. */ + const headerTableSizes = (...values: number[]) => { + const payload = Buffer.alloc(6 * values.length); + values.forEach((value, i) => { + payload.writeUInt16BE(0x1, i * 6); + payload.writeUInt32BE(value, i * 6 + 2); + }); + return payload; + }; + /** RFC 7541 §6.3 Dynamic Table Size Update. */ + const sizeUpdate = (size: number) => hpackInt(size, 5, 0x20); + const isSettings = (f: Frame) => f.type === FrameType.SETTINGS && (f.flags & 0x1) === 0; + /** A SETTINGS frame (not an ACK) that sets HEADER_TABLE_SIZE to `size`. */ + const setsHeaderTableSize = (f: Frame, size: number) => { + if (!isSettings(f)) return false; + for (let i = 0; i + 6 <= f.payload.length; i += 6) { + if (f.payload.readUInt16BE(i) === 0x1 && f.payload.readUInt32BE(i + 2) === size) return true; + } + return false; + }; + + // 40 fields of about 243 bytes each (name + value + 32, RFC 7541 §4.1): a 4096-byte table holds 16. + const bigHeaders: Record = {}; + for (let i = 0; i < 40; i++) { + bigHeaders[`x-big-${i}`] = String(i).padStart(3, "0") + Buffer.alloc(200, "v").toString(); + } + + async function listen( + onStream: (stream: http2.ServerHttp2Stream, headers: http2.IncomingHttpHeaders) => void, + options: http2.ServerOptions = {}, + ) { + const h2server = http2.createServer(options); + h2server.on("stream", onStream); + h2server.listen(0, "127.0.0.1"); + await once(h2server, "listening"); + return { h2server, h2port: (h2server.address() as net.AddressInfo).port }; + } + + /** A raw client that opens the connection with the given SETTINGS payloads (default: one empty frame). */ + async function rawClient(h2port: number, ...settingsPayloads: Buffer[]) { + const c = await RawH2.connect(h2port); + c.sendPreface(); + if (settingsPayloads.length === 0) c.sendEmptySettings(); + for (const payload of settingsPayloads) c.sendFrame(FrameType.SETTINGS, 0, 0, payload); + await c.waitFor(isSettings); + c.sendSettingsAck(); + return c; + } + + /** The header block of a HEADERS or PUSH_PROMISE frame. */ + function headerBlock(frame: Frame) { + // END_HEADERS, not PADDED, no PRIORITY: the payload is the whole header block, after the + // 4-byte promised stream id of a PUSH_PROMISE. + expect(frame.flags & 0x2c).toBe(0x04); + return inspectHeaderBlock(frame.payload.subarray(frame.type === FrameType.PUSH_PROMISE ? 4 : 0)); + } + + /** A request on `streamId` (default: GET /), then the header block of the response. */ + async function responseBlock(c: RawH2, streamId: number, request = requestHeaderBlock("GET")) { + c.sendFrame(FrameType.HEADERS, 0x5 /* END_STREAM|END_HEADERS */, streamId, request); + return headerBlock(await c.waitFor(f => f.type === FrameType.HEADERS && f.streamId === streamId)); + } + + const respondWithSharedHeader = (stream: http2.ServerHttp2Stream) => { + stream.respond({ ":status": 200, "x-shared": "the same value on every response" }); + stream.end("ok"); + }; + + describe("the peer's value bounds the encoder", () => { + test("server: after the peer sends 0, the next response says so and no response uses the table", async () => { + const { h2server, h2port } = await listen(respondWithSharedHeader); + const c = await rawClient(h2port, headerTableSizes(0)); + try { + expect([await responseBlock(c, 1), await responseBlock(c, 3), await responseBlock(c, 5)]).toEqual([ + { sizeUpdates: [0], dynamicIndexes: [] }, + { sizeUpdates: [], dynamicIndexes: [] }, + { sizeUpdates: [], dynamicIndexes: [] }, + ]); + } finally { + c.destroy(); + h2server.close(); + } + }); + + // The peer's decoder evicts at each of its SETTINGS values, so the smallest one since the + // last block goes first. nghttp2 rejects a first size update above that minimum. + test.each([ + ["two SETTINGS frames", [headerTableSizes(0), headerTableSizes(4096)]], + ["two entries of one SETTINGS frame", [headerTableSizes(0, 4096)]], + ])("server: 0 then 4096 in %s is announced as the minimum, then the final size", async (_, settingsPayloads) => { + const { h2server, h2port } = await listen(respondWithSharedHeader); + const c = await rawClient(h2port, ...settingsPayloads); + try { + expect(await responseBlock(c, 1)).toEqual({ sizeUpdates: [0, 4096], dynamicIndexes: [] }); + // The table is in use again: "x-shared" now comes out of it. + const second = await responseBlock(c, 3); + expect(second.sizeUpdates).toEqual([]); + expect(second.dynamicIndexes.length).toBeGreaterThan(0); + } finally { + c.destroy(); + h2server.close(); + } + }); + + test("server: a header block that is never sent does not swallow the size update", async () => { + const dropped = new Error("dropped"); + let thrown: unknown; + const { h2server, h2port } = await listen(stream => { + try { + // ":status" is already encoded when the value of "x-throws" is read. + const throws = { + toString() { + throw dropped; + }, + }; + stream.respond({ ":status": 200, "x-throws": throws as unknown as string }); + } catch (e) { + thrown = e; + } + stream.respond({ ":status": 200 }); + stream.end("ok"); + }); + const c = await rawClient(h2port, headerTableSizes(0)); + try { + expect(await responseBlock(c, 1)).toEqual({ sizeUpdates: [0], dynamicIndexes: [] }); + expect(thrown).toBe(dropped); + } finally { + c.destroy(); + h2server.close(); + } + }); + + test("server: trailers say so when the change arrives after the response headers", async () => { + const { h2server, h2port } = await listen(stream => { + stream.respond({ ":status": 200 }, { waitForTrailers: true }); + stream.on("wantTrailers", () => stream.sendTrailers({ "x-trailer": "done" })); + stream.session!.on("remoteSettings", settings => { + if (settings.headerTableSize === 0) stream.end("ok"); + }); + }); + const c = await rawClient(h2port); + try { + const response = await responseBlock(c, 1); + c.sendFrame(FrameType.SETTINGS, 0, 0, headerTableSizes(0)); + const trailers = await c.waitFor( + f => f.type === FrameType.HEADERS && f.streamId === 1 && (f.flags & 0x1) !== 0, + ); + expect([response, headerBlock(trailers)]).toEqual([ + { sizeUpdates: [], dynamicIndexes: [] }, + { sizeUpdates: [0], dynamicIndexes: [] }, + ]); + } finally { + c.destroy(); + h2server.close(); + } + }); + + test("server: a PUSH_PROMISE says so when it is the first header block", async () => { + const { h2server, h2port } = await listen(stream => { + stream.pushStream({ ":path": "/pushed" }, (err, pushed) => { + if (err) throw err; + pushed.respond({ ":status": 200 }); + pushed.end("pushed"); + }); + stream.respond({ ":status": 200 }); + stream.end("ok"); + }); + const c = await rawClient(h2port, headerTableSizes(0)); + try { + c.sendFrame(FrameType.HEADERS, 0x5 /* END_STREAM|END_HEADERS */, 1, requestHeaderBlock("GET")); + const promise = await c.waitFor(f => f.type === FrameType.PUSH_PROMISE && f.streamId === 1); + const responses = [ + await c.waitFor(f => f.type === FrameType.HEADERS && f.streamId === 1), + await c.waitFor(f => f.type === FrameType.HEADERS && f.streamId === 2), + ]; + expect([promise, ...responses].map(headerBlock)).toEqual([ + { sizeUpdates: [0], dynamicIndexes: [] }, + { sizeUpdates: [], dynamicIndexes: [] }, + { sizeUpdates: [], dynamicIndexes: [] }, + ]); + } finally { + c.destroy(); + h2server.close(); + } + }); + + test("server: a peer table size above 4096 does not grow the encoder's table", async () => { + const { h2server, h2port } = await listen(stream => { + stream.respond({ ":status": 200, ...bigHeaders }); + stream.end("ok"); + }); + const c = await rawClient(h2port, headerTableSizes(65536)); + try { + const blocks = [await responseBlock(c, 1), await responseBlock(c, 3)]; + expect(blocks.map(b => b.sizeUpdates)).toEqual([[], []]); + // 16 entries fit in 4096 bytes, so no index can go past 61 + 16. + expect(Math.max(0, ...blocks.flatMap(b => b.dynamicIndexes))).toBeLessThanOrEqual(77); + } finally { + c.destroy(); + h2server.close(); + } + }); + + test("server: a local headerTableSize above 4096 does not size the encoder", async () => { + // The client's decoder keeps the default 4096 bytes. An encoder that kept 65536 bytes of + // entries would point at fields the client has already evicted. + const { h2server, h2port } = await listen( + stream => { + stream.respond({ ":status": 200, ...bigHeaders }); + stream.end(); + }, + { settings: { headerTableSize: 65536 } }, + ); + const client = http2.connect(`http://127.0.0.1:${h2port}`); + try { + for (let i = 0; i < 3; i++) { + const req = client.request({ ":path": "/" }); + const [headers] = await once(req, "response"); + expect(headers).toMatchObject(bigHeaders); + req.resume(); + await once(req, "close"); + } + } finally { + client.close(); + h2server.close(); + } + }); + + test("client: after the server sends 0, the next request says so and no request uses the table", async () => { + const raw = await RawH2Server.listen(); + const client = http2.connect(`http://127.0.0.1:${raw.port}`); + client.on("error", () => {}); + /** Sends a request, answers it with a bare 200, returns the request's header block. */ + const requestBlock = async (streamId: number, beforeResponse?: () => Promise) => { + const req = client.request({ ":path": "/", "x-request-id": "abcdef0123456789" }); + req.on("error", () => {}); + req.resume(); + const headers = await raw.waitFor(f => f.type === FrameType.HEADERS && f.streamId === streamId); + await beforeResponse?.(); + raw.sendFrame(FrameType.HEADERS, 0x5 /* END_STREAM|END_HEADERS */, streamId, Buffer.from([0x88])); + await once(req, "close"); + return headerBlock(headers); + }; + try { + // The first request leaves before the server's SETTINGS arrives. + const first = await requestBlock(1, () => { + raw.sendFrame(FrameType.SETTINGS, 0, 0, headerTableSizes(0)); + raw.sendFrame(FrameType.SETTINGS, 0x1, 0); + // The client's ACK: from here on its encoder is bound by 0. + return raw.waitFor(f => f.type === FrameType.SETTINGS && (f.flags & 0x1) === 1); + }); + expect([first, await requestBlock(3), await requestBlock(5)]).toEqual([ + { sizeUpdates: [], dynamicIndexes: [] }, + { sizeUpdates: [0], dynamicIndexes: [] }, + { sizeUpdates: [], dynamicIndexes: [] }, + ]); + } finally { + client.destroy(); + raw.close(); + } + }); + }); + + describe("the local value bounds the decoder once the peer ACKs it", () => { + /** A literal field with incremental indexing (§6.2.1): the first one becomes dynamic index 62. */ + const indexed = (name: string, value: string) => + Buffer.concat([Buffer.from([0x40]), hpackLiteral(name), hpackLiteral(value)]); + const DYNAMIC_INDEX_62 = Buffer.from([0x80 | 62]); + + /** A server that records the "x-probe" request header of every stream. */ + async function probeServer(options: http2.ServerOptions, onFirstStream?: (session: http2.Http2Session) => void) { + const probes: (string | undefined)[] = []; + const listening = await listen((stream, headers) => { + probes.push(headers["x-probe"] as string | undefined); + stream.respond({ ":status": 200 }); + stream.end("ok"); + if (probes.length === 1) onFirstStream?.(stream.session!); + }, options); + return { ...listening, probes }; + } + + test("a local 0 does not bind the decoder before the peer ACKs it", async () => { + const { h2server, h2port, probes } = await probeServer({ settings: { headerTableSize: 0 } }); + const c = await RawH2.connect(h2port); + try { + c.sendPreface(); + c.sendEmptySettings(); + // No ACK for the server's SETTINGS yet: this encoder still works against the default 4096. + await responseBlock(c, 1, requestHeaderBlock("GET", indexed("x-probe", "kept in the table"))); + await responseBlock(c, 3, requestHeaderBlock("GET", DYNAMIC_INDEX_62)); + expect(probes).toEqual(["kept in the table", "kept in the table"]); + expect(c.frames.filter(f => f.type === FrameType.GOAWAY)).toEqual([]); + } finally { + c.destroy(); + h2server.close(); + } + }); + + test("after the ACK, a size update above the local value is a COMPRESSION_ERROR", async () => { + const { h2server, h2port } = await probeServer({ settings: { headerTableSize: 0 } }); + const c = await rawClient(h2port); + try { + c.sendFrame(FrameType.HEADERS, 0x5, 1, Buffer.concat([sizeUpdate(4096), requestHeaderBlock("GET")])); + expect(goawayErrorCode(await c.waitForGoaway())).toBe(ErrorCode.COMPRESSION_ERROR); + } finally { + c.destroy(); + h2server.close(); + } + }); + + test("a raised local value is the new limit once the peer ACKs it, and the table grows", async () => { + const RAISED = 8192; + const { h2server, h2port, probes } = await probeServer({}, session => + session.settings({ headerTableSize: RAISED }), + ); + const c = await rawClient(h2port); + try { + await responseBlock(c, 1); + await c.waitFor(f => setsHeaderTableSize(f, RAISED)); + c.sendSettingsAck(); + // 7 + 6000 + 32 bytes: this entry only fits in the raised table. + const big = indexed("x-probe", Buffer.alloc(6000, "p").toString()); + await responseBlock(c, 3, Buffer.concat([sizeUpdate(RAISED), requestHeaderBlock("GET", big)])); + await responseBlock(c, 5, requestHeaderBlock("GET", DYNAMIC_INDEX_62)); + expect(probes.map(p => p?.length)).toEqual([undefined, 6000, 6000]); + expect(c.frames.filter(f => f.type === FrameType.GOAWAY)).toEqual([]); + } finally { + c.destroy(); + h2server.close(); + } + }); + + // Both halves against each other: the peer's encoder announces the raised size, and the + // decoder of the side that raised it has to accept that. + test.each([ + ["server", 1024], + ["server", 0], + ["client", 1024], + ["client", 0], + ] as const)("a %s that raises its headerTableSize from %d to 4096 keeps the session", async (side, initial) => { + const settings = { headerTableSize: initial }; + let serverSession!: http2.Http2Session; + const { h2server, h2port } = await listen(respondWithSharedHeader, side === "server" ? { settings } : {}); + h2server.on("session", session => (serverSession = session)); + const client = http2.connect(`http://127.0.0.1:${h2port}`, side === "client" ? { settings } : {}); + const get = async () => { + const req = client.request({ ":path": "/", "x-request-id": "abcdef0123456789" }); + const [headers] = await once(req, "response"); + req.resume(); + await once(req, "close"); + return headers[":status"]; + }; + try { + const statuses = [await get()]; + const raised = Promise.withResolvers(); + // The callback runs when the peer's ACK arrives. + (side === "server" ? serverSession : client).settings({ headerTableSize: 4096 }, () => raised.resolve()); + await raised.promise; + statuses.push(await get(), await get()); + expect(statuses).toEqual([200, 200, 200]); + } finally { + client.close(); + h2server.close(); + } + }); + }); +}); + function requestHeaderBlock(method: "GET" | "POST", extra: Buffer = Buffer.alloc(0)): Buffer { return Buffer.concat([ Buffer.from([method === "POST" ? 0x83 : 0x82, 0x86, 0x84, 0x01]), From 5d9069e52c53c373b05a6966509cd3a4cadafdf1 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 18 Sep 2026 19:08:03 +0000 Subject: [PATCH 2/5] node:http2: accept a header block of only size updates, keep repeated size updates in step A header block that holds RFC 7541 6.3 size updates and no field is valid. lshpack applies the updates and then reports an error because no field follows, so the session died with COMPRESSION_ERROR. The engine now ends such a block cleanly. An update above the acknowledged limit is still an error. A pending size update can be written into more than one block. Each time, the encoder now evicts down to the announced minimum, as the peer does when the update arrives. session.state reports the real size of each table: the peer's value (capped) for the encoder, the acknowledged local value for the decoder. The conformance tests no longer tear down a raw client while a pushed stream is open without an error listener. On Windows that reset the stream and failed the run. --- src/runtime/api/bun/h2/connection.rs | 5 ++ src/runtime/api/bun/h2/hpack.rs | 53 +++++++++++++++++- src/runtime/api/bun/h2_frame_parser.rs | 14 +++-- test/js/node/http2/h2-conformance.test.ts | 66 ++++++++++++++++++++++- 4 files changed, 133 insertions(+), 5 deletions(-) diff --git a/src/runtime/api/bun/h2/connection.rs b/src/runtime/api/bun/h2/connection.rs index 66030e6cf3cf..89a46b8a7386 100644 --- a/src/runtime/api/bun/h2/connection.rs +++ b/src/runtime/api/bun/h2/connection.rs @@ -1411,6 +1411,11 @@ impl Connection { sink.on_header(target, h.name, h.value, h.never_index); } Err(_) => { + // A block of size updates and no field, for example empty trailers right after + // a table size change. The decoder has applied the updates. + if off == 0 && self.hpack.is_size_update_only(&block) { + break; + } // §4.3: a header-block decoding error is a connection COMPRESSION_ERROR. self.send_go_away(sink, ErrorCode::CompressionError, b"HPACK decode error"); fatal = true; diff --git a/src/runtime/api/bun/h2/hpack.rs b/src/runtime/api/bun/h2/hpack.rs index 8edc6bb80404..8602f288f065 100644 --- a/src/runtime/api/bun/h2/hpack.rs +++ b/src/runtime/api/bun/h2/hpack.rs @@ -59,6 +59,11 @@ impl Coder { self.dec_capacity = size; } + /// Capacity of the encoder's dynamic table: the peer's value, capped. + pub(crate) fn encoder_capacity(&self) -> u32 { + self.enc_capacity + } + /// One SETTINGS_HEADER_TABLE_SIZE entry from the peer; call it for every entry, in wire /// order. Evicts right away, like the peer's decoder did when it sent the value. pub(crate) fn set_peer_header_table_size(&mut self, size: u32) { @@ -77,11 +82,17 @@ impl Coder { /// Call at the start of every outbound header block, before its first field: appends the /// §6.3 size update(s) the peer's decoder still has to see. They stay pending until /// [`Self::size_update_sent`], so a block that is built but never sent does not lose them. - pub(crate) fn write_pending_size_update(&self, block: &mut Vec) { + /// + /// More than one block can carry them: a block opened while another one is still being built, + /// or the block after a dropped one. An update to the minimum makes the peer evict down to + /// it each time it arrives, so the encoder evicts the same way each time it writes one. + pub(crate) fn write_pending_size_update(&mut self, block: &mut Vec) { let Some(min) = self.unannounced_min else { return; }; if min < self.enc_capacity { + self.hpack.set_encoder_max_capacity(min); + self.hpack.set_encoder_max_capacity(self.enc_capacity); write_table_size_update(block, min); } write_table_size_update(block, self.enc_capacity); @@ -110,6 +121,46 @@ impl Coder { pub(crate) fn decode<'a>(&'a mut self, src: &[u8]) -> Result, HpackError> { self.hpack.decode(src) } + + /// True when `block` holds §6.3 size updates and nothing else, all within the acknowledged + /// limit. Such a block is valid and has no fields. lshpack applies the updates and then + /// fails [`Self::decode`] because no field follows, so a caller asks this after that failure. + /// Every other failure of `decode` makes this false. + pub(crate) fn is_size_update_only(&self, block: &[u8]) -> bool { + let mut rest = block; + if rest.is_empty() { + return false; + } + while let Some((&first, tail)) = rest.split_first() { + if first & 0xe0 != 0x20 { + return false; + } + rest = tail; + let mut value = u64::from(first & 0x1f); + if value == 0x1f { + // Up to 4 continuation bytes: the range lshpack accepts without further checks. + let mut shift = 0; + loop { + let Some((&byte, tail)) = rest.split_first() else { + return false; + }; + rest = tail; + if shift > 21 { + return false; + } + value += u64::from(byte & 0x7f) << shift; + shift += 7; + if byte & 0x80 == 0 { + break; + } + } + } + if value > u64::from(self.dec_capacity) { + return false; + } + } + true + } } /// RFC 7541 §5.1 + §6.3: append `value` as a 5-bit-prefix integer with the `001` pattern (0x20). diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index f56d28a4e62c..48ce6343fd56 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -1036,6 +1036,9 @@ pub(crate) struct H2FrameParser { /// a header block already in flight when the limit is lowered must not be rejected; the /// engine raises/lowers its own limit as ACKs arrive). enforced_max_header_list_size: Cell, + /// The engine decoder's table limit: our SETTINGS_HEADER_TABLE_SIZE that the peer ACKed last. + /// Mirrored here so `state` never needs the engine borrow. + acked_header_table_size: Cell, // only available after receiving settings or ACK remote_settings: Cell>, @@ -3854,6 +3857,7 @@ impl crate::api::h2::connection::Sink for H2FrameParser { // legacy settings() host fn doesn't hit MAX_PENDING_SETTINGS_ACK. self.outstanding_settings .set(self.outstanding_settings.get().saturating_sub(1)); + self.acked_header_table_size.set(settings.header_table_size); let g = self.global(); let js = rewrite_settings_to_js(settings, g); // node exposes the custom settings this side submitted on localSettings.customSettings. @@ -4727,7 +4731,10 @@ impl H2FrameParser { let settings = self.remote_settings.get().unwrap_or_default(); let remote_iws = settings.initial_window_size; let local_iws = self.local_settings.get().initial_window_size; - let local_hts = self.local_settings.get().header_table_size; + let encoder_table_size = match self.hpack.get() { + Some(hpack) => hpack.encoder_capacity(), + None => crate::api::h2::hpack::DEFAULT_HEADER_TABLE_SIZE, + }; result.put( global_object, b"remoteWindowSize", @@ -4741,12 +4748,12 @@ impl H2FrameParser { result.put( global_object, b"deflateDynamicTableSize", - JSValue::js_number(local_hts as f64), + JSValue::js_number(encoder_table_size as f64), ); result.put( global_object, b"inflateDynamicTableSize", - JSValue::js_number(local_hts as f64), + JSValue::js_number(self.acked_header_table_size.get() as f64), ); result.put( global_object, @@ -7388,6 +7395,7 @@ impl H2FrameParser { wire_custom_settings: JsCell::new(Vec::new()), remote_custom_settings_filter: JsCell::new(Vec::new()), remote_custom_settings: JsCell::new(Vec::new()), + acked_header_table_size: Cell::new(crate::api::h2::hpack::DEFAULT_HEADER_TABLE_SIZE), enforced_max_header_list_size: Cell::new( FullSettingsPayload::default().max_header_list_size, ), diff --git a/test/js/node/http2/h2-conformance.test.ts b/test/js/node/http2/h2-conformance.test.ts index d4932cebacb8..8cfc29130e45 100644 --- a/test/js/node/http2/h2-conformance.test.ts +++ b/test/js/node/http2/h2-conformance.test.ts @@ -1243,7 +1243,12 @@ describe("SETTINGS_HEADER_TABLE_SIZE (RFC 7541 §4.2)", () => { options: http2.ServerOptions = {}, ) { const h2server = http2.createServer(options); - h2server.on("stream", onStream); + h2server.on("stream", (stream, headers) => { + // The raw client goes away as soon as it has the frames it checks. On Windows that resets + // a stream that is still open. + stream.on("error", () => {}); + onStream(stream, headers); + }); h2server.listen(0, "127.0.0.1"); await once(h2server, "listening"); return { h2server, h2port: (h2server.address() as net.AddressInfo).port }; @@ -1372,6 +1377,8 @@ describe("SETTINGS_HEADER_TABLE_SIZE (RFC 7541 §4.2)", () => { const { h2server, h2port } = await listen(stream => { stream.pushStream({ ":path": "/pushed" }, (err, pushed) => { if (err) throw err; + // Not a 'stream' event of the server, so `listen` does not cover it. + pushed.on("error", () => {}); pushed.respond({ ":status": 200 }); pushed.end("pushed"); }); @@ -1386,6 +1393,10 @@ describe("SETTINGS_HEADER_TABLE_SIZE (RFC 7541 §4.2)", () => { await c.waitFor(f => f.type === FrameType.HEADERS && f.streamId === 1), await c.waitFor(f => f.type === FrameType.HEADERS && f.streamId === 2), ]; + // Let both responses finish, so the teardown below does not cut one off. + for (const streamId of [1, 2]) { + await c.waitFor(f => f.type === FrameType.DATA && f.streamId === streamId && (f.flags & 0x1) !== 0); + } expect([promise, ...responses].map(headerBlock)).toEqual([ { sizeUpdates: [0], dynamicIndexes: [] }, { sizeUpdates: [], dynamicIndexes: [] }, @@ -1521,6 +1532,59 @@ describe("SETTINGS_HEADER_TABLE_SIZE (RFC 7541 §4.2)", () => { } }); + // An encoder with a pending size update puts it at the start of its next block, and that + // block can be empty trailers. The peers this change makes correct send such blocks. + test.each([ + ["within the limit is a valid block with no field", 4096, "trailers {}"], + ["above the limit is a COMPRESSION_ERROR", 8192, "GOAWAY 9"], + ])("a trailers block that holds only a size update %s", async (_, size, expected) => { + const trailers = Promise.withResolvers(); + const { h2server, h2port } = await listen(stream => { + stream.on("trailers", t => trailers.resolve(`trailers ${JSON.stringify(t)}`)); + stream.resume(); + }); + const c = await rawClient(h2port); + try { + c.sendFrame(FrameType.HEADERS, 0x4 /* END_HEADERS */, 1, requestHeaderBlock("POST")); + c.sendFrame(FrameType.DATA, 0, 1, Buffer.from("body")); + c.sendFrame(FrameType.HEADERS, 0x5 /* END_STREAM|END_HEADERS */, 1, sizeUpdate(size)); + const goaway = c.waitForGoaway().then(f => `GOAWAY ${goawayErrorCode(f)}`); + goaway.catch(() => {}); + expect(await Promise.race([trailers.promise, goaway])).toBe(expected); + } finally { + c.destroy(); + h2server.close(); + } + }); + + test("session.state reports the size of each table, not the local setting", async () => { + const states: http2.SessionState[] = []; + const { h2server, h2port } = await listen( + stream => { + states.push(stream.session!.state); + stream.respond({ ":status": 200 }); + stream.end("ok"); + }, + { settings: { headerTableSize: 65536 } }, + ); + const c = await RawH2.connect(h2port); + try { + c.sendPreface(); + c.sendFrame(FrameType.SETTINGS, 0, 0, headerTableSizes(1024)); + // Request 1 arrives before this peer ACKs the server's 65536. Request 3 arrives after. + await responseBlock(c, 1); + c.sendSettingsAck(); + await responseBlock(c, 3); + expect(states.map(s => [s.deflateDynamicTableSize, s.inflateDynamicTableSize])).toEqual([ + [1024, 4096], + [1024, 65536], + ]); + } finally { + c.destroy(); + h2server.close(); + } + }); + test("a raised local value is the new limit once the peer ACKs it, and the table grows", async () => { const RAISED = 8192; const { h2server, h2port, probes } = await probeServer({}, session => From 4e7d56e32398573e821a1a8205157900ab20a5cb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 18 Sep 2026 19:40:59 +0000 Subject: [PATCH 3/5] node:http2: shorten the HPACK comments, take out the repeated eviction The encoder evicted again each time it wrote a pending size update. No test covers a second write, so the encoder writes the update and does nothing else, as in the first commit. --- src/http/lshpack.rs | 5 +-- src/runtime/api/bun/h2/connection.rs | 10 ++--- src/runtime/api/bun/h2/hpack.rs | 61 ++++++++------------------ src/runtime/api/bun/h2_frame_parser.rs | 13 +++--- 4 files changed, 28 insertions(+), 61 deletions(-) diff --git a/src/http/lshpack.rs b/src/http/lshpack.rs index da9f1cbc6f2e..3884b52e4add 100644 --- a/src/http/lshpack.rs +++ b/src/http/lshpack.rs @@ -126,9 +126,8 @@ impl HPACK { lshpack_wrapper_enc_set_max_capacity(self, max_capacity as c_uint); } - /// Adjust the decoder's dynamic-table capacity after init: the limit on the - /// peer's RFC 7541 §6.3 Dynamic Table Size Updates, and the table's size - /// until the peer sends one. Evicts entries to fit. + /// Sets the limit on the peer's RFC 7541 §6.3 size updates and the current + /// table size, both. Evicts entries to fit. pub fn set_decoder_max_capacity(&mut self, max_capacity: u32) { lshpack_wrapper_dec_set_max_capacity(self, max_capacity as c_uint); } diff --git a/src/runtime/api/bun/h2/connection.rs b/src/runtime/api/bun/h2/connection.rs index 89a46b8a7386..c079c34dd949 100644 --- a/src/runtime/api/bun/h2/connection.rs +++ b/src/runtime/api/bun/h2/connection.rs @@ -254,9 +254,7 @@ pub(crate) trait Sink { } /// A SETTINGS entry with an id outside the standard registry (node's remoteCustomSettings). fn on_remote_custom_setting(&self, _id: u16, _value: u32) {} - /// Transition shim while the outbound path still flows through the embedder's legacy encoder: - /// one SETTINGS_HEADER_TABLE_SIZE entry from the peer, for every entry in wire order, so that - /// encoder can follow it (`hpack::Coder::set_peer_header_table_size`). + /// One SETTINGS_HEADER_TABLE_SIZE entry from the peer, called per entry in wire order. fn on_remote_header_table_size(&self, _size: u32) {} /// One decoded header field. `name`/`value` alias a shared buffer — copy before returning. fn on_header(&self, _stream_id: u32, _name: &[u8], _value: &[u8], _never_index: bool) {} @@ -842,8 +840,7 @@ impl Connection { return true; } self.remote_settings.apply(sid, value); - // The peer's HEADER_TABLE_SIZE governs OUR encoder. Every entry counts, not only - // the last one: the peer's decoder evicts at each (RFC 7541 §4.2). + // Per entry, not once per frame: the peer's decoder evicts at each (RFC 7541 §4.2). if sid == SettingId::HeaderTableSize { self.hpack.set_peer_header_table_size(value); sink.on_remote_header_table_size(value); @@ -1411,8 +1408,7 @@ impl Connection { sink.on_header(target, h.name, h.value, h.never_index); } Err(_) => { - // A block of size updates and no field, for example empty trailers right after - // a table size change. The decoder has applied the updates. + // Size updates and no field, for example empty trailers. lshpack applied them. if off == 0 && self.hpack.is_size_update_only(&block) { break; } diff --git a/src/runtime/api/bun/h2/hpack.rs b/src/runtime/api/bun/h2/hpack.rs index 8602f288f065..6a1e3b2e44ae 100644 --- a/src/runtime/api/bun/h2/hpack.rs +++ b/src/runtime/api/bun/h2/hpack.rs @@ -1,43 +1,32 @@ //! HPACK coder (RFC 7541) over the lshpack binding — the only reused piece in the rewrite. //! -//! Centralizes the dynamic-table-size handling. Each table belongs to the side that decodes with -//! it: the encoder's table follows the PEER's SETTINGS_HEADER_TABLE_SIZE, and every change is -//! announced with a §6.3 Dynamic Table Size Update at the start of the next header block so the -//! peer's decoder evicts in lockstep. The decoder's table follows OUR setting, from the moment -//! the peer acknowledges it. Decode results alias a shared buffer and MUST be copied before the -//! next call (see lshpack.rs). +//! Centralizes the dynamic-table-size handling: the encoder follows the PEER's +//! SETTINGS_HEADER_TABLE_SIZE and announces each change with a §6.3 Dynamic Table Size Update at +//! the start of the next header block. The decoder follows OUR setting once the peer ACKs it. +//! Decode results alias a shared buffer and MUST be copied before the next call (see lshpack.rs). #![allow(dead_code)] use bun_http::lshpack::{DecodeResult, HpackError, HpackHandle}; -/// RFC 9113 §6.5.2: SETTINGS_HEADER_TABLE_SIZE in both directions until a SETTINGS frame says -/// otherwise. +/// RFC 9113 §6.5.2: the initial SETTINGS_HEADER_TABLE_SIZE of both sides. pub(crate) const DEFAULT_HEADER_TABLE_SIZE: u32 = 4096; -/// Upper bound of the encoder's dynamic table, whatever the peer allows: a peer that advertises a -/// huge table must not make this side retain that much header history. Node's default +/// A peer must not decide how much header history this side retains. Node's default /// `maxDeflateDynamicTableSize`. pub(crate) const MAX_ENCODER_TABLE_SIZE: u32 = 4096; pub(crate) struct Coder { hpack: HpackHandle, - /// Capacity of the encoder's dynamic table. enc_capacity: u32, - /// `Some` while the peer's decoder has not been told about a capacity change: the smallest - /// capacity the encoder had since the last header block that reached the transport. The - /// peer's decoder evicts at each of its SETTINGS, so RFC 7541 §4.2 wants that minimum - /// signaled before the final value. + /// `Some` while a capacity change is not announced: the smallest capacity since the last + /// block that was sent. RFC 7541 §4.2 wants that minimum signaled before the final value. unannounced_min: Option, - /// Limit of the decoder's dynamic table: our SETTINGS_HEADER_TABLE_SIZE that the peer - /// acknowledged last. + /// Our SETTINGS_HEADER_TABLE_SIZE that the peer ACKed last. dec_capacity: u32, } impl Coder { - /// Both tables start at the protocol default. The encoder moves with - /// [`Self::set_peer_header_table_size`], the decoder with - /// [`Self::set_acked_header_table_size`]. pub(crate) fn new() -> Self { Coder { hpack: HpackHandle::new(DEFAULT_HEADER_TABLE_SIZE), @@ -47,10 +36,8 @@ impl Coder { } } - /// The peer acknowledged a SETTINGS frame of ours that carried this - /// SETTINGS_HEADER_TABLE_SIZE. Until the ACK the peer's encoder still works against the - /// previous value, so the decoder must not move earlier. From the ACK on, the value bounds - /// the peer's §6.3 size updates. + /// Call on the SETTINGS ACK, not when the SETTINGS frame is sent: until the ACK the peer's + /// encoder still works against the previous value. pub(crate) fn set_acked_header_table_size(&mut self, size: u32) { if size == self.dec_capacity { return; @@ -59,13 +46,11 @@ impl Coder { self.dec_capacity = size; } - /// Capacity of the encoder's dynamic table: the peer's value, capped. pub(crate) fn encoder_capacity(&self) -> u32 { self.enc_capacity } - /// One SETTINGS_HEADER_TABLE_SIZE entry from the peer; call it for every entry, in wire - /// order. Evicts right away, like the peer's decoder did when it sent the value. + /// Call for every SETTINGS_HEADER_TABLE_SIZE entry of the peer, in wire order. pub(crate) fn set_peer_header_table_size(&mut self, size: u32) { let capacity = size.min(MAX_ENCODER_TABLE_SIZE); if capacity == self.enc_capacity && self.unannounced_min.is_none() { @@ -79,27 +64,18 @@ impl Coder { }); } - /// Call at the start of every outbound header block, before its first field: appends the - /// §6.3 size update(s) the peer's decoder still has to see. They stay pending until - /// [`Self::size_update_sent`], so a block that is built but never sent does not lose them. - /// - /// More than one block can carry them: a block opened while another one is still being built, - /// or the block after a dropped one. An update to the minimum makes the peer evict down to - /// it each time it arrives, so the encoder evicts the same way each time it writes one. - pub(crate) fn write_pending_size_update(&mut self, block: &mut Vec) { + /// Call at the start of every outbound header block. The update stays pending until + /// [`Self::size_update_sent`], so a block that is built but never sent does not lose it. + pub(crate) fn write_pending_size_update(&self, block: &mut Vec) { let Some(min) = self.unannounced_min else { return; }; if min < self.enc_capacity { - self.hpack.set_encoder_max_capacity(min); - self.hpack.set_encoder_max_capacity(self.enc_capacity); write_table_size_update(block, min); } write_table_size_update(block, self.enc_capacity); } - /// The header block opened with [`Self::write_pending_size_update`] was handed to the - /// transport. pub(crate) fn size_update_sent(&mut self) { self.unannounced_min = None; } @@ -122,10 +98,9 @@ impl Coder { self.hpack.decode(src) } - /// True when `block` holds §6.3 size updates and nothing else, all within the acknowledged - /// limit. Such a block is valid and has no fields. lshpack applies the updates and then - /// fails [`Self::decode`] because no field follows, so a caller asks this after that failure. - /// Every other failure of `decode` makes this false. + /// True when `block` is only §6.3 size updates within the ACKed limit: a valid block with no + /// field. lshpack applies such updates and then fails [`Self::decode`] because no field + /// follows, so ask this after that failure. pub(crate) fn is_size_update_only(&self, block: &[u8]) -> bool { let mut rest = block; if rest.is_empty() { diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index 48ce6343fd56..1be9828a5a27 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -1036,8 +1036,7 @@ pub(crate) struct H2FrameParser { /// a header block already in flight when the limit is lowered must not be rejected; the /// engine raises/lowers its own limit as ACKs arrive). enforced_max_header_list_size: Cell, - /// The engine decoder's table limit: our SETTINGS_HEADER_TABLE_SIZE that the peer ACKed last. - /// Mirrored here so `state` never needs the engine borrow. + /// The engine decoder's table limit, mirrored so `state` never needs the engine borrow. acked_header_table_size: Cell, // only available after receiving settings or ACK remote_settings: Cell>, @@ -2000,8 +1999,8 @@ impl H2FrameParser { }) } - /// Opens an outbound header block: the peer's decoder learns about a changed dynamic table - /// size here and nowhere else (RFC 7541 §4.2). Pair with `header_block_sent`. + /// Call before the first field of every outbound header block. Pair with + /// `header_block_sent`. fn begin_header_block(&self, encoded_headers: &mut Vec) { self.hpack.with_mut(|hpack| { if let Some(hpack) = hpack.as_mut() { @@ -2010,9 +2009,8 @@ impl H2FrameParser { }); } - /// The block opened with `begin_header_block` was written. A block that is dropped before - /// this (a header that fails validation, an option that throws) keeps the size update pending - /// for the next one. + /// Call after the block's frames are written. A block that is dropped before this keeps the + /// size update pending for the next one. fn header_block_sent(&self) { self.hpack.with_mut(|hpack| { if let Some(hpack) = hpack.as_mut() { @@ -7633,7 +7631,6 @@ impl H2FrameParser { unsafe { bun_jsc::AbortHandle::arm_owner(this, context) }; } - // Our own headerTableSize sizes the engine's decoder. This encoder follows the peer's. this_ref .hpack .set(Some(crate::api::h2::hpack::Coder::new())); From dec174f36c2d4ad5cb990623f84dd245c6693ca0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 18 Sep 2026 20:22:41 +0000 Subject: [PATCH 4/5] node:http2: consume a pending HPACK size update before the header block is written A JS transport gets a header block that is larger than the cork buffer from inside request(). A peer that answers inside that write can deliver a SETTINGS frame with a new header table size before request() is done with the block. The pending size update was cleared after the write, so that new size was never announced. A header block that user code sent from inside the same write repeated the size update of the outer block, and the peer evicted the entries the outer block had just inserted. The update is now consumed once nothing can drop the block any more, before its first frame byte is written. A block that is dropped before that point keeps the update pending, as before. --- src/runtime/api/bun/h2/connection.rs | 4 +- src/runtime/api/bun/h2/hpack.rs | 4 +- src/runtime/api/bun/h2_frame_parser.rs | 18 ++-- test/js/node/http2/h2-conformance.test.ts | 111 +++++++++++++++++++++- 4 files changed, 124 insertions(+), 13 deletions(-) diff --git a/src/runtime/api/bun/h2/connection.rs b/src/runtime/api/bun/h2/connection.rs index c079c34dd949..92dce6f31f74 100644 --- a/src/runtime/api/bun/h2/connection.rs +++ b/src/runtime/api/bun/h2/connection.rs @@ -2043,6 +2043,7 @@ impl Connection { /// it exceeds the peer's max frame size (§4.3/§6.10), and advance the send-side stream state. pub(crate) fn send_header_block(&mut self, sink: &impl Sink, stream_id: u32, end_stream: bool) { let block = std::mem::take(&mut self.enc_buf); + self.hpack.size_update_committed(); let max = (self.remote_settings.max_frame_size as usize).max(1); let total = block.len(); @@ -2078,7 +2079,6 @@ impl Connection { ); off += len; } - self.hpack.size_update_sent(); self.enc_buf = block; self.enc_buf.clear(); @@ -2173,6 +2173,7 @@ impl Connection { /// promised request headers staged via begin_header_block/encode_header (RFC 9113 §6.6). pub(crate) fn send_push_promise(&mut self, sink: &impl Sink, parent_id: u32, promised_id: u32) { let block = std::mem::take(&mut self.enc_buf); + self.hpack.size_update_committed(); let max = (self.remote_settings.max_frame_size as usize).max(5); // First frame: PUSH_PROMISE = 4-byte promised id + (head of) the header block. @@ -2205,7 +2206,6 @@ impl Connection { ); o += len; } - self.hpack.size_update_sent(); self.enc_buf = block; self.enc_buf.clear(); diff --git a/src/runtime/api/bun/h2/hpack.rs b/src/runtime/api/bun/h2/hpack.rs index 6a1e3b2e44ae..7c227a578619 100644 --- a/src/runtime/api/bun/h2/hpack.rs +++ b/src/runtime/api/bun/h2/hpack.rs @@ -65,7 +65,7 @@ impl Coder { } /// Call at the start of every outbound header block. The update stays pending until - /// [`Self::size_update_sent`], so a block that is built but never sent does not lose it. + /// [`Self::size_update_committed`], so a block that is built but never sent does not lose it. pub(crate) fn write_pending_size_update(&self, block: &mut Vec) { let Some(min) = self.unannounced_min else { return; @@ -76,7 +76,7 @@ impl Coder { write_table_size_update(block, self.enc_capacity); } - pub(crate) fn size_update_sent(&mut self) { + pub(crate) fn size_update_committed(&mut self) { self.unannounced_min = None; } diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index 1be9828a5a27..6e888d57378c 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -2000,7 +2000,7 @@ impl H2FrameParser { } /// Call before the first field of every outbound header block. Pair with - /// `header_block_sent`. + /// `commit_header_block`. fn begin_header_block(&self, encoded_headers: &mut Vec) { self.hpack.with_mut(|hpack| { if let Some(hpack) = hpack.as_mut() { @@ -2009,12 +2009,13 @@ impl H2FrameParser { }); } - /// Call after the block's frames are written. A block that is dropped before this keeps the - /// size update pending for the next one. - fn header_block_sent(&self) { + /// Call when nothing can drop the block any more, before its first frame byte is written. A + /// JS transport can deliver a peer SETTINGS frame during that write, and its size update + /// belongs to the next block. + fn commit_header_block(&self) { self.hpack.with_mut(|hpack| { if let Some(hpack) = hpack.as_mut() { - hpack.size_update_sent(); + hpack.size_update_committed(); } }); } @@ -5806,6 +5807,7 @@ impl H2FrameParser { bun_output::scoped_log!(H2FrameParser, "trailers encoded_size {}", encoded_size); + self.commit_header_block(); let mut writer = self.to_writer(); if encoded_size <= actual_max_frame_size { @@ -5859,7 +5861,6 @@ impl H2FrameParser { offset += chunk_size; } } - self.header_block_sent(); let identifier = stream.get_identifier(); identifier.ensure_still_alive(); if stream.state == StreamState::HALF_CLOSED_REMOTE { @@ -6214,6 +6215,7 @@ impl H2FrameParser { } } + self.commit_header_block(); let max_frame = self.remote_settings .get() @@ -6273,7 +6275,6 @@ impl H2FrameParser { offset += chunk; } } - self.header_block_sent(); let _ = self.flush(); Ok(JSValue::js_number(promised_id as f64)) @@ -7135,6 +7136,7 @@ impl H2FrameParser { } } + self.commit_header_block(); let frame = FrameHeader { type_: FrameType::HTTP_FRAME_HEADERS as u8, flags, @@ -7179,6 +7181,7 @@ impl H2FrameParser { let first_chunk_size = actual_max_frame_size - priority_overhead; let headers_flags = flags & !(HeadersFrameFlags::END_HEADERS as u8); + self.commit_header_block(); let headers_frame = FrameHeader { type_: FrameType::HTTP_FRAME_HEADERS as u8, flags: headers_flags @@ -7227,7 +7230,6 @@ impl H2FrameParser { offset += chunk_size; } } - self.header_block_sent(); if end_stream { stream.end_after_headers = true; diff --git a/test/js/node/http2/h2-conformance.test.ts b/test/js/node/http2/h2-conformance.test.ts index 8cfc29130e45..2a497b9c82cb 100644 --- a/test/js/node/http2/h2-conformance.test.ts +++ b/test/js/node/http2/h2-conformance.test.ts @@ -12,7 +12,7 @@ import { bunEnv, bunExe, gcTick, normalizeBunSnapshot, tls as tlsCert } from "ha import { EventEmitter, once } from "node:events"; import http2 from "node:http2"; import net from "node:net"; -import { Readable, Writable } from "node:stream"; +import { Duplex, Readable, Writable } from "node:stream"; import tls from "node:tls"; const PREFACE = Buffer.from("PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n", "latin1"); @@ -1483,6 +1483,115 @@ describe("SETTINGS_HEADER_TABLE_SIZE (RFC 7541 §4.2)", () => { raw.close(); } }); + + describe("client on a JS transport, where a large header block reaches the transport inside request()", () => { + // Larger than the 16 KB cork, so the block goes to the transport's write() before request() returns. + const oversized: Record = {}; + for (let i = 0; i < 90; i++) oversized[`x-big-${i}`] = Buffer.alloc(200, "v").toString() + i; + + /** A raw peer on a Duplex. `onHeaders` runs inside the client's write of that HEADERS frame. */ + function rawPeer(onHeaders: (frame: Frame, peer: Duplex) => void) { + let buf = Buffer.alloc(0); + let prefaceLeft = PREFACE.length; + const peer = new Duplex({ + read() {}, + write(chunk: Buffer, _encoding, callback) { + buf = Buffer.concat([buf, chunk]); + const skip = Math.min(prefaceLeft, buf.length); + prefaceLeft -= skip; + buf = buf.subarray(skip); + while (prefaceLeft === 0 && buf.length >= 9 && buf.length >= 9 + buf.readUIntBE(0, 3)) { + const length = buf.readUIntBE(0, 3); + const frame: Frame = { + length, + type: buf.readUInt8(3), + flags: buf.readUInt8(4), + streamId: buf.readUInt32BE(5) & 0x7fffffff, + payload: buf.subarray(9, 9 + length), + }; + buf = buf.subarray(9 + length); + if (frame.type === FrameType.SETTINGS && (frame.flags & 0x1) === 0) { + peer.push(encodeFrame(FrameType.SETTINGS, 0, 0)); + peer.push(encodeFrame(FrameType.SETTINGS, 0x1, 0)); + } else if (frame.type === FrameType.HEADERS) { + onHeaders(frame, peer); + // On a later tick: the client drops a response that arrives before request() returns. + setImmediate(() => peer.push(encodeFrame(FrameType.HEADERS, 0x5, frame.streamId, Buffer.from([0x88])))); + } + } + callback(); + }, + }); + return peer; + } + + test("a table size that arrives during that write is announced in the next block", async () => { + let insideRequest = false; + const seen: { insideRequest: boolean; frame: Frame }[] = []; + const peer = rawPeer((frame, peer) => { + seen.push({ insideRequest, frame }); + if (frame.streamId === 3) peer.push(encodeFrame(FrameType.SETTINGS, 0, 0, headerTableSizes(0))); + }); + const client = http2.connect("http://localhost", { createConnection: () => peer }); + client.on("error", () => {}); + const get = async (headers: http2.OutgoingHttpHeaders = {}) => { + insideRequest = true; + const req = client.request({ ":path": "/", ...headers }); + insideRequest = false; + req.on("error", () => {}); + req.resume(); + await once(req, "response"); + }; + try { + await get(); + await get(oversized); + await get(); + const [, second, third] = seen; + // Without this the test does not reach the case it is for. + expect({ streamId: second.frame.streamId, insideRequest: second.insideRequest }).toEqual({ + streamId: 3, + insideRequest: true, + }); + expect(headerBlock(third.frame)).toEqual({ sizeUpdates: [0], dynamicIndexes: [] }); + } finally { + client.destroy(); + } + }); + + test("a block that user code sends during that write does not repeat the size update", async () => { + let client: http2.ClientHttp2Session; + const seen: Frame[] = []; + const peer = rawPeer((frame, peer) => { + seen.push(frame); + // Lowered, then raised: the next block has to open with size updates to 0 and to 4096. + if (frame.streamId === 1) peer.push(encodeFrame(FrameType.SETTINGS, 0, 0, headerTableSizes(0, 4096))); + if (frame.streamId === 3) { + const nested = client.request({ ":path": "/nested" }); + nested.on("error", () => {}); + nested.resume(); + } + }); + client = http2.connect("http://localhost", { createConnection: () => peer }); + client.on("error", () => {}); + const get = async (headers: http2.OutgoingHttpHeaders = {}) => { + const req = client.request({ ":path": "/", ...headers }); + req.on("error", () => {}); + req.resume(); + await once(req, "response"); + }; + try { + await get(); + await get(oversized); + const [, outer, nested] = seen; + expect([outer.streamId, nested?.streamId]).toEqual([3, 5]); + expect(outer.payload.subarray(0, 4)).toEqual(Buffer.concat([sizeUpdate(0), sizeUpdate(4096)])); + // A second update to 0 makes the peer evict what the outer block has just inserted. + expect(headerBlock(nested)).toEqual({ sizeUpdates: [], dynamicIndexes: [] }); + } finally { + client.destroy(); + } + }); + }); }); describe("the local value bounds the decoder once the peer ACKs it", () => { From 0e0f362c90b805b36cb94516d1bd7e28d8603790 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 18 Sep 2026 20:48:10 +0000 Subject: [PATCH 5/5] node:http2: keep a size update that arrives while a header block is built User code runs between the fields of an outbound header block: header value coercion, getters, the options object. On a JS transport that code can deliver a peer SETTINGS frame. The new table size was recorded as pending, and then the block that was open consumed it, although the block was opened before the change and does not announce it. A block now consumes only what was pending when it was opened. A change that arrives later stays pending for the next block. The block that is open when the change arrives can still be inconsistent with the peer's table. That needs the header values to be read before the encoder starts. --- src/runtime/api/bun/h2/connection.rs | 8 +++-- src/runtime/api/bun/h2/hpack.rs | 30 +++++++++++----- src/runtime/api/bun/h2_frame_parser.rs | 32 +++++++++-------- test/js/node/http2/h2-conformance.test.ts | 43 +++++++++++++++++++++-- 4 files changed, 84 insertions(+), 29 deletions(-) diff --git a/src/runtime/api/bun/h2/connection.rs b/src/runtime/api/bun/h2/connection.rs index 92dce6f31f74..88b6d9abeec8 100644 --- a/src/runtime/api/bun/h2/connection.rs +++ b/src/runtime/api/bun/h2/connection.rs @@ -381,6 +381,7 @@ pub(crate) struct Connection { /// Scratch buffer for the outbound HPACK-encoded header block. enc_buf: Vec, + enc_announced: hpack::AnnouncedAt, /// Reusable scratch for end-of-batch window replenishment (stream id, increment). replenish_buf: Vec<(u32, u32)>, /// Reused buffer for evicting closed streams after each receive pass (no per-call allocation). @@ -424,6 +425,7 @@ impl Connection { sent_reset_limit: RateLimit::new(DEFAULT_STREAM_RESET_BURST, DEFAULT_STREAM_RESET_RATE), reset_flood: false, enc_buf: Vec::new(), + enc_announced: Default::default(), replenish_buf: Vec::new(), evict_buf: Vec::new(), preface_received: 0, @@ -2017,7 +2019,7 @@ impl Connection { /// Begin a new outbound header block. Emits any pending §6.3 dynamic-table size update first. pub(crate) fn begin_header_block(&mut self) { self.enc_buf.clear(); - self.hpack.write_pending_size_update(&mut self.enc_buf); + self.enc_announced = self.hpack.write_pending_size_update(&mut self.enc_buf); } /// HPACK-encode one header field into the current block. Returns false on encode failure. @@ -2043,7 +2045,7 @@ impl Connection { /// it exceeds the peer's max frame size (§4.3/§6.10), and advance the send-side stream state. pub(crate) fn send_header_block(&mut self, sink: &impl Sink, stream_id: u32, end_stream: bool) { let block = std::mem::take(&mut self.enc_buf); - self.hpack.size_update_committed(); + self.hpack.size_update_committed(self.enc_announced); let max = (self.remote_settings.max_frame_size as usize).max(1); let total = block.len(); @@ -2173,7 +2175,7 @@ impl Connection { /// promised request headers staged via begin_header_block/encode_header (RFC 9113 §6.6). pub(crate) fn send_push_promise(&mut self, sink: &impl Sink, parent_id: u32, promised_id: u32) { let block = std::mem::take(&mut self.enc_buf); - self.hpack.size_update_committed(); + self.hpack.size_update_committed(self.enc_announced); let max = (self.remote_settings.max_frame_size as usize).max(5); // First frame: PUSH_PROMISE = 4-byte promised id + (head of) the header block. diff --git a/src/runtime/api/bun/h2/hpack.rs b/src/runtime/api/bun/h2/hpack.rs index 7c227a578619..58b5f90f19ae 100644 --- a/src/runtime/api/bun/h2/hpack.rs +++ b/src/runtime/api/bun/h2/hpack.rs @@ -16,12 +16,18 @@ pub(crate) const DEFAULT_HEADER_TABLE_SIZE: u32 = 4096; /// `maxDeflateDynamicTableSize`. pub(crate) const MAX_ENCODER_TABLE_SIZE: u32 = 4096; +/// What a header block announced when it was opened. See [`Coder::size_update_committed`]. +#[derive(Clone, Copy, Default)] +pub(crate) struct AnnouncedAt(u32); + pub(crate) struct Coder { hpack: HpackHandle, enc_capacity: u32, /// `Some` while a capacity change is not announced: the smallest capacity since the last /// block that was sent. RFC 7541 §4.2 wants that minimum signaled before the final value. unannounced_min: Option, + /// Counts the changes of `unannounced_min`. + generation: u32, /// Our SETTINGS_HEADER_TABLE_SIZE that the peer ACKed last. dec_capacity: u32, } @@ -32,6 +38,7 @@ impl Coder { hpack: HpackHandle::new(DEFAULT_HEADER_TABLE_SIZE), enc_capacity: DEFAULT_HEADER_TABLE_SIZE, unannounced_min: None, + generation: 0, dec_capacity: DEFAULT_HEADER_TABLE_SIZE, } } @@ -62,22 +69,27 @@ impl Coder { Some(min) => min.min(capacity), None => capacity, }); + self.generation = self.generation.wrapping_add(1); } /// Call at the start of every outbound header block. The update stays pending until /// [`Self::size_update_committed`], so a block that is built but never sent does not lose it. - pub(crate) fn write_pending_size_update(&self, block: &mut Vec) { - let Some(min) = self.unannounced_min else { - return; - }; - if min < self.enc_capacity { - write_table_size_update(block, min); + pub(crate) fn write_pending_size_update(&self, block: &mut Vec) -> AnnouncedAt { + if let Some(min) = self.unannounced_min { + if min < self.enc_capacity { + write_table_size_update(block, min); + } + write_table_size_update(block, self.enc_capacity); } - write_table_size_update(block, self.enc_capacity); + AnnouncedAt(self.generation) } - pub(crate) fn size_update_committed(&mut self) { - self.unannounced_min = None; + /// User JS runs while a block is built and can deliver a peer SETTINGS frame. A change that + /// arrived after the block was opened is not in the block, so it stays pending. + pub(crate) fn size_update_committed(&mut self, announced: AnnouncedAt) { + if announced.0 == self.generation { + self.unannounced_min = None; + } } #[inline] diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index 6e888d57378c..253e65cdf6da 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -2001,21 +2001,23 @@ impl H2FrameParser { /// Call before the first field of every outbound header block. Pair with /// `commit_header_block`. - fn begin_header_block(&self, encoded_headers: &mut Vec) { - self.hpack.with_mut(|hpack| { - if let Some(hpack) = hpack.as_mut() { - hpack.write_pending_size_update(encoded_headers); - } - }); + fn begin_header_block( + &self, + encoded_headers: &mut Vec, + ) -> crate::api::h2::hpack::AnnouncedAt { + self.hpack.with_mut(|hpack| match hpack.as_mut() { + Some(hpack) => hpack.write_pending_size_update(encoded_headers), + None => Default::default(), + }) } /// Call when nothing can drop the block any more, before its first frame byte is written. A /// JS transport can deliver a peer SETTINGS frame during that write, and its size update /// belongs to the next block. - fn commit_header_block(&self) { + fn commit_header_block(&self, announced: crate::api::h2::hpack::AnnouncedAt) { self.hpack.with_mut(|hpack| { if let Some(hpack) = hpack.as_mut() { - hpack.size_update_committed(); + hpack.size_update_committed(announced); } }); } @@ -5595,7 +5597,7 @@ impl H2FrameParser { if encoded_headers.try_reserve(16384).is_err() { return Err(global_object.throw(format_args!("Failed to allocate header buffer"))); } - self.begin_header_block(&mut encoded_headers); + let announced = self.begin_header_block(&mut encoded_headers); // max header name length for lshpack let mut name_buffer = [0u8; 4096]; @@ -5807,7 +5809,7 @@ impl H2FrameParser { bun_output::scoped_log!(H2FrameParser, "trailers encoded_size {}", encoded_size); - self.commit_header_block(); + self.commit_header_block(announced); let mut writer = self.to_writer(); if encoded_size <= actual_max_frame_size { @@ -6059,7 +6061,7 @@ impl H2FrameParser { let mut name_buffer = [0u8; 4096]; let mut encoded_headers: Vec = Vec::new(); - self.begin_header_block(&mut encoded_headers); + let announced = self.begin_header_block(&mut encoded_headers); let mut single_value_headers = [false; SINGLE_VALUE_HEADERS_LEN]; // A PUSH_PROMISE carries a REQUEST, so request pseudo-headers are valid even on the server. @@ -6215,7 +6217,7 @@ impl H2FrameParser { } } - self.commit_header_block(); + self.commit_header_block(announced); let max_frame = self.remote_settings .get() @@ -6466,7 +6468,7 @@ impl H2FrameParser { if encoded_headers.try_reserve(16384).is_err() { return Err(global_object.throw(format_args!("Failed to allocate header buffer"))); } - self.begin_header_block(&mut encoded_headers); + let announced = self.begin_header_block(&mut encoded_headers); // max header name length for lshpack let mut name_buffer = [0u8; 4096]; let stream_id: u32 = if stream_id.is_number() { @@ -7136,7 +7138,7 @@ impl H2FrameParser { } } - self.commit_header_block(); + self.commit_header_block(announced); let frame = FrameHeader { type_: FrameType::HTTP_FRAME_HEADERS as u8, flags, @@ -7181,7 +7183,7 @@ impl H2FrameParser { let first_chunk_size = actual_max_frame_size - priority_overhead; let headers_flags = flags & !(HeadersFrameFlags::END_HEADERS as u8); - self.commit_header_block(); + self.commit_header_block(announced); let headers_frame = FrameHeader { type_: FrameType::HTTP_FRAME_HEADERS as u8, flags: headers_flags diff --git a/test/js/node/http2/h2-conformance.test.ts b/test/js/node/http2/h2-conformance.test.ts index 2a497b9c82cb..2526a4b7e89d 100644 --- a/test/js/node/http2/h2-conformance.test.ts +++ b/test/js/node/http2/h2-conformance.test.ts @@ -1490,7 +1490,7 @@ describe("SETTINGS_HEADER_TABLE_SIZE (RFC 7541 §4.2)", () => { for (let i = 0; i < 90; i++) oversized[`x-big-${i}`] = Buffer.alloc(200, "v").toString() + i; /** A raw peer on a Duplex. `onHeaders` runs inside the client's write of that HEADERS frame. */ - function rawPeer(onHeaders: (frame: Frame, peer: Duplex) => void) { + function rawPeer(onHeaders: (frame: Frame, peer: Duplex) => void, settings?: Buffer) { let buf = Buffer.alloc(0); let prefaceLeft = PREFACE.length; const peer = new Duplex({ @@ -1511,7 +1511,7 @@ describe("SETTINGS_HEADER_TABLE_SIZE (RFC 7541 §4.2)", () => { }; buf = buf.subarray(9 + length); if (frame.type === FrameType.SETTINGS && (frame.flags & 0x1) === 0) { - peer.push(encodeFrame(FrameType.SETTINGS, 0, 0)); + peer.push(encodeFrame(FrameType.SETTINGS, 0, 0, settings)); peer.push(encodeFrame(FrameType.SETTINGS, 0x1, 0)); } else if (frame.type === FrameType.HEADERS) { onHeaders(frame, peer); @@ -1591,6 +1591,45 @@ describe("SETTINGS_HEADER_TABLE_SIZE (RFC 7541 §4.2)", () => { client.destroy(); } }); + + // User code runs between the fields of a header block. This checks one thing: a size that + // arrives there is not lost. The block that is open at that moment can still be wrong, + // which is why its last field is the one that delivers the SETTINGS frame. + test("a table size that arrives while user code builds a block is announced in the next block", async () => { + let insideRequest = false; + const seen: Frame[] = []; + const peer = rawPeer(frame => seen.push(frame), headerTableSizes(0)); + const client = http2.connect("http://localhost", { createConnection: () => peer }); + client.on("error", () => {}); + const arrivedInsideRequest: boolean[] = []; + client.on("remoteSettings", settings => { + if (settings.headerTableSize === 4096) arrivedInsideRequest.push(insideRequest); + }); + const get = async (headers: http2.OutgoingHttpHeaders = {}) => { + insideRequest = true; + const req = client.request({ ":path": "/", ...headers }); + insideRequest = false; + req.on("error", () => {}); + req.resume(); + await once(req, "response"); + }; + try { + await get(); + const deliversSettings = { + toString() { + peer.push(encodeFrame(FrameType.SETTINGS, 0, 0, headerTableSizes(4096))); + return "value"; + }, + }; + await get({ "x-last": deliversSettings as unknown as string }); + await get(); + // Without this the test does not reach the case it is for. + expect(arrivedInsideRequest).toEqual([true]); + expect(seen.map(frame => headerBlock(frame).sizeUpdates)).toEqual([[0], [], [4096]]); + } finally { + client.destroy(); + } + }); }); });