quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.55k stars 360 forks source link

ReadExactError::FinishedEarly byte count is sometimes incorrect #1861

Closed BiagioFesta closed 4 weeks ago

BiagioFesta commented 1 month ago

Maybe I am missing something, but:

Client

The client opens a unidirectional stream, it writes 5 bytes, and after 1 second idle it FIN the stream.

let mut stream = conn.open_uni().await?;
send.write_all(b"hello").await?;
tokio::time::sleep(Duration::from_secs(1)).await;
stream.finish()?;
stream.stopped().await;

Server

The server accepts the unidirectional stream, it tries to read 1024 bytes.

let mut buffer = [0; 1024];

let mut stream = connection.accept_uni().await?;
stream.read_exact(&mut buffer).await?;

Expected behavior:

On the server side, read_exact should fail with FinishedEarly(5), as it read the FIN bit after 5 bytes (without filling the entire buffer).

ERROR server: stream finished early (5 bytes read)

Current behavior (commit: 393b617ef1c76c7a6f0dad33905466fb266580bd)

On the server side, read_exact instead fails with FinishedEarly(0). Note the 0 bytes are read.

ERROR server: stream finished early (0 bytes read)

Additional Notes

It works as expected if tokio::time::sleep is removed.

Attached, a dirty-modified version of quinn/examples/(client|server).rs that I've used to reproduce the above scenario example_finish_early.txt

Ralith commented 1 month ago

Thanks for the report! This is probably a bug in the ReadExact::poll implementation that arises when the finish notification is delivered separately from other data, or perhaps whenever multiple operations are required. The math is a bit difficult to follow so it's not immediately obvious to me what the bug is, but that difficulty makes it highly suspect and indicates a need for revision regardless.

https://github.com/quinn-rs/quinn/blob/393b617ef1c76c7a6f0dad33905466fb266580bd/quinn/src/recv_stream.rs#L553-L567