Repository: pion/webrtc
Pull request #2213 (2022-05-11 00:19:02)

Fix the zero value of ICECredentialType
Ensure that the zero value of ICECredentialType is password.


codecov[bot]: # [Codecov](https://codecov.io/gh/pion/webrtc/pull/2213?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
> Merging [#2213](https://codecov.io/gh/pion/webrtc/pull/2213?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (a293f08) into [master](https://codecov.io/gh/pion/webrtc/commit/37a145a51fa5174c9c8093f355dafeb1d8f6b639?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (37a145a) will **increase** coverage by `1.65%`.
> The diff coverage is `92.30%`.

```diff
@@            Coverage Diff             @@
##           master    #2213      +/-   ##
==========================================
+ Coverage   77.10%   78.75%   +1.65%     
==========================================
  Files          87       76      -11     
  Lines        9153     8253     -900     
==========================================
- Hits         7057     6500     -557     
+ Misses       1666     1365     -301     
+ Partials      430      388      -42     
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `78.75% <92.30%> (-0.07%)` | :arrow_down: |
| wasm | `?` | |

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/2213?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [iceserver.go](https://codecov.io/gh/pion/webrtc/pull/2213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aWNlc2VydmVyLmdv) | `82.30% <83.33%> (-3.63%)` | :arrow_down: |
| [icecredentialtype.go](https://codecov.io/gh/pion/webrtc/pull/2213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aWNlY3JlZGVudGlhbHR5cGUuZ28=) | `83.33% <100.00%> (-1.52%)` | :arrow_down: |
| [sctptransport.go](https://codecov.io/gh/pion/webrtc/pull/2213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-c2N0cHRyYW5zcG9ydC5nbw==) | `80.52% <0.00%> (-0.75%)` | :arrow_down: |
| [rtptransceiver\_js.go](https://codecov.io/gh/pion/webrtc/pull/2213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cnRwdHJhbnNjZWl2ZXJfanMuZ28=) | | |
| [iceserver\_js.go](https://codecov.io/gh/pion/webrtc/pull/2213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aWNlc2VydmVyX2pzLmdv) | | |
| [js\_utils.go](https://codecov.io/gh/pion/webrtc/pull/2213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-anNfdXRpbHMuZ28=) | | |
| [datachannel\_js.go](https://codecov.io/gh/pion/webrtc/pull/2213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-ZGF0YWNoYW5uZWxfanMuZ28=) | | |
| [icetransport\_js.go](https://codecov.io/gh/pion/webrtc/pull/2213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aWNldHJhbnNwb3J0X2pzLmdv) | | |
| [dtlstransport\_js.go](https://codecov.io/gh/pion/webrtc/pull/2213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-ZHRsc3RyYW5zcG9ydF9qcy5nbw==) | | |
| ... and [5 more](https://codecov.io/gh/pion/webrtc/pull/2213/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/webrtc/pull/2213?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/webrtc/pull/2213?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Last update [37a145a...a293f08](https://codecov.io/gh/pion/webrtc/pull/2213?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).


Patch:
diff --git a/icecredentialtype.go b/icecredentialtype.go
index ef51a32a..b16a5d5c 100644
--- a/icecredentialtype.go
+++ b/icecredentialtype.go
@@ -12,7 +12,7 @@ type ICECredentialType int
 const (
 	// ICECredentialTypePassword describes username and password based
 	// credentials as described in https://tools.ietf.org/html/rfc5389.
-	ICECredentialTypePassword ICECredentialType = iota + 1
+	ICECredentialTypePassword ICECredentialType = iota
 
 	// ICECredentialTypeOauth describes token based credential as described
 	// in https://tools.ietf.org/html/rfc7635.
@@ -25,21 +25,19 @@ const (
 	iceCredentialTypeOauthStr    = "oauth"
 )
 
-func newICECredentialType(raw string) ICECredentialType {
+func newICECredentialType(raw string) (ICECredentialType, error) {
 	switch raw {
 	case iceCredentialTypePasswordStr:
-		return ICECredentialTypePassword
+		return ICECredentialTypePassword, nil
 	case iceCredentialTypeOauthStr:
-		return ICECredentialTypeOauth
+		return ICECredentialTypeOauth, nil
 	default:
-		return ICECredentialType(Unknown)
+		return ICECredentialTypePassword, errInvalidICECredentialTypeString
 	}
 }
 
 func (t ICECredentialType) String() string {
 	switch t {
-	case Unknown:
-		return ""
 	case ICECredentialTypePassword:
 		return iceCredentialTypePasswordStr
 	case ICECredentialTypeOauth:
@@ -52,15 +50,13 @@ func (t ICECredentialType) String() string {
 // UnmarshalJSON parses the JSON-encoded data and stores the result
 func (t *ICECredentialType) UnmarshalJSON(b []byte) error {
 	var val string
-	var tmp ICECredentialType
 	if err := json.Unmarshal(b, &val); err != nil {
 		return err
 	}
 
-	tmp = newICECredentialType(val)
-
-	if (tmp == ICECredentialType(Unknown)) && (val != "") {
-		return fmt.Errorf("%w: (%s)", errInvalidICECredentialTypeString, val)
+	tmp, err := newICECredentialType(val)
+	if err != nil {
+		return fmt.Errorf("%w: (%s)", err, val)
 	}
 
 	*t = tmp
diff --git a/iceserver.go b/iceserver.go
index a1d3be1b..c9a9bc6b 100644
--- a/iceserver.go
+++ b/iceserver.go
@@ -123,17 +123,19 @@ func (s *ICEServer) iceserverUnmarshalFields(m map[string]interface{}) error {
 	}
 	if val, ok := m["credentialType"]; ok {
 		ct, ok := val.(string)
-		if !ok || (newICECredentialType(ct) == ICECredentialType(Unknown)) {
+		if !ok {
 			return errInvalidICEServer
 		}
-		s.CredentialType = newICECredentialType(ct)
+		tpe, err := newICECredentialType(ct)
+		if err != nil {
+			return err
+		}
+		s.CredentialType = tpe
 	} else {
-		s.CredentialType = ICECredentialType(Unknown)
+		s.CredentialType = ICECredentialTypePassword
 	}
 	if val, ok := m["credential"]; ok {
 		switch s.CredentialType {
-		case ICECredentialType(Unknown):
-			s.Credential = val
 		case ICECredentialTypePassword:
 			s.Credential = val
 		case ICECredentialTypeOauth:
@@ -172,8 +174,6 @@ func (s ICEServer) MarshalJSON() ([]byte, error) {
 	if s.Credential != nil {
 		m["credential"] = s.Credential
 	}
-	if s.CredentialType != ICECredentialType(Unknown) {
-		m["credentialType"] = s.CredentialType
-	}
+	m["credentialType"] = s.CredentialType
 	return json.Marshal(m)
 }
diff --git a/peerconnection_js.go b/peerconnection_js.go
index 4b0d538e..71d2e9d2 100644
--- a/peerconnection_js.go
+++ b/peerconnection_js.go
@@ -640,12 +640,16 @@ func valueToICECredential(iceCredentialValue js.Value) interface{} {
 }
 
 func valueToICEServer(iceServerValue js.Value) ICEServer {
+	tpe, err := newICECredentialType(valueToStringOrZero(iceServerValue.Get("credentialType")))
+	if err != nil {
+		tpe = ICECredentialTypePassword
+	}
 	s := ICEServer{
 		URLs:     valueToStrings(iceServerValue.Get("urls")), // required
 		Username: valueToStringOrZero(iceServerValue.Get("username")),
 		// Note: Credential and CredentialType are not currently supported.
 		Credential:     valueToICECredential(iceServerValue.Get("credential")),
-		CredentialType: newICECredentialType(valueToStringOrZero(iceServerValue.Get("credentialType"))),
+		CredentialType: tpe,
 	}
 
 	// default to password


Test patch:
diff --git a/icecredentialtype_test.go b/icecredentialtype_test.go
index a6fb4985..42647710 100644
--- a/icecredentialtype_test.go
+++ b/icecredentialtype_test.go
@@ -17,9 +17,10 @@ func TestNewICECredentialType(t *testing.T) {
 	}
 
 	for i, testCase := range testCases {
+		tpe, err := newICECredentialType(testCase.credentialTypeString)
+		assert.NoError(t, err)
 		assert.Equal(t,
-			testCase.expectedCredentialType,
-			newICECredentialType(testCase.credentialTypeString),
+			testCase.expectedCredentialType, tpe,
 			"testCase: %d %v", i, testCase,
 		)
 	}
@@ -53,9 +54,10 @@ func TestICECredentialType_new(t *testing.T) {
 	}
 
 	for i, testCase := range testCases {
+		tpe, err := newICECredentialType(testCase.expectedString)
+		assert.NoError(t, err)
 		assert.Equal(t,
-			newICECredentialType(testCase.expectedString),
-			testCase.credentialType,
+			tpe, testCase.credentialType,
 			"testCase: %d %v", i, testCase,
 		)
 	}
diff --git a/iceserver_test.go b/iceserver_test.go
index 94e22374..4814f8d7 100644
--- a/iceserver_test.go
+++ b/iceserver_test.go
@@ -112,3 +112,12 @@ func TestICEServer_validate(t *testing.T) {
 		}
 	})
 }
+
+func TestICEServerZeroValue(t *testing.T) {
+	server := ICEServer{
+		URLs:       []string{"turn:galene.org:1195"},
+		Username:   "galene",
+		Credential: "secret",
+	}
+	assert.Equal(t, server.CredentialType, ICECredentialTypePassword)
+}