Skip to content

Commit 7752cd4

Browse files
authored
Fix num_reports usage, see errata 8166 (#187)
https://www.rfc-editor.org/errata/eid8166
1 parent ec84594 commit 7752cd4

File tree

2 files changed

+42
-26
lines changed

2 files changed

+42
-26
lines changed

rfc8888.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,6 @@ func (b CCFeedbackReportBlock) marshal() ([]byte, error) {
241241
binary.BigEndian.PutUint16(buf[beginSequenceOffset:], b.BeginSequence)
242242

243243
length := uint16(len(b.MetricBlocks)) //nolint:gosec // G115
244-
if length > 0 {
245-
length--
246-
}
247244

248245
binary.BigEndian.PutUint16(buf[numReportsOffset:], length)
249246

@@ -265,18 +262,15 @@ func (b *CCFeedbackReportBlock) unmarshal(rawPacket []byte) error {
265262
}
266263
b.MediaSSRC = binary.BigEndian.Uint32(rawPacket[:beginSequenceOffset])
267264
b.BeginSequence = binary.BigEndian.Uint16(rawPacket[beginSequenceOffset:numReportsOffset])
268-
numReportsField := binary.BigEndian.Uint16(rawPacket[numReportsOffset:])
269-
if numReportsField == 0 {
265+
numReports := int(binary.BigEndian.Uint16(rawPacket[numReportsOffset:]))
266+
if numReports == 0 {
270267
return nil
271268
}
272269

273-
if int(b.BeginSequence)+int(numReportsField) > math.MaxUint16 {
270+
if numReports > math.MaxUint16 {
274271
return errIncorrectNumReports
275272
}
276273

277-
endSequence := b.BeginSequence + numReportsField
278-
numReports := int(endSequence - b.BeginSequence + 1)
279-
280274
if len(rawPacket) < reportsOffset+numReports*2 {
281275
return errIncorrectNumReports
282276
}

rfc8888_test.go

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ func TestCCFeedbackReportBlockUnmarshalMarshal(t *testing.T) {
151151
Name: "ReceivedTwoOFFourBlocks",
152152
Data: []byte{
153153
0x00, 0x00, 0x00, 0x01, // SSRC
154-
0x00, 0x02, 0x00, 0x03, // begin_seq, num_reports
154+
0x00, 0x02, 0x00, 0x04, // begin_seq, num_reports
155155
0x9F, 0xFD, 0x9F, 0xFC, // reports[0], reports[1]
156156
0x00, 0x00, 0x00, 0x00, // reports[2], reports[3]
157157
},
@@ -186,7 +186,7 @@ func TestCCFeedbackReportBlockUnmarshalMarshal(t *testing.T) {
186186
Name: "ReceivedTwoOFThreeBlocksPadding",
187187
Data: []byte{
188188
0x00, 0x00, 0x00, 0x01, // SSRC
189-
0x00, 0x02, 0x00, 0x02, // begin_seq, num_reports
189+
0x00, 0x02, 0x00, 0x03, // begin_seq, num_reports
190190
0x9F, 0xFD, 0x9F, 0xFC, // reports[0], reports[1]
191191
0x00, 0x00, 0x00, 0x00, // reports[2], Padding
192192
},
@@ -212,6 +212,41 @@ func TestCCFeedbackReportBlockUnmarshalMarshal(t *testing.T) {
212212
},
213213
},
214214
},
215+
{
216+
Name: "WrapAroundSequenceNumber",
217+
Data: []byte{
218+
0x00, 0x00, 0x00, 0x01, // SSRC
219+
0xff, 0xfe, 0x00, 0x04, // begin_seq, num_reports
220+
0x9F, 0xFD, 0x9F, 0xFC, // reports[0], reports[1]
221+
0x00, 0x00, 0x00, 0x00, // reports[2], reports[3]
222+
},
223+
Want: CCFeedbackReportBlock{
224+
MediaSSRC: 1,
225+
BeginSequence: 65534,
226+
MetricBlocks: []CCFeedbackMetricBlock{
227+
{
228+
Received: true,
229+
ECN: 0,
230+
ArrivalTimeOffset: 8189,
231+
},
232+
{
233+
Received: true,
234+
ECN: 0,
235+
ArrivalTimeOffset: 8188,
236+
},
237+
{
238+
Received: false,
239+
ECN: 0,
240+
ArrivalTimeOffset: 0,
241+
},
242+
{
243+
Received: false,
244+
ECN: 0,
245+
ArrivalTimeOffset: 0,
246+
},
247+
},
248+
},
249+
},
215250
} {
216251
test := test
217252
t.Run(fmt.Sprintf("Unmarshal-%v", test.Name), func(t *testing.T) {
@@ -270,19 +305,6 @@ func TestCCFeedbackReportBlockUnmarshalMarshal(t *testing.T) {
270305
assert.ErrorIs(t, err, errIncorrectNumReports)
271306
})
272307

273-
t.Run("overflowEndSequence", func(t *testing.T) {
274-
var block CCFeedbackReportBlock
275-
data := []byte{
276-
0x00, 0x00, 0x00, 0x01, // SSRC
277-
0xff, 0xfe, 0x00, 0x02, // begin_seq, num_reports
278-
0x9F, 0xFD, 0x9F, 0xFC, // reports[0], reports[1]
279-
0x00, 0x00, 0x00, 0x00, // reports[2], reports[3]
280-
}
281-
err := block.unmarshal(data)
282-
assert.Error(t, err)
283-
assert.ErrorIs(t, err, errIncorrectNumReports)
284-
})
285-
286308
t.Run("overflowNumReports", func(t *testing.T) {
287309
var block CCFeedbackReportBlock
288310
data := append([]byte{
@@ -321,12 +343,12 @@ func TestCCFeedbackReportUnmarshalMarshal(t *testing.T) {
321343
0x00, 0x00, 0x00, 0x01, // Sender SSRC=1
322344

323345
0x00, 0x00, 0x00, 0x01, // SSRC=1
324-
0x00, 0x02, 0x00, 0x03, // begin_seq, num_reports
346+
0x00, 0x02, 0x00, 0x04, // begin_seq, num_reports
325347
0x9F, 0xFD, 0x9F, 0xFC, // reports[0], reports[1]
326348
0x00, 0x00, 0x00, 0x00, // reports[2], reports[3]
327349

328350
0x00, 0x00, 0x00, 0x02, // Media SSRC=2
329-
0x00, 0x02, 0x00, 0x02, // begin_seq=2, num_reports=3
351+
0x00, 0x02, 0x00, 0x03, // begin_seq=2, num_reports=3
330352
0x9F, 0xFD, 0x9F, 0xFC, // reports[0], reports[1]
331353
0x00, 0x00, 0x00, 0x00, // reports[2], Padding
332354

0 commit comments

Comments
 (0)