quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.85k stars 394 forks source link

Yield protocol violation for packets without frames #1693

Closed djc closed 1 year ago

djc commented 1 year ago

Received a private bug report from @QUICTester. Thanks!

This should probably have a test, which I don't have energy for right now. @Ralith any suggestion for where such a test should live?

I don't love that this equates "no frames" to "empty payload" which feels like a potential conceptual jump, but the for-loop formulation makes the alternative a decent bit more verbose...

Ralith commented 1 year ago

any suggestion for where such a test should live?

If we move this check into frame::Iter::new it becomes easy to test in isolation, but also so trivial that we might not bother.

I don't love that this equates "no frames" to "empty payload" which feels like a potential conceptual jump

Aren't these equivalent? If payload data isn't a well-formed frame, that's an error too.

djc commented 1 year ago

If we move this check into frame::Iter::new it becomes easy to test in isolation, but also so trivial that we might not bother.

Yup, that's better -- done.

Aren't these equivalent? If payload data isn't a well-formed frame, that's an error too.

Yup, I do think they're equivalent.