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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/http/lshpack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,14 @@ impl HPACK {
lshpack_wrapper_enc_set_max_capacity(self, max_capacity as c_uint);
}

/// Adjust the decoder's maximum dynamic-table capacity after init: the
/// SETTINGS_HEADER_TABLE_SIZE we advertised, once the peer has ACKed it.
/// Bounds the value a peer's RFC 7541 §6.3 Dynamic Table Size Update may
/// carry; a shrink evicts entries in lockstep with the peer's encoder.
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`.
Expand Down Expand Up @@ -212,6 +220,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);
Expand Down
5 changes: 5 additions & 0 deletions src/jsc/bindings/c-bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,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);
Expand Down
35 changes: 21 additions & 14 deletions src/runtime/api/bun/h2/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,10 @@ pub struct Connection {
/// The initial window size the peer has ACKed (RFC 9113 6.5.3): until our SETTINGS is ACKed,
/// the peer may legitimately send according to the previous value - enforce the larger.
acked_local_initial_window: u32,
/// INITIAL_WINDOW_SIZE values of sent-but-unACKed SETTINGS frames, in send order. §6.5.3:
/// ACKs apply to outstanding SETTINGS in order, so each inbound ACK pops the front - the
/// value the peer actually acknowledged - rather than assuming the latest submission.
pub pending_local_window_acks: std::collections::VecDeque<u32>,
/// Snapshots of sent-but-unACKed SETTINGS frames, in send order. §6.5.3: ACKs apply to
/// outstanding SETTINGS in order, so each inbound ACK pops the front - the values the peer
/// actually acknowledged - rather than assuming the latest submission.
pub pending_local_settings_acks: std::collections::VecDeque<Settings>,
invalid_frame_count: u32,
/// Maximum number of entries accepted in a single inbound SETTINGS frame (node's maxSettings
/// session option, nghttp2's max_settings; not a SETTINGS parameter). Frames carrying more
Expand Down Expand Up @@ -224,7 +224,7 @@ impl Connection {
max_header_list_pairs: 128,
max_invalid_frames: 1000,
acked_local_initial_window: 65_535,
pending_local_window_acks: std::collections::VecDeque::new(),
pending_local_settings_acks: std::collections::VecDeque::new(),
invalid_frame_count: 0,
max_settings: 32,
send_window: SendWindow::new(wire::DEFAULT_WINDOW_SIZE),
Expand Down Expand Up @@ -281,8 +281,8 @@ impl Connection {
pub fn send_settings(&mut self, sink: &impl Sink) {
let mut buf = [0u8; Settings::STANDARD_COUNT * 6];
let n = self.local_settings.pack_standard(&mut buf);
self.pending_local_window_acks
.push_back(self.local_settings.initial_window_size);
self.pending_local_settings_acks
.push_back(self.local_settings);
self.write_frame(sink, FrameType::Settings, 0, 0, &buf[..n]);
}

Expand Down Expand Up @@ -563,14 +563,21 @@ impl Connection {
return true;
}
self.local_settings_acked = true;
// §6.5.3: this ACK acknowledges the oldest outstanding SETTINGS, whose window size
// may differ from the latest submission when several SETTINGS are in flight.
self.acked_local_initial_window = self
.pending_local_window_acks
// §6.5.3: this ACK acknowledges the oldest outstanding SETTINGS, whose values may
// differ from the latest submission when several SETTINGS are in flight.
let acked = self
.pending_local_settings_acks
.pop_front()
.unwrap_or(self.local_settings.initial_window_size);
let snapshot = self.local_settings;
sink.on_local_settings(&snapshot);
.unwrap_or(self.local_settings);
self.acked_local_initial_window = acked.initial_window_size;
// RFC 7541 §6.3: the limit on the peer's Dynamic Table Size Update is the
// SETTINGS_HEADER_TABLE_SIZE the peer has ACKed. Resize OUR decoder to match. (The
// outbound encoder is the mirror: the embedder applies the PEER's header table size
// to it from Sink::on_remote_settings.)
self.hpack.set_decoder_capacity(acked.header_table_size);
// The `localSettings` event / `session.localSettings` must report the values the peer
// just acknowledged (what nghttp2's local_settings holds), not the latest submission.
sink.on_local_settings(&acked);
return false;
}
// node's maxSettings (nghttp2 max_settings): refuse SETTINGS frames carrying more entries
Expand Down
49 changes: 44 additions & 5 deletions src/runtime/api/bun/h2/hpack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,35 @@

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 const MAX_SIZE_UPDATE_BYTES: usize = 6;
/// RFC 7541 §4.2: a header block opens with at most two §6.3 Dynamic Table Size Updates (the
/// interval minimum, then the final value). A §5.1 5-bit-prefix integer for a u32 never needs more
/// than 6 bytes, so this bounds what [`Coder::take_pending_size_update`] can write.
pub const MAX_SIZE_UPDATE_BYTES: usize = 12;

pub struct Coder {
hpack: HpackHandle,
enc_capacity: u32,
/// The maximum the decoder accepts in a §6.3 Dynamic Table Size Update: the
/// SETTINGS_HEADER_TABLE_SIZE we last advertised and the peer has ACKed (§6.5.3).
dec_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<u32>,
Comment thread
claude[bot] marked this conversation as resolved.
/// RFC 7541 §4.2: the SMALLEST capacity the peer requested since the last header block. The
/// peer's decoder evicts eagerly on each of its SETTINGS, so a transient minimum below the
/// final value must still be applied and signaled (as the first of two size updates) or the
/// two tables diverge. Only meaningful while `pending_enc_capacity` is `Some`.
pending_enc_min: u32,
}

impl Coder {
pub fn new(max_capacity: u32) -> Self {
Coder {
hpack: HpackHandle::new(max_capacity),
enc_capacity: max_capacity,
dec_capacity: max_capacity,
pending_enc_capacity: None,
pending_enc_min: max_capacity,
}
}

Expand All @@ -36,18 +48,45 @@ impl Coder {
if capacity == self.enc_capacity && self.pending_enc_capacity.is_none() {
return;
}
self.pending_enc_min = match self.pending_enc_capacity {
Some(_) => self.pending_enc_min.min(capacity),
None => capacity,
};
self.pending_enc_capacity = Some(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.
/// Apply OUR SETTINGS_HEADER_TABLE_SIZE to the decoder once the peer has ACKed it. The value
/// we advertise bounds the peer encoder's §6.3 Dynamic Table Size Update (RFC 7541 §6.3), so a
/// decoder left at its construction-time capacity rejects a conforming peer that grows up to
/// the new max. No-op when unchanged: re-applying would reset the table size the peer's
/// encoder last chose via a size update.
pub fn set_decoder_capacity(&mut self, capacity: u32) {
if capacity == self.dec_capacity {
return;
}
self.hpack.set_decoder_max_capacity(capacity);
self.dec_capacity = capacity;
}

/// If a capacity change is pending, apply it and write the §6.3 size-update opcode(s) into
/// `dst` at `offset`. Returns bytes written (0 if none). `dst[offset..]` needs >=
/// MAX_SIZE_UPDATE_BYTES.
pub fn take_pending_size_update(&mut self, dst: &mut [u8], offset: usize) -> usize {
let Some(cap) = self.pending_enc_capacity.take() else {
return 0;
};
let mut n = 0;
// §4.2: when the interval's minimum is below its final value, the minimum MUST be signaled
// first (at most two size updates per block). Applying it evicts our table exactly as the
// peer's decoder already did at that SETTINGS, keeping the two tables in lockstep.
let min = self.pending_enc_min;
if min < cap {
self.hpack.set_encoder_max_capacity(min);
n += write_table_size_update(dst, offset, min);
}
self.hpack.set_encoder_max_capacity(cap);
self.enc_capacity = cap;
write_table_size_update(dst, offset, cap)
n + write_table_size_update(dst, offset + n, cap)
}

#[inline]
Expand Down
65 changes: 44 additions & 21 deletions src/runtime/api/bun/h2_frame_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1319,9 +1319,9 @@
/// Same bridge per stream: (stream id, bytes) pairs drained into the engine's per-stream send
/// windows in rewrite_read.
pending_stream_send_consumed: JsCell<Vec<(u32, u64)>>,
/// INITIAL_WINDOW_SIZE of each SETTINGS frame the legacy encoder sent, in send order, drained
/// into the engine's per-SETTINGS ack queue in rewrite_read (§6.5.3: ACKs apply in order).
pending_settings_window_submissions: JsCell<Vec<u32>>,
/// Snapshot of each SETTINGS frame the legacy encoder sent, in send order, drained into the
/// engine's per-SETTINGS ack queue in rewrite_read (§6.5.3: ACKs apply in order).
pending_settings_submissions: JsCell<Vec<crate::api::h2::settings::Settings>>,
/// Stream ids whose legacy-side lifecycle finished while a dispatch held the engine
/// borrow (the normal request path: receive() -> JS handler -> respond -> END_STREAM).
/// Drained into Connection::close_stream on the next rewrite_read batch.
Expand Down Expand Up @@ -1357,7 +1357,9 @@

streams: JsCell<BunHashMap<u32, *mut Stream>>,

hpack: JsCell<Option<lshpack::HpackHandle>>,
/// Outbound HPACK encoder (the engine in `engine` owns the inbound decoder). `Coder` carries
/// the pending RFC 7541 §6.3 size update queued by a peer SETTINGS_HEADER_TABLE_SIZE change.
hpack: JsCell<Option<crate::api::h2::hpack::Coder>>,

has_nonnative_backpressure: Cell<bool>,
ref_count: bun_ptr::RefCount<Self>, // intrusive — bun.ptr.RefCount(@This(), "ref_count", deinit, .{})
Expand Down Expand Up @@ -2227,6 +2229,18 @@
value: &[u8],
never_index: bool,
) -> Result<usize, bun_core::Error> {
// Every outbound header block (request/respond, trailers, push promise) funnels its fields
// through here into a fresh, initially empty buffer, so the first field of a block is
// where RFC 7541 §6.3 requires a pending Dynamic Table Size Update to be announced.
if encoded_headers.is_empty() {
let mut opcode = [0u8; crate::api::h2::hpack::MAX_SIZE_UPDATE_BYTES];
let n = self.hpack.with_mut(|hpack| {
hpack
.as_mut()
.map_or(0, |hpack| hpack.take_pending_size_update(&mut opcode, 0))
});
encoded_headers.extend_from_slice(&opcode[..n]);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
let old_len = encoded_headers.len();
let required = old_len + name.len() + value.len() + HPACK_ENTRY_OVERHEAD;
// Note: materializing `&mut [u8]` over uninitialized capacity is UB and
Expand Down Expand Up @@ -2399,10 +2413,10 @@
.set(self.outstanding_settings.get() + 1);

self.local_settings.set(settings);
// Remember which INITIAL_WINDOW_SIZE this submission carries so the engine can attribute
// the peer's ACK to it (§6.5.3 - ACKs apply to outstanding SETTINGS in order).
self.pending_settings_window_submissions
.with_mut(|v| v.push(settings.initial_window_size));
// Remember the values this submission carries so the engine can attribute the peer's ACK
// to it (§6.5.3 - ACKs apply to outstanding SETTINGS in order).
self.pending_settings_submissions
.with_mut(|v| v.push(self.rewrite_local_settings()));
let _ = self.local_settings.get().write(&mut stream);
let _ = self.write(&buffer);
true
Expand Down Expand Up @@ -2633,8 +2647,8 @@
};
self.outstanding_settings
.set(self.outstanding_settings.get() + 1);
self.pending_settings_window_submissions
.with_mut(|v| v.push(self.local_settings.get().initial_window_size));
self.pending_settings_submissions
.with_mut(|v| v.push(self.rewrite_local_settings()));
let _ = settings_header.write(&mut preface_stream);
let _ = self.local_settings.get().write(&mut preface_stream);
let _ = self.write(&preface_buffer);
Expand Down Expand Up @@ -5433,10 +5447,10 @@
});
});
// Register SETTINGS submissions the legacy encoder sent since the last batch, so the
// engine attributes each inbound ACK to the right INITIAL_WINDOW_SIZE (§6.5.3).
self.pending_settings_window_submissions.with_mut(|v| {
for w in v.drain(..) {
engine.pending_local_window_acks.push_back(w);
// engine attributes each inbound ACK to the right submission's values (§6.5.3).
self.pending_settings_submissions.with_mut(|v| {
for s in v.drain(..) {
engine.pending_local_settings_acks.push_back(s);
}
});
// Streams whose legacy lifecycle finished since the last batch: evict the engine
Expand Down Expand Up @@ -5601,6 +5615,15 @@
..Default::default()
};
self.remote_settings.set(Some(fp));
// RFC 7541 §4.2: the peer's HEADER_TABLE_SIZE bounds OUR encoder's dynamic table. Queue a
// §6.3 size update for the start of the next outbound header block; a peer that lowered
// the limit (nginx advertises 0 to every upstream) rejects blocks that keep referencing
// the old table. queue_encoder_capacity is a no-op when the value is unchanged.
self.hpack.with_mut(|hpack| {
if let Some(hpack) = hpack.as_mut() {
hpack.queue_encoder_capacity(settings.header_table_size);
}
});

Check warning on line 5626 in src/runtime/api/bun/h2_frame_parser.rs

View check run for this annotation

Claude / Claude Code Review

Single-frame duplicate HEADER_TABLE_SIZE entries bypass §4.2 interval-minimum handling

Nit: the `pending_enc_min` interval-minimum tracking closes the RFC 7541 §4.2 gap for two *separate* SETTINGS frames (`[HTS=0]` then `[HTS=4096]`), but not for a *single* SETTINGS frame carrying duplicate entries (`[HTS=0, HTS=4096]`) — `handle_settings` applies entries cumulatively then invokes `on_remote_settings` once with the final snapshot (4096), so `queue_encoder_capacity(4096)` early-returns and the transient 0 is never seen. Exotic (no mainstream client emits duplicate HTS in one frame;
Comment thread
robobun marked this conversation as resolved.
// §6.9.2 (mirrors the legacy inbound): when the peer's INITIAL_WINDOW_SIZE grows, raise the
// send window of streams opened before its SETTINGS arrived (a client's first request is
// typically sent before the server's SETTINGS lands), then resume queued sends.
Expand Down Expand Up @@ -9219,7 +9242,7 @@
pending_stream_send_consumed: JsCell::new(Vec::new()),
pending_engine_stream_closes: JsCell::new(Vec::new()),
dispatch_depth: Cell::new(0),
pending_settings_window_submissions: JsCell::new(Vec::new()),
pending_settings_submissions: JsCell::new(Vec::new()),
max_rejected_streams: Cell::new(100),
max_session_invalid_frames: Cell::new(1000),
max_outstanding_settings: Cell::new(10),
Expand Down Expand Up @@ -9387,12 +9410,12 @@
.strong_this
.with_mut(|s| s.set_strong(this_value, global_object));

// 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,
// RFC 7541 §4.2: the outbound encoder is bounded by the PEER's SETTINGS_HEADER_TABLE_SIZE,
// which starts at the 4096 default until its SETTINGS arrives (our own headerTableSize
// governs the inbound decoder, owned by the engine). `Coder` owns an `HpackHandle` whose
// teardown runs `lshpack_wrapper_deinit` (cleanup + free), not a bare `mi_free`.
this_ref.hpack.set(Some(crate::api::h2::hpack::Coder::new(
crate::api::h2::wire::DEFAULT_HEADER_TABLE_SIZE,
)));
if is_server {
let _ = this_ref.set_settings(this_ref.local_settings.get());
Expand Down
Loading
Loading