Skip to content

Commit 7dc2af5

Browse files
Sean-Derjupenur
andcommitted
Combine OneByte and TwoByte extension parsing
Reduces duplicated logic and increase coverage. Missing tests for bounds check on Padding and Payload Sizes Co-Authored-By: Juho Nurminen <[email protected]>
1 parent 930bd85 commit 7dc2af5

File tree

3 files changed

+61
-39
lines changed

3 files changed

+61
-39
lines changed

AUTHORS.txt

+3
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ Guilherme <[email protected]>
1818
Haiyang Wang <[email protected]>
1919
Hugo Arregui <[email protected]>
2020
John Bradley <[email protected]>
21+
Juho Nurminen <[email protected]>
2122
Juliusz Chroboczek <[email protected]>
2223
2324
Kazuyuki Honda <[email protected]>
@@ -31,6 +32,7 @@ Raphael Derosso Pereira <[email protected]>
3132
Rob Lofthouse <[email protected]>
3233
Robin Raymond <[email protected]>
3334
35+
3436
Sean DuBois <[email protected]>
3537
Sean DuBois <[email protected]>
3638
Sean DuBois <[email protected]>
@@ -40,6 +42,7 @@ Steffen Vogel <[email protected]>
4042
Tarrence van As <[email protected]>
4143
wangzixiang <[email protected]>
4244
Woodrow Douglass <[email protected]>
45+
訾明华 <[email protected]>
4346

4447
# List of contributors not appearing in Git history
4548

packet.go

+30-39
Original file line numberDiff line numberDiff line change
@@ -149,64 +149,55 @@ func (h *Header) Unmarshal(buf []byte) (n int, err error) { //nolint:gocognit
149149
n += 2
150150
extensionLength := int(binary.BigEndian.Uint16(buf[n:])) * 4
151151
n += 2
152+
extensionEnd := n + extensionLength
152153

153-
if expected := n + extensionLength; len(buf) < expected {
154-
return n, fmt.Errorf("size %d < %d: %w",
155-
len(buf), expected,
156-
errHeaderSizeInsufficientForExtension,
157-
)
154+
if len(buf) < extensionEnd {
155+
return n, fmt.Errorf("size %d < %d: %w", len(buf), extensionEnd, errHeaderSizeInsufficientForExtension)
158156
}
159157

160-
switch h.ExtensionProfile {
161-
// RFC 8285 RTP One Byte Header Extension
162-
case extensionProfileOneByte:
163-
end := n + extensionLength
164-
for n < end {
158+
if h.ExtensionProfile == extensionProfileOneByte || h.ExtensionProfile == extensionProfileTwoByte {
159+
var (
160+
extid uint8
161+
payloadLen int
162+
)
163+
164+
for n < extensionEnd {
165165
if buf[n] == 0x00 { // padding
166166
n++
167167
continue
168168
}
169169

170-
extid := buf[n] >> 4
171-
payloadLen := int(buf[n]&^0xF0 + 1)
172-
n++
170+
if h.ExtensionProfile == extensionProfileOneByte {
171+
extid = buf[n] >> 4
172+
payloadLen = int(buf[n]&^0xF0 + 1)
173+
n++
173174

174-
if extid == extensionIDReserved {
175-
break
176-
}
175+
if extid == extensionIDReserved {
176+
break
177+
}
178+
} else {
179+
extid = buf[n]
180+
n++
177181

178-
extension := Extension{id: extid, payload: buf[n : n+payloadLen]}
179-
h.Extensions = append(h.Extensions, extension)
180-
n += payloadLen
181-
}
182+
if len(buf) <= n {
183+
return n, fmt.Errorf("size %d < %d: %w", len(buf), n, errHeaderSizeInsufficientForExtension)
184+
}
182185

183-
// RFC 8285 RTP Two Byte Header Extension
184-
case extensionProfileTwoByte:
185-
end := n + extensionLength
186-
for n < end {
187-
if buf[n] == 0x00 { // padding
186+
payloadLen = int(buf[n])
188187
n++
189-
continue
190188
}
191189

192-
extid := buf[n]
193-
n++
194-
195-
payloadLen := int(buf[n])
196-
n++
190+
if extensionPayloadEnd := n + payloadLen; len(buf) <= extensionPayloadEnd {
191+
return n, fmt.Errorf("size %d < %d: %w", len(buf), extensionPayloadEnd, errHeaderSizeInsufficientForExtension)
192+
}
197193

198194
extension := Extension{id: extid, payload: buf[n : n+payloadLen]}
199195
h.Extensions = append(h.Extensions, extension)
200196
n += payloadLen
201197
}
202-
203-
default: // RFC3550 Extension
204-
if len(buf) < n+extensionLength {
205-
return n, fmt.Errorf("%w: %d < %d",
206-
errHeaderSizeInsufficientForExtension, len(buf), n+extensionLength)
207-
}
208-
209-
extension := Extension{id: 0, payload: buf[n : n+extensionLength]}
198+
} else {
199+
// RFC3550 Extension
200+
extension := Extension{id: 0, payload: buf[n:extensionEnd]}
210201
h.Extensions = append(h.Extensions, extension)
211202
n += len(h.Extensions[0].payload)
212203
}

packet_test.go

+28
Original file line numberDiff line numberDiff line change
@@ -1192,6 +1192,34 @@ func TestRFC8285TwoByteSetExtensionShouldErrorWhenPayloadTooLarge(t *testing.T)
11921192
}
11931193
}
11941194

1195+
func TestRFC8285Padding(t *testing.T) {
1196+
header := &Header{}
1197+
1198+
for _, payload := range [][]byte{
1199+
{
1200+
0b00010000, // header.Extension = true
1201+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // SequenceNumber, Timestamp, SSRC
1202+
0xBE, 0xDE, // header.ExtensionProfile = extensionProfileOneByte
1203+
0, 1, // extensionLength
1204+
0, 0, 0, // padding
1205+
1, // extid
1206+
},
1207+
{
1208+
0b00010000, // header.Extension = true
1209+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // SequenceNumber, Timestamp, SSRC
1210+
0x10, 0x00, // header.ExtensionProfile = extensionProfileOneByte
1211+
0, 1, // extensionLength
1212+
0, 0, 0, // padding
1213+
1, // extid
1214+
},
1215+
} {
1216+
_, err := header.Unmarshal(payload)
1217+
if !errors.Is(err, errHeaderSizeInsufficientForExtension) {
1218+
t.Fatal("Expected errHeaderSizeInsufficientForExtension")
1219+
}
1220+
}
1221+
}
1222+
11951223
func TestRFC3550SetExtensionShouldErrorWhenNonZero(t *testing.T) {
11961224
payload := []byte{
11971225
// Payload

0 commit comments

Comments
 (0)