Skip to content

fix: return error on packet length overflow in TransportLayerCC and CCFeedbackReport - #235

Merged
JoTurk merged 1 commit into
pion:mainfrom
AdamMagued:fix/issue-234-marshal-overflow-packet-too-large
Oct 21, 2026
Merged

JoTurk merged 1 commit into
pion:mainfrom
AdamMagued:fix/issue-234-marshal-overflow-packet-too-large

Conversation

@AdamMagued

Copy link
Copy Markdown
Contributor

Fixes #234

Problem

In TransportLayerCC and CCFeedbackReport, Marshal calculates the packet length in 32-bit words and casts it to a uint16. When a packet contains a large number of deltas or feedback metric blocks such that the payload exceeds math.MaxUint16 words, the integer wraps around. This causes the allocated buffer to be undersized, triggering a panic: runtime error: slice bounds out of range during marshaling.

Solution

  • Added overflow checks in TransportLayerCC.Marshal / MarshalTo and CCFeedbackReport.Marshal / MarshalTo returning errPacketTooLarge when the packet size cannot fit within the RTCP length field.
  • Added unit tests in transport_layer_cc_test.go and rfc8888_test.go verifying that oversized packets return an error gracefully rather than panicking.

Verified via container sandbox test suite.

@AdamMagued
AdamMagued force-pushed the fix/issue-234-marshal-overflow-packet-too-large branch from b51175d to 767fa86 Compare October 17, 2026 03:56
@codecov

codecov Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.67%. Comparing base (a39ade4) to head (7471b36).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #235      +/-   ##
==========================================
+ Coverage   84.65%   84.67%   +0.02%     
==========================================
  Files          22       22              
  Lines        1668     1671       +3     
==========================================
+ Hits         1412     1415       +3     
  Misses        256      256              
Flag Coverage Δ
go 84.67% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Reported-by: sollniss <sollniss@web.de>
Signed-off-by: Adam Magued <adamismailmageud@gmail.com>
@JoTurk
JoTurk force-pushed the fix/issue-234-marshal-overflow-packet-too-large branch from 767fa86 to 7471b36 Compare October 21, 2026 01:48
@JoTurk

JoTurk commented Oct 21, 2026

Copy link
Copy Markdown
Member

hello, we needed to change the API for the twcc change, but the CCFB fix is good, thank you.

@JoTurk
JoTurk merged commit 87fbc9c into pion:main Oct 21, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

TransportLayerCC / CCFeedbackReport: Marshal panics on packets over the uint16 length limit

2 participants