fix(interceptor): clamp total_lost to signed 24-bit max per RFC 3550#77
Open
nightness wants to merge 3 commits into
Open
fix(interceptor): clamp total_lost to signed 24-bit max per RFC 3550#77nightness wants to merge 3 commits into
nightness wants to merge 3 commits into
Conversation
nightness
force-pushed
the
fix/interceptor-total-lost
branch
3 times, most recently
from
7267904 to
65f0c5d
Compare
nightness
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates RTCP receiver report loss handling to respect RFC 3550’s definition of total_lost as a signed 24-bit value, preventing out-of-range values that break/incorrectly serialize reception reports.
Changes:
- Clamp
total_lost/total_lost_since_reportto0x7FFFFFinstead of0xFFFFFF. - Update inline comment to reference RFC 3550 §6.4.1 signed 24-bit definition.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
nightness
added a commit
to Brainwires/webrtc-rs-rtc
that referenced
this pull request
Apr 8, 2026
- Introduce MAX_TOTAL_LOST named constant (0x7F_FFFF) for the signed 24-bit ceiling per RFC 3550 §6.4.1 - Clamp total_lost_since_report before adding to self.total_lost, and use saturating_add to prevent overflow/wrap - Fix test assertion from 0xFFFFFF to MAX_TOTAL_LOST (0x7F_FFFF) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
nightness
added a commit
to Brainwires/webrtc-rs-rtc
that referenced
this pull request
Apr 8, 2026
- Reword MAX_TOTAL_LOST doc to explain u32 storage clamped to signed 24-bit positive range for RFC 3550 wire format - Add matching doc on ReceptionReport.total_lost field - Fix marshal bounds check from (1 << 25) to 0xFF_FFFF (correct 24-bit limit) - Add boundary tests for 24-bit marshal limit (0xFF_FFFF ok, 0x100_0000 rejected) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
RFC 3550 §6.4.1 defines total_lost as a 24-bit signed integer. The previous clamp used 0xFFFFFF (unsigned 24-bit max = 16,777,215) which allowed values that overflow the signed 24-bit range. Fix the clamp to 0x7FFFFF (signed 24-bit positive max = 8,388,607) so values are always representable in the reception report field.
- Introduce MAX_TOTAL_LOST named constant (0x7F_FFFF) for the signed 24-bit ceiling per RFC 3550 §6.4.1 - Clamp total_lost_since_report before adding to self.total_lost, and use saturating_add to prevent overflow/wrap - Fix test assertion from 0xFFFFFF to MAX_TOTAL_LOST (0x7F_FFFF) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Reword MAX_TOTAL_LOST doc to explain u32 storage clamped to signed 24-bit positive range for RFC 3550 wire format - Add matching doc on ReceptionReport.total_lost field - Fix marshal bounds check from (1 << 25) to 0xFF_FFFF (correct 24-bit limit) - Add boundary tests for 24-bit marshal limit (0xFF_FFFF ok, 0x100_0000 rejected) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
nightness
force-pushed
the
fix/interceptor-total-lost
branch
from
600462f to
6a8d164
Compare
Author
|
Rebased onto upstream/master so this PR contains only its own changes. Previous branch structure caused merge conflicts when PRs were merged in sequence. Each PR is now independently mergeable. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
receiver_stream.rs: fix thetotal_lostclamp from0xFFFFFF(unsigned 24-bit max = 16,777,215) to0x7FFFFF(signed 24-bit positive max = 8,388,607).MAX_TOTAL_LOSTnamed constant for the signed 24-bit ceiling, replacing bare0x7FFFFFliterals.total_lost_since_reportbefore adding toself.total_lostand usesaturating_addto prevent overflow/wrap.MAX_TOTAL_LOSTandReceptionReport.total_lostto clarify u32 storage clamped to signed 24-bit positive range for RFC 3550 wire format.>= (1 << 25)to> 0xFF_FFFF(correct 24-bit limit).RFC 3550 §6.4.1 defines
total_lostas a 24-bit signed integer. The RTCP reception report structure marshals it as a signed value (seertc-rtcpPR #69 which adds the> 0x7F_FFFFbounds check). The previous clamp at0xFFFFFFallowed values between0x800000and0xFFFFFFto pass, which are in-range for unsigned 24-bit but out-of-range for signed 24-bit — those values would fail or produce incorrect RTCP reports.Test plan
total_lostis clamped at 8,388,607 rather than wrapping or failing RTCP serializationcargo fmt --checkpassescargo clippypasses0xFF_FFFFaccepted,0x100_0000rejected