Repository navigation
fix(quic): refuse a FIN below received stream data - #5906
Conversation
RFC 9000 section 4.5 makes a final size below data already received a FINAL_SIZE_ERROR. `Recv::ingest` checked a FIN only against a final size it already knew, so a first FIN below the high water mark was recorded: the reader got the bytes past it and then blocked forever, since the final size never matched what was read. Refuse it as `Recv::reset` already does for RESET_STREAM. Un-ignores `stream_final_size_below_received` and closes the m1/quic/final-size-below-received quest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 … |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description check | |
The description clearly explains the RFC requirement, the defect, the implementation, the regression test, and the impact. It directly matches the changeset. |
| Title check | |
The title is concise, specific, and accurately identifies the primary change: rejecting a FIN below received stream data. |
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches
-
✨ Simplify code
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
…elow-received Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
RFC 9000 section 4.5 requires FINAL_SIZE_ERROR when a STREAM frame's FIN sets a final size below data already received.
Recv::ingestonly checked a FIN against a final size it already knew, so a first FIN below the high water mark was accepted: the reader got the bytes past it and then blocked forever, because the final size never matched what was read. A peer can stall a reader this way. Tracked by the quest m1/quic/final-size-below-received.Approach
Recv::ingestrefuses a FIN whose end is below the stream's high water mark, the checkRecv::resetalready makes for RESET_STREAM.stream_final_size_below_receivedis no longer ignored.Impact
Alternatives
None: the RFC requires this error.
Decisions (iterate round 1)
Made unattended with the recommended option:
match self.final_offset(). The fold reads slightly cleaner but rewrites upstream lines, which makes a future quinn sync or an upstream offer harder.left: None, right: Some(FINAL_SIZE_ERROR)) and passes with it; the harness drives a simulated-timePair, so no wall clock.Verification
cargo test -p moq-quic --lib final_size: 8 pass; the new test fails with the check disabled.just check: pass.Follow-ups
Recv::ingest, so it doesn't conflict with feat(quic)!: bound incomplete messages with receive leases #5878's receive-lease changes in the same file.(Written by Claude Opus 5.5)