Repository: pion/webrtc
Pull request #2303 (2022-09-21 03:45:33)

Fix ORTC accept datachannel sctpTransport is nil
#### Description

When using ORTC API, the new accepted channel don't set the sctpTransport, see https://github.com/pion/webrtc/blob/master/sctptransport.go#L219.

it will panic if someone try to use it:

```
pair, err := channel.Transport().Transport().ICETransport().GetSelectedCandidatePair()
```


codecov[bot]: # [Codecov](https://codecov.io/gh/pion/webrtc/pull/2303?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
Base: **77.40**% // Head: **70.16**% // Decreases project coverage by **`-7.24%`** :warning:
> Coverage data is based on head [(`95e611b`)](https://codecov.io/gh/pion/webrtc/pull/2303?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) compared to base [(`045df4c`)](https://codecov.io/gh/pion/webrtc/commit/045df4c4bfd47ae0d657af98053f18e9a707ac34?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).
> Patch has no changes to coverable lines.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #2303      +/-   ##
==========================================
- Coverage   77.40%   70.16%   -7.25%     
==========================================
  Files          87       64      -23     
  Lines        9247     3355    -5892     
==========================================
- Hits         7158     2354    -4804     
+ Misses       1655      873     -782     
+ Partials      434      128     -306     
```

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

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pion/webrtc/pull/2303?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [stats.go](https://codecov.io/gh/pion/webrtc/pull/2303/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-c3RhdHMuZ28=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [constants.go](https://codecov.io/gh/pion/webrtc/pull/2303/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Y29uc3RhbnRzLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [sdpsemantics.go](https://codecov.io/gh/pion/webrtc/pull/2303/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-c2Rwc2VtYW50aWNzLmdv) | `0.00% <0.00%> (-82.76%)` | :arrow_down: |
| [icecandidatepair.go](https://codecov.io/gh/pion/webrtc/pull/2303/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aWNlY2FuZGlkYXRlcGFpci5nbw==) | `0.00% <0.00%> (-81.82%)` | :arrow_down: |
| [rtpcodec.go](https://codecov.io/gh/pion/webrtc/pull/2303/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cnRwY29kZWMuZ28=) | `13.33% <0.00%> (-80.00%)` | :arrow_down: |
| [track\_local.go](https://codecov.io/gh/pion/webrtc/pull/2303/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-dHJhY2tfbG9jYWwuZ28=) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
| [icemux.go](https://codecov.io/gh/pion/webrtc/pull/2303/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aWNlbXV4Lmdv) | `0.00% <0.00%> (-54.55%)` | :arrow_down: |
| [internal/mux/endpoint.go](https://codecov.io/gh/pion/webrtc/pull/2303/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aW50ZXJuYWwvbXV4L2VuZHBvaW50Lmdv) | `13.79% <0.00%> (-44.83%)` | :arrow_down: |
| [atomicbool.go](https://codecov.io/gh/pion/webrtc/pull/2303/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-YXRvbWljYm9vbC5nbw==) | `57.14% <0.00%> (-42.86%)` | :arrow_down: |
| [dtlsrole.go](https://codecov.io/gh/pion/webrtc/pull/2303/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-ZHRsc3JvbGUuZ28=) | `71.42% <0.00%> (-28.58%)` | :arrow_down: |
| ... and [37 more](https://codecov.io/gh/pion/webrtc/pull/2303/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/pion/webrtc/pull/2303?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).

Sean-Der: Sorry I missed this @jerry-tao ! I will merge right now, thank you so much for the contribution :) 

Patch:
diff --git a/datachannel.go b/datachannel.go
index 1ee1516c..17277de3 100644
--- a/datachannel.go
+++ b/datachannel.go
@@ -67,7 +67,7 @@ type DataChannel struct {
 // This constructor is part of the ORTC API. It is not
 // meant to be used together with the basic WebRTC API.
 func (api *API) NewDataChannel(transport *SCTPTransport, params *DataChannelParameters) (*DataChannel, error) {
-	d, err := api.newDataChannel(params, api.settingEngine.LoggerFactory.NewLogger("ortc"))
+	d, err := api.newDataChannel(params, nil, api.settingEngine.LoggerFactory.NewLogger("ortc"))
 	if err != nil {
 		return nil, err
 	}
@@ -82,13 +82,14 @@ func (api *API) NewDataChannel(transport *SCTPTransport, params *DataChannelPara
 
 // newDataChannel is an internal constructor for the data channel used to
 // create the DataChannel object before the networking is set up.
-func (api *API) newDataChannel(params *DataChannelParameters, log logging.LeveledLogger) (*DataChannel, error) {
+func (api *API) newDataChannel(params *DataChannelParameters, sctpTransport *SCTPTransport, log logging.LeveledLogger) (*DataChannel, error) {
 	// https://w3c.github.io/webrtc-pc/#peer-to-peer-data-api (Step #5)
 	if len(params.Label) > 65535 {
 		return nil, &rtcerr.TypeError{Err: ErrStringSizeLimit}
 	}
 
 	d := &DataChannel{
+		sctpTransport:     sctpTransport,
 		statsID:           fmt.Sprintf("DataChannel-%d", time.Now().UnixNano()),
 		label:             params.Label,
 		protocol:          params.Protocol,
diff --git a/peerconnection.go b/peerconnection.go
index 21d2b17d..41d04d4d 100644
--- a/peerconnection.go
+++ b/peerconnection.go
@@ -1991,7 +1991,7 @@ func (pc *PeerConnection) CreateDataChannel(label string, options *DataChannelIn
 		}
 	}
 
-	d, err := pc.api.newDataChannel(params, pc.log)
+	d, err := pc.api.newDataChannel(params, nil, pc.log)
 	if err != nil {
 		return nil, err
 	}
diff --git a/sctptransport.go b/sctptransport.go
index db29c3d7..9c449876 100644
--- a/sctptransport.go
+++ b/sctptransport.go
@@ -224,7 +224,7 @@ ACCEPT:
 			Ordered:           ordered,
 			MaxPacketLifeTime: maxPacketLifeTime,
 			MaxRetransmits:    maxRetransmits,
-		}, r.api.settingEngine.LoggerFactory.NewLogger("ortc"))
+		}, r, r.api.settingEngine.LoggerFactory.NewLogger("ortc"))
 		if err != nil {
 			r.log.Errorf("Failed to accept data channel: %v", err)
 			r.onError(err)


Test patch:
diff --git a/ortc_datachannel_test.go b/ortc_datachannel_test.go
index eb58bfac..e1b0c51b 100644
--- a/ortc_datachannel_test.go
+++ b/ortc_datachannel_test.go
@@ -12,6 +12,36 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func TestDataChannel_ORTC_SCTPTransport(t *testing.T) {
+	lim := test.TimeOut(time.Second * 20)
+	defer lim.Stop()
+
+	report := test.CheckRoutines(t)
+	defer report()
+
+	stackA, stackB, err := newORTCPair()
+	assert.NoError(t, err)
+
+	getSelectedCandidatePairErrChan := make(chan error)
+	stackB.sctp.OnDataChannel(func(d *DataChannel) {
+		_, getSelectedCandidatePairErr := d.Transport().Transport().ICETransport().GetSelectedCandidatePair()
+		getSelectedCandidatePairErrChan <- getSelectedCandidatePairErr
+	})
+
+	assert.NoError(t, signalORTCPair(stackA, stackB))
+
+	var id uint16 = 1
+	_, err = stackA.api.NewDataChannel(stackA.sctp, &DataChannelParameters{
+		Label: "Foo",
+		ID:    &id,
+	})
+	assert.NoError(t, err)
+
+	assert.NoError(t, <-getSelectedCandidatePairErrChan)
+	assert.NoError(t, stackA.close())
+	assert.NoError(t, stackB.close())
+}
+
 func TestDataChannel_ORTCE2E(t *testing.T) {
 	lim := test.TimeOut(time.Second * 20)
 	defer lim.Stop()