Skip to content

add a failing-behaviour test for the quic framing desync in issue 143 - #144

Closed
fabracht wants to merge 2 commits into
mainfrom
repro-quic-framing
Closed

fabracht wants to merge 2 commits into
mainfrom
repro-quic-framing

Conversation

@fabracht

Copy link
Copy Markdown
Contributor

Summary

Demonstrates the framing corruption reported in #143 against real QUIC, so the bug is reproducible rather than argued from the source. No production code changes and no fix.

  • drives two real quinn endpoints over the repo's test certificates
  • writes a length prefix, then cancels write_all mid-payload with a 2 ms timeout, exactly as send_to_peer does
  • writes a second complete frame afterwards, as the transport would on the next send
  • the reader receives the abandoned frame's length prefix and then never completes it: it waits for bytes that will never arrive, having swallowed the following frame as payload

This is a characterization test — it passes because the bug is present. When #143 is fixed its assertion should invert to "the stream stays synchronized", at which point it becomes the regression test for the fix.

No version bump or changelog entry: the crate's behaviour is unchanged.

Test plan

  • cargo make clippy passes with zero warnings
  • cargo make test: 24 suites, 1201 passed, 0 failed
  • the new test passes on 3 consecutive isolated runs (~3s each), and in the full suite
  • the cancellation itself is asserted, so the test fails loudly rather than passing vacuously if the write ever completes within the timeout

@fabracht

Copy link
Copy Markdown
Contributor Author

Closing without merging.

Review found the test exercises the wrong layer: it drives raw quinn::SendStream::write_all on an open_bi() stream and never calls send_to_peer. It therefore characterizes a quinn property — write_all is not cancellation-safe — rather than MQDB's behaviour. Every fix proposed in #143 changes send_to_peer or the peer map, none change quinn, so this test would still pass unchanged after the fix. The claim in the description that its assertion "should invert" once #143 is fixed is wrong.

Further problems, for the record:

  • framed.is_ok() means "did not time out", not "the frame completed"; a stream reset would make it report the opposite of what happened.
  • The assertion message says the next frame was swallowed as payload, but nothing checks that — removing the second frame's writes leaves the result unchanged.
  • The cancellation depends on wall-clock throughput, because the reader drains continuously, rather than on flow control.
  • It added two #[allow(clippy::cast_possible_truncation)] where u32::try_from suffices.
  • The description still says it uses the repo's test certificates, which it stopped doing after the first CI run failed on gitignored test_certs/.

Running it was still useful: it confirmed the mechanism in #143 empirically, and it showed that a real reproduction through send_to_peer is not possible today because receiver_task always drains and drops on a full inbox. That is now an argument for the writer-task fix, recorded in the implementation brief on #143, which asks for the fix to land together with an MQDB-level test.

@fabracht fabracht closed this Sep 21, 2026
@fabracht
fabracht deleted the repro-quic-framing branch September 21, 2026 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant