[kixelated]

Finishes quest/m1/quic-zero-rtt-leftovers.md: moq-quic keeps only the code that recognizes and drops a 0-RTT packet, and that code now lives in the packet decoder.

Changes

• 0-RTT is skipped while decoding. PartialDecode::new skips 0-RTT packets and decodes the packet coalesced after them, so a coalesced datagram's later packets still decode. A datagram holding only 0-RTT packets returns the new PacketDecodeError::ZeroRtt. ProtectedHeader::decode returns Ok(None) for a 0-RTT packet, after moving the cursor past its payload. A 0-RTT packet longer than the datagram is still InvalidHeader.
• ZeroRtt moved out of LongType into LongHeaderType, so no decoded header can hold one. That deletes PartialDecode::is_0rtt(), the endpoint's leading-packet loop, the check in packet_crypto::unprotect_header, and the 0-RTT arm in Connection::process_decrypted_packet.
• PartialDecode::space() and Header::space() no longer map 0-RTT to the Data space.
• Kept the rustls early-data guards in QuicClientConfig::{with_initial, try_from} and QuicServerConfig::{with_initial, try_from}, and their test. rustls 0.23.46 leaves early data off by default, but the guards also normalize caller-supplied configs: rustls's QUIC ServerConnection::new rejects any max_early_data_size other than 0 or 0xffff_ffff (rustls-0.23.46/src/quic.rs:283), and start_session unwraps that result, so without the guard such a config panics on the first accepted connection. The Plan said to keep the guard and its test in that case. This PR first removed them, and Pullfrog's review thread found the panic, so cfb28a6 restored them. rs/moq-quic/src/crypto/rustls.rs, the README, and both MIGRATE.md files now match main.
• Tests:
  • New packet::tests::zero_rtt_alone_is_dropped covers a datagram of one or two 0-RTT packets, plus a truncated one.
  • New packet::tests::zero_rtt_coalesced_is_skipped covers 0-RTT before, between, and after Handshake and short packets.
  • zero_rtt_packets_are_dropped (end to end) sends a 0-RTT packet carrying an ACK for packet 1000, which the server never sent, so processing it would be a PROTOCOL_VIOLATION. The packet goes before and after a coalesced Initial, alone on the established connection, and before a 1-RTT packet. No case processes the ACK, and the 1-RTT STREAM frame is read. With LongType::ZeroRtt gone, the test builds a Handshake header and rewrites its type bits, which header protection doesn't cover.
