cloudflare / cloudflare/quiche
zero-length STREAM FIN can be dropped before it is sent
- Dominant language
- Rust
- Stars
- 11.8k
- Forks
- 1.1k
- Avg merge
- 21h 9m
- Merged PRs (30d)
- 6
Description
`stream_send(id, b"", true)` can set `fin_off` at an already ACKed data
offset, then the stream can be collected before any STREAM frame carrying
`FIN` is emitted.
The peer receives all bytes but never receives final size / EOF.
## Relevant code
`src/stream/send_buf.rs`
```rust
if fin {
self.fin_off = Some(max_off);
}
if self.ack_off() >= max_off {
return Ok(SendReserve {
inner: self,
reserved: 0,
fin,
});
}
```
```rust
pub fn is_complete(&self) -> bool {
if let Some(fin_off) = self.fin_off {
if self.acked == (0..fin_off) {
return true;
}
}
false
}
```
`src/lib.rs`
```rust
stream.send.ack_and_drop(offset, length)
```
The ACK path ignores whether the ACKed STREAM frame carried `FIN`.
## RFC 9000
Exact text:
- Section 3.1: "STREAM frame containing the FIN bit is sent"
- Section 3.1: "all stream data has been successfully acknowledged"
- Section 4.5: "A sender always communicates the final size"
- Section 13.3: "sending ceases when a packet containing that information is acknowledged"
- Section 19.8: "length of 0"
- Section 19.8: "offset of the next byte that would be sent"
For `STREAM offset=N length=0 FIN=true`, the FIN-bearing STREAM frame is the
packet information that communicates final size `N`. ACKing an earlier
data-only STREAM frame for `0..N` is not an ACK of that later FIN frame.
## Reproducer
Paste into `src/tests.rs`:
```rust
#[rstest]
fn stream_empty_fin_is_not_complete_until_fin_acked(
#[values("cubic", "bbr2_gcongestion")] cc_algorithm_name: &str,
) {
let mut buf = [0; 65535];
let mut pipe = test_utils::Pipe::new(cc_algorithm_name).unwrap();
assert_eq!(pipe.handshake(), Ok(()));
assert_eq!(pipe.client.stream_send(4, b"hello", true), Ok(5));
assert_eq!(pipe.advance(), Ok(()));
assert_eq!(pipe.server.stream_recv(4, &mut buf), Ok((5, true)));
assert_eq!(pipe.server.stream_send(4, b"response", false), Ok(8));
let flight = test_utils::emit_flight(&mut pipe.server).unwrap();
test_utils::process_flight(&mut pipe.client, flight).unwrap();
assert_eq!(pipe.client.stream_recv(4, &mut buf), Ok((8, false)));
assert_eq!(pipe.server.stream_send(4, b"", true), Ok(0));
let flight = test_utils::emit_flight(&mut pipe.client).unwrap();
test_utils::process_flight(&mut pipe.server, flight).unwrap();
assert!(!pipe.server.streams.is_collected(4));
let flight = test_utils::emit_flight(&mut pipe.server).unwrap();
let mut frames = Vec::new();
for (mut pkt, _) in flight {
frames.extend(
test_utils::decode_pkt(&mut pipe.client, &mut pkt).unwrap(),
);
}
assert!(frames.iter().any(|f| matches!(
f,
frame::Frame::Stream { stream_id: 4, data }
if data.off() == 8 && data.len() == 0 && data.fin()
)));
}
```
Result on `0.29.2`:
```text
assertion failed: !pipe.server.streams.is_collected(4)
```
## Fix suggestion
Track final-size delivery separately from byte-range ACKs. In particular,
an empty FIN should remain flushable/retransmittable until a FIN-bearing
STREAM frame at `fin_off` has been emitted and ACKed.
Contributor guide
Research direction
Start with `src/stream/send_buf.rs`, especially the FIN setup, completion check, and ACK handling shown in the issue; then inspect the ACK path in `src/lib.rs`. Add the reproducer to `src/tests.rs` and run it with both listed congestion-control algorithms. Done means the stream remains uncollected until the empty FIN at the final offset is emitted and acknowledged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 58/100