Repository: pion/rtp
Pull request #61 (2020-05-18 04:41:57)

Fix extension padding on MarshalTo to dirty buffer
MarshalTo to non-zero buffer didn't clear padding bytes.
(regression from 4b1b5825)

### Reference issue
Ref #56 and #58 


codecov[bot]: # [Codecov](https://codecov.io/gh/pion/rtp/pull/61?src=pr&el=h1) Report
> Merging [#61](https://codecov.io/gh/pion/rtp/pull/61?src=pr&el=desc) into [master](https://codecov.io/gh/pion/rtp/commit/ee3e7ab3d3222745a80bc10bdf6569da843d7bc3&el=desc) will **increase** coverage by `0.03%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/pion/rtp/pull/61/graphs/tree.svg?width=650&height=150&src=pr&token=4KrbiBsspj)](https://codecov.io/gh/pion/rtp/pull/61?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #61      +/-   ##
==========================================
+ Coverage   88.85%   88.88%   +0.03%     
==========================================
  Files          13       13              
  Lines         628      630       +2     
==========================================
+ Hits          558      560       +2     
  Misses         54       54              
  Partials       16       16              
```

| Flag | Coverage Δ | |
|---|---|---|
| #go | `88.88% <100.00%> (+0.03%)` | :arrow_up: |
| #wasm | `88.88% <100.00%> (+0.03%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pion/rtp/pull/61?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [packet.go](https://codecov.io/gh/pion/rtp/pull/61/diff?src=pr&el=tree#diff-cGFja2V0Lmdv) | `82.85% <100.00%> (+0.16%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/rtp/pull/61?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/rtp/pull/61?src=pr&el=footer). Last update [ee3e7ab...587a739](https://codecov.io/gh/pion/rtp/pull/61?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

sgotti: @at-wat LGTM! I totally forgot that the buf could be reused. Thanks!

Patch:
diff --git a/packet.go b/packet.go
index 6986623..eb8a200 100644
--- a/packet.go
+++ b/packet.go
@@ -301,7 +301,10 @@ func (h *Header) MarshalTo(buf []byte) (n int, err error) {
 		binary.BigEndian.PutUint16(buf[extHeaderPos+2:extHeaderPos+4], uint16(roundedExtSize/4))
 
 		// add padding to reach 4 bytes boundaries
-		n += roundedExtSize - extSize
+		for i := 0; i < roundedExtSize-extSize; i++ {
+			buf[n] = 0
+			n++
+		}
 	}
 
 	h.PayloadOffset = n


Test patch:
diff --git a/packet_test.go b/packet_test.go
index 89eba2b..e4870e6 100644
--- a/packet_test.go
+++ b/packet_test.go
@@ -247,6 +247,33 @@ func TestRFC8285OneByteMultipleExtensionsWithPadding(t *testing.T) {
 	if !bytes.Equal(ext3, ext3Expect) {
 		t.Errorf("Extension has incorrect data. Got: %v+, Expected: %v+", ext3, ext3Expect)
 	}
+
+	rawPktReMarshal := []byte{
+		0x90, 0xe0, 0x69, 0x8f, 0xd9, 0xc2, 0x93, 0xda, 0x1c, 0x64,
+		0x27, 0x82, 0xBE, 0xDE, 0x00, 0x03, 0x10, 0xAA, 0x21, 0xBB,
+		0xBB, 0x33, 0xCC, 0xCC, 0xCC, 0xCC, 0x00, 0x00, // padding is moved to the end by re-marshaling
+		// Payload
+		0x98, 0x36, 0xbe, 0x88, 0x9e,
+	}
+	dstBuf := map[string][]byte{
+		"CleanBuffer": make([]byte, 1000),
+		"DirtyBuffer": make([]byte, 1000),
+	}
+	for i := range dstBuf["DirtyBuffer"] {
+		dstBuf["DirtyBuffer"][i] = 0xFF
+	}
+	for name, buf := range dstBuf {
+		buf := buf
+		t.Run(name, func(t *testing.T) {
+			n, err := p.MarshalTo(buf)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if !bytes.Equal(buf[:n], rawPktReMarshal) {
+				t.Errorf("Marshal failed raw \nMarshaled:\n%s\nrawPkt:\n%s", hex.Dump(buf[:n]), hex.Dump(rawPktReMarshal))
+			}
+		})
+	}
 }
 
 func TestRFC8285OneByteMultipleExtensions(t *testing.T) {