fix(rtcp,sctp): RFC 3550 compliance and SCTP INIT safety hardening#69
Open
nightness wants to merge 2 commits into
Open
fix(rtcp,sctp): RFC 3550 compliance and SCTP INIT safety hardening#69nightness wants to merge 2 commits into
nightness wants to merge 2 commits into
Conversation
nightness
force-pushed
the
fix/rtcp-sctp-correctness
branch
from
97589cf to
8874195
Compare
6 tasks
rainliu
approved these changes
Apr 4, 2026
Member
rainliu
left a comment
rainliu
left a comment
There was a problem hiding this comment.
The changes look good to me.
But please fix existing tests failure due to the above changes.
rtc-rtcp: Change ReceptionReport::total_lost from u32 to i32 (RFC 3550
§6.4.1). The field is a signed 24-bit integer; negative values occur
when duplicate packets cause received > expected. Deserialization now
sign-extends from bit 23; serialisation validates against the signed
24-bit range (-8 388 608..=8 388 607).
rtc-sctp: Replace `initiate_tag.as_ref().unwrap()` with a compiler-
verified `let Some(...) else { return None }` guard. The unwrap was
technically safe due to an earlier check on line 214, but that coupling
was invisible to the compiler. The new guard is self-documenting and
eliminates the brittle dependency.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The reception_report::total_lost field was changed from u32 to i32 (signed 24-bit per RFC 3550 §6.4.1). Update the interceptor's ReceiverStream to use i32 and clamp to the signed 24-bit max (0x7FFFFF) instead of the unsigned 0xFFFFFF. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
nightness
force-pushed
the
fix/rtcp-sctp-correctness
branch
from
633b171 to
e5e7ab3
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
2 participants
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
rtc-rtcp -- RFC 3550 section 6.4.1 compliance
ReceptionReport::total_lostwasu32but the field is defined by RFC 3550 as a signed 24-bit integer. Negative values occur when duplicate packets arrive (received > expected). This change:i32rtc-interceptor -- signed total_lost propagation
ReceiverStream::total_lostfromu32toi32to match the RTCP type change0x7FFFFF) instead of unsigned (0xFFFFFF)rtc-sctp -- SCTP INIT tag guard
handle_first_packethad a guard on line 214 that checkedinitiate_tag.is_none()before reaching the.unwrap()on line 239, but the compiler could not verify the coupling. Replaces the unwrap with alet Some(...) else { return None }guard directly at the use site -- self-documenting and compiler-verified.rtc-stun -- restore error_code Display tests
Reverted the
ErrorCodeAttribute::Displayimpl back tofrom_utf8_lossy(matching PR #64) because thefrom_utf8+Err(fmt::Error)approach causesformat!()to panic on invalid UTF-8 input. Restored the two test cases that verify this behavior.Test Plan
cargo build -p rtc-rtcp -p rtc-sctppassescargo test -p rtc-rtcp-- 52 tests passcargo test -p rtc-stun-- 65 tests pass (including restored error_code tests)cargo test-- full workspace passes (0 failures)cargo clippy-- no warningscargo fmt --check-- cleanGenerated with Claude Code