From bda55ff32fec40307734f9146121f85c6c16bc20 Mon Sep 17 00:00:00 2001 From: Dylan Pulver Date: Tue, 1 Sep 2026 12:42:56 +0300 Subject: [PATCH] Pad profile extensions in report MarshalSize RFC 3550 profile-specific extensions are padded out to a whole 32-bit word when a sender or receiver report is marshalled, but neither MarshalSize accounted for that padding. ReceiverReport.MarshalSize left the extensions out entirely, and ReceiverReport.Header added getPadding(len(ProfileExtensions)), a 0-3 byte pad count, to the header length instead of the size of the extensions. Any receiver report carrying extensions was therefore written with a header length that under-reported the packet, and rtcp.Unmarshal rejected its own output. SenderReport.MarshalSize already added the extensions, but not their padding, so it was wrong whenever the extension length was not a multiple of four. Both 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. --- packet_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++ receiver_report.go | 19 +++++++---------- sender_report.go | 4 +++- 3 files changed, 61 insertions(+), 13 deletions(-) diff --git a/packet_test.go b/packet_test.go index ef7a8b8..04d2cfa 100644 --- a/packet_test.go +++ b/packet_test.go @@ -4,6 +4,7 @@ package rtcp import ( + "bytes" "testing" "github.com/stretchr/testify/assert" @@ -153,3 +154,53 @@ func TestUnmarshalMaxLengthRawPacket(t *testing.T) { assert.True(t, ok) assert.Equal(t, rawPacket, []byte(*parsed)) } + +// Sender and receiver reports carry profile-specific extensions that are padded +// out to a whole 32-bit word, so both the header length field and MarshalSize +// have to account for that padding. +func TestReportProfileExtensionsRoundTrip(t *testing.T) { + for _, extLength := range []int{1, 4, 5, 8, 24} { + extensions := bytes.Repeat([]byte{0xab}, extLength) + padded := append(bytes.Repeat([]byte{0xab}, extLength), make([]byte, getPadding(extLength))...) + reports := []ReceptionReport{{SSRC: 2}} + + receiverReport := &ReceiverReport{SSRC: 1, Reports: reports, ProfileExtensions: extensions} + senderReport := &SenderReport{SSRC: 1, Reports: reports, ProfileExtensions: extensions} + + for _, test := range []struct { + Name string + Packet Packet + Header func() Header + Want Packet + }{ + { + Name: "ReceiverReport", + Packet: receiverReport, + Header: receiverReport.Header, + Want: &ReceiverReport{SSRC: 1, Reports: reports, ProfileExtensions: padded}, + }, + { + Name: "SenderReport", + Packet: senderReport, + Header: senderReport.Header, + Want: &SenderReport{SSRC: 1, Reports: reports, ProfileExtensions: padded}, + }, + } { + data, err := test.Packet.Marshal() + assert.NoErrorf(t, err, "Marshal %s with %d extension bytes", test.Name, extLength) + + assert.Lenf(t, data, test.Packet.MarshalSize(), + "MarshalSize %s with %d extension bytes", test.Name, extLength) + assert.Equalf(t, uint16(len(data)/4-1), test.Header().Length, //nolint:gosec // G115 + "header length %s with %d extension bytes", test.Name, extLength) + + packets, err := Unmarshal(data) + assert.NoErrorf(t, err, "Unmarshal %s with %d extension bytes", test.Name, extLength) + if !assert.Lenf(t, packets, 1, "packet count %s with %d extension bytes", test.Name, extLength) { + continue + } + assert.Equalf(t, test.Want, packets[0], + "round trip %s with %d extension bytes", test.Name, extLength) + } + } +} diff --git a/receiver_report.go b/receiver_report.go index 7e08a2a..7ee76b4 100644 --- a/receiver_report.go +++ b/receiver_report.go @@ -77,16 +77,9 @@ func (r ReceiverReport) Marshal() ([]byte, error) { return nil, errTooManyReports } - pe := make([]byte, len(r.ProfileExtensions)) - copy(pe, r.ProfileExtensions) - - // if the length of the profile extensions isn't devisible - // by 4, we need to pad the end. - for (len(pe) & 0x3) != 0 { - pe = append(pe, 0) //nolint:makezero - } - - rawPacket = append(rawPacket, pe...) //nolint:makezero + // The buffer is already sized for the profile extensions and their padding, + // so any trailing pad bytes are left as the zeroes MarshalSize allocated. + copy(packetBody[ssrcLength+receptionReportLength*len(r.Reports):], r.ProfileExtensions) hData, err := r.Header().Marshal() if err != nil { @@ -166,7 +159,9 @@ func (r *ReceiverReport) MarshalSize() int { repsLength += rep.len() } - return headerLength + ssrcLength + repsLength + extLength := len(r.ProfileExtensions) + + return headerLength + ssrcLength + repsLength + extLength + getPadding(extLength) } // Header returns the Header associated with this packet. @@ -174,7 +169,7 @@ func (r *ReceiverReport) Header() Header { return Header{ Count: uint8(len(r.Reports)), //nolint:gosec // G115 Type: TypeReceiverReport, - Length: uint16((r.MarshalSize()/4)-1) + uint16(getPadding(len(r.ProfileExtensions))), //nolint:gosec // G115 + Length: uint16((r.MarshalSize() / 4) - 1), //nolint:gosec // G115 } } diff --git a/sender_report.go b/sender_report.go index 9e5ed57..23f482d 100644 --- a/sender_report.go +++ b/sender_report.go @@ -237,7 +237,9 @@ func (r *SenderReport) MarshalSize() int { repsLength += rep.len() } - return headerLength + srHeaderLength + repsLength + len(r.ProfileExtensions) + extLength := len(r.ProfileExtensions) + + return headerLength + srHeaderLength + repsLength + extLength + getPadding(extLength) } // Header returns the Header associated with this packet.