Repository navigation
fix(moq-mux): start the TS step look-ahead at the packet that showed the step - #7683
Conversation
Quest-Claim: 2a28196e5f5001c1
…the step `Steps::judge` handed `trigger` the demux cursor from before `peek`. When the framer resynced past junk to find that packet, the cursor sat behind it, so `Ahead::scan`'s `self.scan - self.at` staging check overflowed in debug and wrapped in release, ending the look-ahead at once. The scan now starts at the packet itself, with the framer locked onto it. Commits the `ts` fuzz target's 567-byte finding as a regression input and adds a unit test that splices junk before a switch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 2192b19
No actionable bugs found. Starting look-ahead at the located packet fixes the ordering invariant directly, and the focused test plus fuzz input cover the resynchronization case.
Static GitHub review only; I did not run tests or fuzzing.
|
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — full PR (4 files, 2 commits):
Steps::judgeseeds the look-ahead at the step packet —triggernow receives the post-peekframer(synced onto the packet atat) and setsscan: at, so the look-ahead can no longer begin withscan < at. This eliminates theself.scan - self.atunderflow inAhead::scanthat panicked in debug and wrapped to a hugeErrin release, skipping the step.- New unit test
a_resync_before_the_step_reads_on_from_it— inserts 5 junk bytes before a switch and asserts the rewind is still judged, exercising the resync path the fix targets. - Fuzz regression
rs/moq-mux/fuzz/regressions/ts/step-scan— the 567-bytetsinput, replayed byfuzz::tests::regressions. - Quest cleanup —
ts-step-scan.mdand its README link removed.
I verified the mechanics against Demux::peek/cursor and Framer::next: peek returns the packet-start offset and leaves the framer synced there, and cursor <= at always, so scan: at reproduces the old scan progression exactly after the first iteration while replacing the underflowing initial staging check with 0. I also ran container::ts::step::tests (5 passed) and confirmed the new test fails with attempt to subtract with overflow at step.rs:425 when scan is seeded from the pre-peek cursor, so the regression coverage is genuine.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Finishes quest/m1/fuzz-findings/ts-step-scan.md, one child of the media fuzz findings epic.
Summary
The
tsfuzz target's 567-byte input from the moq-dev/moq#7313 findings comment overflowedself.scan - self.atinAhead::scan(rs/moq-mux/src/container/ts/step.rs).Cause.
Steps::judgegavetriggerthe demux cursor from beforepeek. If the framer had to resync past junk to find the packet that showed the step, that packet's offsetatwas ahead of the cursor, so the look-ahead began withscan < at. In debug builds the staging check panicked. In release builds it wrapped to a huge value, so the look-ahead returnedErr(scan)at once. That cleared a point before the step and skipped judging it.Fix. The look-ahead now starts at
at, with the framer thatpeeklocked onto that packet, so it reads that packet again first, as it always did.scancan no longer be behindat, and the subtraction stays unsaturated. The restart's remainder and the demux drain still use the routed cursor, so the junk before the step still goes to the next instance.a_resync_before_the_step_reads_on_from_itis a new unit test that splices 5 junk bytes before a switch, built with the module'spes/secondhelpers. It panics with the same overflow onmainand passes with the fix.rs/moq-mux/fuzz/regressions/ts/step-scan, whichfuzz::tests::regressionsreplays.Public API: none. Wire: none.
Checks
cargo fmt --all --check: passescargo clippy -p moq-mux --all-targets --all-features -- -D warnings: passescargo test -p moq-mux --all-features: 1988 passed, 3 ignored, plus doc and example testsquest check: passesOnly moq-mux was built (disk on the host is tight). Its change touches no other crate.
Choices made without asking
ts/step-scan, matching the quest name, asmkv/short-block-headermatches its finding.Review
quest review), gpt-6.1-sol, medium effort(Written by Claude Opus 5.5)