From f4944a7cc248d842bbbe1d34cc3fe9049b6acdc0 Mon Sep 17 00:00:00 2001 From: Randell Jesup Date: Thu, 26 Feb 2026 13:40:38 -0500 Subject: [PATCH 1/2] subtract out some overheads from the max datagram size --- neqo-http3/src/connection_client.rs | 12 ++++++++++-- .../extended_connect/tests/webtransport/datagrams.rs | 8 ++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/neqo-http3/src/connection_client.rs b/neqo-http3/src/connection_client.rs index d76c5b17ac..a3d3b5365d 100644 --- a/neqo-http3/src/connection_client.rs +++ b/neqo-http3/src/connection_client.rs @@ -894,9 +894,17 @@ impl Http3Client { /// /// This cannot panic. The max varint length is 8. pub fn webtransport_max_datagram_size(&self, session_id: StreamId) -> Res { + let session_id_len = u64::try_from(Encoder::varint_len(session_id.as_u64())) + .map_err(|_| Error::Internal)?; + // Reserve space for ACK frames and packet number growth beyond what is + // seen at session start (when max_datagram_size is sampled). A worst-case + // ACK frame is ~50 bytes (RecvdPackets::USEFUL_ACK_LEN) and packet number + // varints can grow by up to 3 bytes over the life of a connection, giving + // ~53 bytes of realistic overhead; 64 provides a small margin. + const DATAGRAM_OVERHEAD: u64 = 64; Ok(self.conn.max_datagram_size()? - - u64::try_from(Encoder::varint_len(session_id.as_u64())) - .map_err(|_| Error::Internal)?) + .saturating_sub(session_id_len) + .saturating_sub(DATAGRAM_OVERHEAD)) } pub fn webtransport_set_datagram_high_water_mark( diff --git a/neqo-http3/src/features/extended_connect/tests/webtransport/datagrams.rs b/neqo-http3/src/features/extended_connect/tests/webtransport/datagrams.rs index 6f8e8ee552..6a92221e7c 100644 --- a/neqo-http3/src/features/extended_connect/tests/webtransport/datagrams.rs +++ b/neqo-http3/src/features/extended_connect/tests/webtransport/datagrams.rs @@ -16,6 +16,8 @@ use crate::{ }; const DGRAM: &[u8] = &[0, 100]; +// Overhead subtracted by webtransport_max_datagram_size on the client side. +const CLIENT_DATAGRAM_OVERHEAD: u64 = 64; #[test] fn no_datagrams() { @@ -63,7 +65,8 @@ fn do_datagram_test(wt: &mut WtTest, wt_session: &WebTransportRequest) { assert_eq!( wt.max_datagram_size(wt_session.stream_id()), Ok(DATAGRAM_SIZE - - u64::try_from(Encoder::varint_len(wt_session.stream_id().as_u64())).unwrap()) + - u64::try_from(Encoder::varint_len(wt_session.stream_id().as_u64())).unwrap() + - CLIENT_DATAGRAM_OVERHEAD) ); assert_eq!(wt_session.send_datagram(DGRAM, None, now(), 0, 0), Ok(())); @@ -99,7 +102,8 @@ fn datagrams_server_only() { assert_eq!( wt.max_datagram_size(wt_session.stream_id()), Ok(DATAGRAM_SIZE - - u64::try_from(Encoder::varint_len(wt_session.stream_id().as_u64())).unwrap()) + - u64::try_from(Encoder::varint_len(wt_session.stream_id().as_u64())).unwrap() + - CLIENT_DATAGRAM_OVERHEAD) ); assert_eq!( From b5a260ea3647d822a38d06f2eaf0e6954300c1b4 Mon Sep 17 00:00:00 2001 From: Randell Jesup Date: Sun, 15 Mar 2026 22:09:06 -0400 Subject: [PATCH 2/2] remove debug_assert that could cause debug builds to fail in a race condition, plus one that should never happen --- neqo-http3/src/features/extended_connect/session.rs | 1 - .../src/features/extended_connect/webtransport_session.rs | 8 +++----- neqo-http3/tests/connect_udp.rs | 1 - 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/neqo-http3/src/features/extended_connect/session.rs b/neqo-http3/src/features/extended_connect/session.rs index 03b999159c..44b009d65a 100644 --- a/neqo-http3/src/features/extended_connect/session.rs +++ b/neqo-http3/src/features/extended_connect/session.rs @@ -444,7 +444,6 @@ impl Session { qtrace!("[{self}] send_datagram state={:?}", self.state); if self.state != State::Active { qdebug!("[{self}]: cannot send datagram in {:?} state.", self.state); - debug_assert!(false); return Err(Error::Unavailable); } if conn.remote_datagram_size() == 0 && self.protocol.datagram_capsule_support() { diff --git a/neqo-http3/src/features/extended_connect/webtransport_session.rs b/neqo-http3/src/features/extended_connect/webtransport_session.rs index 5be93db06c..173512e173 100644 --- a/neqo-http3/src/features/extended_connect/webtransport_session.rs +++ b/neqo-http3/src/features/extended_connect/webtransport_session.rs @@ -419,11 +419,9 @@ impl Protocol for Session { _buf: &[u8], _now: Instant, ) -> Res<()> { - debug_assert!( - false, - "[{self}] WebTransport does not support datagram capsules." - ); - Ok(()) + // WebTransport uses QUIC datagrams, not HTTP DATAGRAM Capsules. + // datagram_capsule_support() returns false so this should never be called. + Err(Error::Unavailable) } fn set_datagram_high_water_mark(&mut self, mark: f64) { diff --git a/neqo-http3/tests/connect_udp.rs b/neqo-http3/tests/connect_udp.rs index 860c7a36ec..cdf99f5d1b 100644 --- a/neqo-http3/tests/connect_udp.rs +++ b/neqo-http3/tests/connect_udp.rs @@ -357,7 +357,6 @@ fn connect_via_proxy() { } #[test] -#[cfg_attr(debug_assertions, should_panic(expected = "assertion failed: false"))] fn send_dgram_on_non_active_session() { let (mut client, _proxy, connect_udp_session_id) = initiate_new_session();