Pad profile extensions in report MarshalSize

[dylanpulver]

Description

rtcp.Unmarshal rejects a ReceiverReport that ReceiverReport.Marshal just produced, whenever ProfileExtensions is non-empty:

rr := &rtcp.ReceiverReport{SSRC: 1, ProfileExtensions: make([]byte, 24)}
b, _ := rr.Marshal()          // 32 bytes written, header length field says 8
_, err := rtcp.Unmarshal(b)   // rtcp: packet too short

Two mistakes about the padding RFC 3550 requires on profile-specific extensions:

• ReceiverReport.MarshalSize leaves the extensions out altogether — it returns 8 for the packet above — so CompoundPacket.MarshalSize, which sums its members, under-reports too.
• ReceiverReport.Header compensates with getPadding(len(r.ProfileExtensions)), the 0–3 byte pad count rather than the size of the extensions. For a 24-byte extension that term is 0, so the header length stays 1.

SenderReport is the in-repo precedent for the right shape: its MarshalSize adds the extensions and its Header is the plain (MarshalSize()/4)-1. It adds the unpadded length though, so it is wrong for any extension length that is not a multiple of four.

Measured on b12c5f1, MarshalSize / bytes written / header length / round trip:

ext	RR before	RR after	SR before	SR after
1	8 / 12 / 4 / err	12 / 12 / 2 / ok	29 / 29 / 6 / err	32 / 32 / 7 / ok
5	8 / 16 / 4 / err	16 / 16 / 3 / ok	33 / 33 / 7 / err	36 / 36 / 8 / ok
24	8 / 32 / 1 / err	32 / 32 / 7 / ok	ok	ok

Both MarshalSize methods now add the padded length, ReceiverReport.Header drops the pad term, and ReceiverReport.Marshal copies the extensions into the already-sized buffer the way SenderReport.Marshal does.

Why nothing caught it: every case in TestReceiverReportRoundTrip sets ProfileExtensions: []byte{}, so the receiver side is never marshalled with extensions at all, and the one TestSenderReportRoundTrip case that has them uses exactly four bytes.

The test went in packet_test.go since it covers both types. Reverting both source files fails it on all five extension lengths for the receiver report and on 1 and 5 for the sender report; adding the extensions without the padding still fails 1 and 5 on both; fixing only the receiver report leaves the sender-report cases failing. go test -race ./... green, golangci-lint run clean on v2.10.1 (the version CI pins).

Worth flagging: receiver reports with extensions now go out with a different, correct header length. The old output could not be parsed, so I doubt anything relies on it, but it is a wire-format change.

I used Claude Code to help investigate and write this up; the failure came out of fuzzing Marshal and Unmarshal against each other.

Reference issue

None — found while fuzzing the marshal/unmarshal round trip.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.68%. Comparing base (b12c5f1) to head (bda55ff).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #226      +/-   ##
==========================================
+ Coverage   77.50%   77.68%   +0.17%     
==========================================
  Files          22       22              
  Lines        2032     2030       -2     
==========================================
+ Hits         1575     1577       +2     
+ Misses        359      356       -3     
+ Partials       98       97       -1     

Flag	Coverage Δ
go	77.68% <100.00%> (+0.17%)	⬆️

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.

[JoTurk]

Thank you