Repository: pion/webrtc
Pull request #1610 (2020-12-24 07:20:03)

Use proper values for application media section
Update from draft-ietf-mmusic-sctp-sdp


codecov[bot]: # [Codecov](https://codecov.io/gh/pion/webrtc/pull/1610?src=pr&el=h1) Report
> Merging [#1610](https://codecov.io/gh/pion/webrtc/pull/1610?src=pr&el=desc) (06cc316) into [master](https://codecov.io/gh/pion/webrtc/commit/de913a99a885cf17d14a3ffa3de2f3b99039ad7d?el=desc) (de913a9) will **decrease** coverage by `4.93%`.
> The diff coverage is `n/a`.

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

```diff
@@            Coverage Diff             @@
##           master    #1610      +/-   ##
==========================================
- Coverage   75.38%   70.45%   -4.94%     
==========================================
  Files          78       57      -21     
  Lines        5611     2112    -3499     
==========================================
- Hits         4230     1488    -2742     
+ Misses       1011      524     -487     
+ Partials      370      100     -270     
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `?` | |
| wasm | `70.45% <ø> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pion/webrtc/pull/1610?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [icetcp.go](https://codecov.io/gh/pion/webrtc/pull/1610/diff?src=pr&el=tree#diff-aWNldGNwLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [sdpsemantics.go](https://codecov.io/gh/pion/webrtc/pull/1610/diff?src=pr&el=tree#diff-c2Rwc2VtYW50aWNzLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [rtpcodec.go](https://codecov.io/gh/pion/webrtc/pull/1610/diff?src=pr&el=tree#diff-cnRwY29kZWMuZ28=) | `0.00% <0.00%> (-89.48%)` | :arrow_down: |
| [icecandidatepair.go](https://codecov.io/gh/pion/webrtc/pull/1610/diff?src=pr&el=tree#diff-aWNlY2FuZGlkYXRlcGFpci5nbw==) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [track\_local.go](https://codecov.io/gh/pion/webrtc/pull/1610/diff?src=pr&el=tree#diff-dHJhY2tfbG9jYWwuZ28=) | `0.00% <0.00%> (-80.00%)` | :arrow_down: |
| [stats.go](https://codecov.io/gh/pion/webrtc/pull/1610/diff?src=pr&el=tree#diff-c3RhdHMuZ28=) | `0.00% <0.00%> (-67.86%)` | :arrow_down: |
| [gathering\_complete\_promise.go](https://codecov.io/gh/pion/webrtc/pull/1610/diff?src=pr&el=tree#diff-Z2F0aGVyaW5nX2NvbXBsZXRlX3Byb21pc2UuZ28=) | `60.00% <0.00%> (-40.00%)` | :arrow_down: |
| [dtlsrole.go](https://codecov.io/gh/pion/webrtc/pull/1610/diff?src=pr&el=tree#diff-ZHRsc3JvbGUuZ28=) | `68.96% <0.00%> (-31.04%)` | :arrow_down: |
| [networktype.go](https://codecov.io/gh/pion/webrtc/pull/1610/diff?src=pr&el=tree#diff-bmV0d29ya3R5cGUuZ28=) | `46.80% <0.00%> (-19.15%)` | :arrow_down: |
| [rtptransceiverdirection.go](https://codecov.io/gh/pion/webrtc/pull/1610/diff?src=pr&el=tree#diff-cnRwdHJhbnNjZWl2ZXJkaXJlY3Rpb24uZ28=) | `68.75% <0.00%> (-18.75%)` | :arrow_down: |
| ... and [25 more](https://codecov.io/gh/pion/webrtc/pull/1610/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/webrtc/pull/1610?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/webrtc/pull/1610?src=pr&el=footer). Last update [de913a9...06cc316](https://codecov.io/gh/pion/webrtc/pull/1610?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/sdp.go b/sdp.go
index 5bdd6666..7454696f 100644
--- a/sdp.go
+++ b/sdp.go
@@ -216,8 +216,8 @@ func addDataMediaSection(d *sdp.SessionDescription, shouldAddCandidates bool, dt
 		MediaName: sdp.MediaName{
 			Media:   mediaSectionApplication,
 			Port:    sdp.RangedPort{Value: 9},
-			Protos:  []string{"DTLS", "SCTP"},
-			Formats: []string{"5000"},
+			Protos:  []string{"UDP", "DTLS", "SCTP"},
+			Formats: []string{"webrtc-datachannel"},
 		},
 		ConnectionInformation: &sdp.ConnectionInformation{
 			NetworkType: "IN",
@@ -230,7 +230,7 @@ func addDataMediaSection(d *sdp.SessionDescription, shouldAddCandidates bool, dt
 		WithValueAttribute(sdp.AttrKeyConnectionSetup, dtlsRole.String()).
 		WithValueAttribute(sdp.AttrKeyMID, midValue).
 		WithPropertyAttribute(RTPTransceiverDirectionSendrecv.String()).
-		WithPropertyAttribute("sctpmap:5000 webrtc-datachannel 1024").
+		WithPropertyAttribute("sctp-port:5000").
 		WithICECredentials(iceParams.UsernameFragment, iceParams.Password)
 
 	for _, f := range dtlsFingerprints {


Test patch:
diff --git a/datachannel_go_test.go b/datachannel_go_test.go
index e68337c5..9fdd6ec7 100644
--- a/datachannel_go_test.go
+++ b/datachannel_go_test.go
@@ -10,6 +10,8 @@ import (
 	"io/ioutil"
 	"math/big"
 	"reflect"
+	"regexp"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -519,3 +521,60 @@ func TestEOF(t *testing.T) {
 		<-dcbClosedCh // (2)
 	})
 }
+
+// Assert that a Session Description that doesn't follow
+// draft-ietf-mmusic-sctp-sdp is still accepted
+func TestDataChannel_NonStandardSessionDescription(t *testing.T) {
+	to := test.TimeOut(time.Second * 20)
+	defer to.Stop()
+
+	report := test.CheckRoutines(t)
+	defer report()
+
+	offerPC, answerPC, err := newPair()
+	assert.NoError(t, err)
+
+	_, err = offerPC.CreateDataChannel("foo", nil)
+	assert.NoError(t, err)
+
+	onDataChannelCalled := make(chan struct{})
+	answerPC.OnDataChannel(func(_ *DataChannel) {
+		close(onDataChannelCalled)
+	})
+
+	offer, err := offerPC.CreateOffer(nil)
+	assert.NoError(t, err)
+
+	offerGatheringComplete := GatheringCompletePromise(offerPC)
+	assert.NoError(t, offerPC.SetLocalDescription(offer))
+	<-offerGatheringComplete
+
+	offer = *offerPC.LocalDescription()
+
+	// Replace with old values
+	const (
+		oldApplication = "m=application 63743 DTLS/SCTP 5000\r"
+		oldAttribute   = "a=sctpmap:5000 webrtc-datachannel 256\r"
+	)
+
+	offer.SDP = regexp.MustCompile(`m=application (.*?)\r`).ReplaceAllString(offer.SDP, oldApplication)
+	offer.SDP = regexp.MustCompile(`a=sctp-port(.*?)\r`).ReplaceAllString(offer.SDP, oldAttribute)
+
+	// Assert that replace worked
+	assert.True(t, strings.Contains(offer.SDP, oldApplication))
+	assert.True(t, strings.Contains(offer.SDP, oldAttribute))
+
+	assert.NoError(t, answerPC.SetRemoteDescription(offer))
+
+	answer, err := answerPC.CreateAnswer(nil)
+	assert.NoError(t, err)
+
+	answerGatheringComplete := GatheringCompletePromise(answerPC)
+	assert.NoError(t, answerPC.SetLocalDescription(answer))
+	<-answerGatheringComplete
+	assert.NoError(t, offerPC.SetRemoteDescription(*answerPC.LocalDescription()))
+
+	<-onDataChannelCalled
+	assert.NoError(t, offerPC.Close())
+	assert.NoError(t, answerPC.Close())
+}