• The packet fuzz target's invariant now matches the skip. The decoded packet must be the bytes just before the remainder, and any skipped prefix must start with a 0-RTT long header.
• Fixed the comment in moq-uring's version-negotiation test. A v1 0-RTT packet is dropped without a reply, so getting a reply proves the version is negotiated before the type is read.
• Deleted the quest and its link in quest/m1/README.md.
• Merged origin/main and resolved a README conflict: this PR's early-data line plus main's Metrics rename in the BBR3 line.
• Merged origin/main again at e25f626 (after moq-dev/moq#7028, moq-dev/moq#7650, and moq-dev/moq#7649 landed). No conflicts. This PR's added and removed lines are byte-identical to cfb28a6's, and the early-data guards survive refactor(quic)!: seal moq-quic's crypto to rustls #7028 in rs/moq-quic/src/crypto/rustls.rs.
• Removed Required from quest/m2/quic-long-type.md, plus its "until fix(moq-quic): delete the remaining 0-RTT code #7492 lands" line. Main planned that follow-up with this quest as its blocker, so deleting this quest left a dangling link that quest check flagged.

Public API: none. packet is a private module, so PacketDecodeError, ProtectedHeader, and LongType aren't exported. Wire: none.

Decisions

1. Keep is_0rtt(), or drop 0-RTT another way?

   • A: keep is_0rtt() as the only recognizer, and make the connection arm unreachable! (what this PR first did).
   • ✅ B (maintainer, 2026-11-01): PartialDecode::new skips 0-RTT packets itself, and ZeroRtt moves out of LongType into LongHeaderType, so no decoded header can hold one. That deletes is_0rtt(), the endpoint's leading-packet loop, the unprotect_header check, and the connection arm. Later packets in a coalesced datagram must still decode after a skipped 0-RTT packet, and tests must show a 0-RTT packet dropped both alone and coalesced.
   • C: inline the matches! at both drop sites. That needs a new accessor, which is is_0rtt() under another name.

   The quest file is deleted in this PR, so this answer is recorded only here.

Notes for review

• Caller-supplied rustls configs still have early data forced off, as on main, so the crypto::ServerConfig contract ("Sessions must reject early data", rs/moq-quic/src/crypto.rs:121) holds.
• The error for a datagram holding only 0-RTT is logged at trace level through the existing malformed header: {e} lines in the endpoint and in handle_coalesced. Before, the endpoint dropped it silently.
• One behavior change in the endpoint. The old loop dropped a datagram whose first packet was 0-RTT if any later packet failed to decode. Now a later packet's error comes back from PartialDecode::new. If that packet has an unsupported version and the datagram is at least 1200 bytes, the server sends Version Negotiation. That is also what happens when such a packet arrives first, so a peer can't get anything it couldn't already get. Real peers don't mix versions in one datagram.
• LongType now has a single variant, Handshake. Collapsing it is listed as a follow-up and was left out of this PR to keep the shape the decision asked for.

Checks

At 82fcbcf (main merged at e25f626):

• just rs check-test-changed over this PR's changed files (moq-quic, moq-uring, and their dependents): clippy, tests, fmt, doc, shear, sort, and hakari all passed. 6152 + 556 + 277 tests passed, including zero_rtt_alone_is_dropped, zero_rtt_coalesced_is_skipped, and zero_rtt_packets_are_dropped.
• quest check --base origin/main: 549 documents ok.

Review

Round 1 (new session after the merge): quest review --base origin/main, Codex gpt-6.1-sol at medium effort, head 82fcbcf. No findings.

Online: Pullfrog's only finding, on c26bbaa, was fixed in cfb28a6 (thread resolved). Pullfrog and the OpenAI review both reported no new issues at cfb28a6.

Follow-ups

• Collapsing the one-variant LongType is planned as quest/m2/quic-long-type.md, which this PR unblocks.
• moq-dev/moq#7490 will hit an adjacent-line conflict in rs/moq-quic/src/tests/token.rs:84-85 (poll_transmit → transmit next to ProtectedHeader::Initial → Some(ProtectedHeader::Initial)). Whichever lands second keeps both changes. Nothing this PR adds calls an API that moq-quic owns its waits #7490 renames.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 5 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9480f98b-5f92-414d-836b-f844cd0f009a

Commits

Reviewing files that changed from the base of the PR and between e25f626 and 82fcbcf.

Files selected for processing: 11

• quest/m1/README.md
• quest/m1/quic-zero-rtt-leftovers.md
• quest/m2/quic-long-type.md
• rs/moq-quic/src/connection/mod.rs
• rs/moq-quic/src/connection/packet_crypto.rs
• rs/moq-quic/src/endpoint.rs
• rs/moq-quic/src/fuzz.rs
• rs/moq-quic/src/packet.rs
• rs/moq-quic/src/tests/characterize/frames.rs
• rs/moq-quic/src/tests/token.rs
• rs/moq-uring/tests/integration/endpoint.rs

• 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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: c26bbaabf19469fa2c3c22603f59ee565d470a1b.

No actionable bugs found. Moving 0-RTT rejection into decoding is a sound simplification: decoded headers can no longer carry unsupported early data, while the skip loop preserves subsequent coalesced packets and checks payload bounds before advancing. The endpoint and connection paths consistently use that decoder. The rustls default-policy reliance is documented; caller-supplied configs must leave early data disabled.

Verification: static inspection of the decoder, routing/decryption paths, rustls adapters and upstream 0.23.46 defaults, plus the changed regression/fuzz coverage. I did not run tests, fuzzing, or builds.

(Written by gpt-6.1-sol)

[pullfrog]

Important

Removing the early-data normalization in QuicServerConfig::with_initial/try_from exposes a panic. rustls 0.23.46 only accepts max_early_data_size of 0 or 0xffff_ffff (rustls-0.23.46/src/quic.rs:283), and start_session unwraps that result at rs/moq-quic/src/crypto/rustls.rs:469. A caller-supplied rustls::ServerConfig with a normal early-data size (any other value) now panics on the first accepted connection; the deleted guard coerced it to 0, so this is a regression.

Reviewed changes

• 0-RTT dropped inside the decoder: PartialDecode::new skips leading 0-RTT packets and decodes the packet coalesced after them; a datagram of only 0-RTT returns the new PacketDecodeError::ZeroRtt; ProtectedHeader::decode returns Option and advances the cursor past a 0-RTT payload.
• ZeroRtt leaves the decoded types: moved from LongType into LongHeaderType, deleting PartialDecode::is_0rtt, the endpoint's leading-packet loop, the unprotect_header check, the process_decrypted_packet arm, and the 0-RTT-to-Data mapping in both space()s.
• rustls early-data guards removed: Quic{Client,Server}Config::{with_initial, try_from} no longer force early data off; the crate README, both MIGRATE.mds, and the rustls doc comments now tell callers to leave it off.
• Tests: new zero_rtt_alone_is_dropped / zero_rtt_coalesced_is_skipped, the characterize test rewritten to prove a 0-RTT ACK is dropped while a coalesced Initial/1-RTT is read, the fuzz packet invariant updated, and a moq-uring comment fixed.
• Quest deleted: quest/m1/quic-zero-rtt-leftovers.md and its README.md link.

The parser change itself holds: the skip advances exactly the 0-RTT packet's declared length, cannot loop or panic, and keeps data()/remainder partitioned with the input; the documented Version-Negotiation behavior change for a coalesced unsupported version is sound. The finding below is only in the rustls config path.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: cfb28a66511dfda6180edb8a3bad39bd3999b96c.

The independent caller-supplied rustls config finding is fixed. Restoring all four normalization guards prevents unsupported early-data sizes from reaching the unwrapped QUIC constructor and preserves the transport's early-data rejection contract. The adapter and its resumption test now match the base branch. My previous review missed that non-default-config panic; relying on defaults alone was insufficient.

No new actionable bugs found. Keeping the decoder simplification while retaining the existing TLS guards is the smaller, sound scope. The other update changes came from the base merge.

Verification: static comparison against the previous reviewed head and current base, including rustls 0.23.46 constructor validation. Tests and builds were not run.

(Written by gpt-6.1-sol)

[pullfrog]

✅ No new issues found.

Reviewed changes

Since pullfrog's prior review at c26bbaabf, the only change is that this PR reverted its rustls over-deletion and restored main exactly, leaving the decoder change as the sole behavioral delta.

• rustls early-data guards restored: Quic{Client,Server}Config::{with_initial, try_from} coerce enable_early_data/max_early_data_size off again, with supplied_configs_reject_early_data_and_keep_resumption and main's wording in the crate README and both MIGRATE.md files. This closes the accept-path panic the prior review flagged, and also stops the one rustls-accepted 0xffff_ffff value from leaving the TLS layer accepting early data the transport discards.
• Decoder change unchanged: the PartialDecode::new 0-RTT skip, the move of ZeroRtt into LongHeaderType, and the removed endpoint/connection/unprotect_header code are byte-identical to the version the prior review verified.

rs/moq-quic/src/crypto/rustls.rs, the crate README, and both MIGRATE.md files now diff empty against origin/main; the remaining 10 files are the reviewed decoder change plus the quest deletion.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}