search

quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.76k stars 380 forks source link

quinn-proto: `Endpoint::connect` retrieves timestamp from OS #1648

Closed omegablitz closed 1 year ago

omegablitz commented 1 year ago

While debugging an issue I was running into, I found that quinn_proto::Endpoint::connect fetches a timestamp from the OS: https://github.com/quinn-rs/quinn/blob/7f260292848a93d615eb43e6e88114a97e64daf1/quinn-proto/src/endpoint.rs#L355

I would have expected Endpoint::connect to take in a now: Instant parameter instead. I got that impression from the crate docs.

Is this intended behavior?

Thanks for your help!

djc commented 1 year ago

Nope, I think this is an oversight -- thanks for reporting! I checked for Instant::now calls in quinn-proto and found that this was the only one that doesn't uphold the documented contract. There is one other one in the BBR bandwidth estimator, but it has a comment saying that the value will be overwritten before it is used -- all the remaining instances are in test code.

Fix submitted in #1649.