From 418455822a23f80b6d69067821df778b5779d1da Mon Sep 17 00:00:00 2001 From: Zac Shenker Date: Sun, 18 Oct 2026 12:24:54 -0600 Subject: [PATCH] fix(quic): refuse a FIN below received stream data RFC 9000 section 4.5 makes a final size below data already received a FINAL_SIZE_ERROR. `Recv::ingest` checked a FIN only against a final size it already knew, so a first FIN below the high water mark was recorded: the reader got the bytes past it and then blocked forever, since the final size never matched what was read. Refuse it as `Recv::reset` already does for RESET_STREAM. Un-ignores `stream_final_size_below_received` and closes the m1/quic/final-size-below-received quest. Co-Authored-By: Claude Opus 5.5 --- quest/m1/quic/README.md | 1 - quest/m1/quic/final-size-below-received.md | 20 -------------------- rs/moq-quic/README.md | 2 +- rs/moq-quic/src/connection/streams/recv.rs | 6 ++++++ rs/moq-quic/src/tests/characterize/frames.rs | 3 --- 5 files changed, 7 insertions(+), 25 deletions(-) delete mode 100644 quest/m1/quic/final-size-below-received.md diff --git a/quest/m1/quic/README.md b/quest/m1/quic/README.md index 9231403feae..e9c9f81807a 100644 --- a/quest/m1/quic/README.md +++ b/quest/m1/quic/README.md @@ -70,7 +70,6 @@ cache limits remain separate; bitrate pacing composes with this memory cap. tail grace - [Schedule datagrams by subscription priority](/quest/m1/quic/datagram-priority.md) - datagrams join their subscription's send group instead of preempting every stream -- [A FIN below received data is a final size error](/quest/m1/quic/final-size-below-received.md) - a shrunken final size closes with FINAL_SIZE_ERROR instead of blocking the reader forever - [A stateless reset keeps a closed connection's reason](/quest/m1/quic/close-reason-after-reset.md) - a reset after a local transport error neither replaces the reason nor reports a second loss - [0-RTT streams adopt a raised stream window](/quest/m1/quic/zero-rtt-window-raise.md) - a server's larger stream window reaches the client's 0-RTT streams, so they no longer stall diff --git a/quest/m1/quic/final-size-below-received.md b/quest/m1/quic/final-size-below-received.md deleted file mode 100644 index 268e025765d..00000000000 --- a/quest/m1/quic/final-size-below-received.md +++ /dev/null @@ -1,20 +0,0 @@ -# [S] A FIN below received data is a final size error - -## Goal - -A STREAM frame whose FIN sets a final size below data already received closes -the connection with FINAL_SIZE_ERROR, as RFC 9000 4.5 requires. Today the -frame is accepted: the reader gets the bytes past the final size, then -`Blocked` forever, since the shrunken final size never matches what was -read. - -`stream_final_size_below_received` in -`rs/moq-quic/src/tests/characterize/frames.rs` must pass with its `#[ignore]` -removed. - -## Plan - -`Recv::ingest` in `rs/moq-quic/src/connection/streams/recv.rs` checks a FIN -only against an already-known final size. When a FIN arrives first, also -reject an `end` below `self.end`, before recording the size. RESET_STREAM -already makes this check (`reset_final_size_below_received`). diff --git a/rs/moq-quic/README.md b/rs/moq-quic/README.md index ba9ec36b6d0..d230d73b143 100644 --- a/rs/moq-quic/README.md +++ b/rs/moq-quic/README.md @@ -54,7 +54,7 @@ Changes on top of the upstream commit, besides the renames: - **BBR3 packet retirement**: per-space packet lookup and round-indexed expiry retire completed snapshots incrementally, preserving the extra ACK batch of metadata for late loss callbacks. The bounded indexes handle sparse packet numbers without walking the live flight. Upstream submission is deferred while quinn#2481 remains open: this fork also carries the packet-space and loss callback adaptations, so a port needs to target that implementation after its shape settles. - **Classic ECN for BBR3**, ported from [moq-dev/noq#12](https://github.com/moq-dev/noq/pull/12): CE exits Startup, stops a bandwidth probe, or lowers the short-term model, once per recovery episode, instead of counting as a loss. - **Pacer rounding**: a wait shorter than a nanosecond rounds up instead of re-arming the pacing timer at the current instant. -- **Malformed control input**: `RETIRE_CONNECTION_ID` for a sequence that was never issued (`issued` is a count, so `sequence >= issued`) and a repeated `grease_quic_bit` or `min_ack_delay` transport parameter close the connection. RFC 9000 section 7.4 only says SHOULD for a duplicate parameter; this fork refuses it, as it does every other known parameter. +- **Malformed control input**: `RETIRE_CONNECTION_ID` for a sequence that was never issued (`issued` is a count, so `sequence >= issued`) and a repeated `grease_quic_bit` or `min_ack_delay` transport parameter close the connection. RFC 9000 section 7.4 only says SHOULD for a duplicate parameter; this fork refuses it, as it does every other known parameter. A STREAM frame whose FIN sets a final size below data already received closes with `FINAL_SIZE_ERROR`, as RFC 9000 section 4.5 requires, where quinn accepts it and leaves the reader blocked forever. Not offered upstream yet. - **qlog pacing rate** in bits per second, as qlog defines it. - moq-dev/noq's congestion-callback regressions, reworked onto quinn's test harness. - [quinn#2724](https://github.com/quinn-rs/quinn/pull/2724) (`moq_sock::udp`): when the kernel rejects a GSO batch with `EIO` or `EINVAL`, the socket halts GSO and resends the batch as individual datagrams instead of dropping it. diff --git a/rs/moq-quic/src/connection/streams/recv.rs b/rs/moq-quic/src/connection/streams/recv.rs index 984043dbc9c..19779313e77 100644 --- a/rs/moq-quic/src/connection/streams/recv.rs +++ b/rs/moq-quic/src/connection/streams/recv.rs @@ -64,6 +64,12 @@ impl Recv { debug!(end, final_offset, "final size error"); return Err(TransportError::FINAL_SIZE_ERROR("")); } + // A first FIN can't end the stream below data already received (RFC 9000 section 4.5) + if frame.fin && end < self.end { + return Err(TransportError::FINAL_SIZE_ERROR( + "lower than high water mark", + )); + } let new_bytes = self.credit_consumed_by(end, received, max_data)?; diff --git a/rs/moq-quic/src/tests/characterize/frames.rs b/rs/moq-quic/src/tests/characterize/frames.rs index f2255a63261..a5e3e4952b3 100644 --- a/rs/moq-quic/src/tests/characterize/frames.rs +++ b/rs/moq-quic/src/tests/characterize/frames.rs @@ -367,10 +367,7 @@ fn stream_final_size_changed() { ); } -/// Today the reader gets all five bytes and then blocks forever: the shrunken final size never -/// matches what was read, so the stream neither finishes nor errors. #[test] -#[ignore = "bug: /quest/m1/quic/final-size-below-received.md"] fn stream_final_size_below_received() { let s = id(Side::Client, Dir::Uni, 0); let frames = Frames::default().stream(s, 0, b"abcde", false);