Repository: pion/webrtc
Pull request #2202 (2022-05-05 20:36:44)

Add json serialization support to ICECredentialType
#### Description
When serializing to/from json, the ICECredentialType should serialize as as a string instead of an enumeration.


codecov[bot]: # [Codecov](https://codecov.io/gh/pion/webrtc/pull/2202?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
> Merging [#2202](https://codecov.io/gh/pion/webrtc/pull/2202?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (074ff14) into [master](https://codecov.io/gh/pion/webrtc/commit/e6736962b7f34a8e322cecdcfba2d913a0f36a6e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (e673696) will **decrease** coverage by `7.04%`.
> The diff coverage is `66.66%`.

> :exclamation: Current head 074ff14 differs from pull request most recent head a09cf0a. Consider uploading reports for the commit a09cf0a to get more accurate results

```diff
@@            Coverage Diff             @@
##           master    #2202      +/-   ##
==========================================
- Coverage   76.95%   69.91%   -7.05%     
==========================================
  Files          87       64      -23     
  Lines        9009     3317    -5692     
==========================================
- Hits         6933     2319    -4614     
+ Misses       1650      870     -780     
+ Partials      426      128     -298     
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `?` | |
| wasm | `69.91% <66.66%> (-0.01%)` | :arrow_down: |

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/2202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [icecredentialtype.go](https://codecov.io/gh/pion/webrtc/pull/2202/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aWNlY3JlZGVudGlhbHR5cGUuZ28=) | `72.00% <66.66%> (-3.00%)` | :arrow_down: |
| [constants.go](https://codecov.io/gh/pion/webrtc/pull/2202/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: |
| [icecandidatepair.go](https://codecov.io/gh/pion/webrtc/pull/2202/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/2202/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: |
| [stats.go](https://codecov.io/gh/pion/webrtc/pull/2202/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%> (-75.00%)` | :arrow_down: |
| [track\_local.go](https://codecov.io/gh/pion/webrtc/pull/2202/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/2202/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/2202/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/2202/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: |
| [sdpsemantics.go](https://codecov.io/gh/pion/webrtc/pull/2202/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%> (-34.49%)` | :arrow_down: |
| ... and [38 more](https://codecov.io/gh/pion/webrtc/pull/2202/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/2202?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/2202?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Last update [e673696...a09cf0a](https://codecov.io/gh/pion/webrtc/pull/2202?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).

wdouglass: marking draft until i figure out the lint/testing stuff
wdouglass: So, as usually happens, a small change turned into a bigger one. I was not expecting this to have so many commits!

Tests are passing, this is ready for review. Squash if you like!

Patch:
diff --git a/errors.go b/errors.go
index e152b27c..6e482e9a 100644
--- a/errors.go
+++ b/errors.go
@@ -233,6 +233,9 @@ var (
 
 	errStatsICECandidateStateInvalid = errors.New("cannot convert to StatsICECandidatePairStateSucceeded invalid ice candidate state")
 
+	errInvalidICECredentialTypeString = errors.New("invalid ICECredentialType")
+	errInvalidICEServer               = errors.New("invalid ICEServer")
+
 	errICETransportNotInNew = errors.New("ICETransport can only be called in ICETransportStateNew")
 
 	errCertificatePEMFormatError = errors.New("bad Certificate PEM format")
diff --git a/icecredentialtype.go b/icecredentialtype.go
index 3967c16d..ef51a32a 100644
--- a/icecredentialtype.go
+++ b/icecredentialtype.go
@@ -1,5 +1,10 @@
 package webrtc
 
+import (
+	"encoding/json"
+	"fmt"
+)
+
 // ICECredentialType indicates the type of credentials used to connect to
 // an ICE server.
 type ICECredentialType int
@@ -7,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
+	ICECredentialTypePassword ICECredentialType = iota + 1
 
 	// ICECredentialTypeOauth describes token based credential as described
 	// in https://tools.ietf.org/html/rfc7635.
@@ -33,6 +38,8 @@ func newICECredentialType(raw string) ICECredentialType {
 
 func (t ICECredentialType) String() string {
 	switch t {
+	case Unknown:
+		return ""
 	case ICECredentialTypePassword:
 		return iceCredentialTypePasswordStr
 	case ICECredentialTypeOauth:
@@ -41,3 +48,26 @@ func (t ICECredentialType) String() string {
 		return ErrUnknownType.Error()
 	}
 }
+
+// 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)
+	}
+
+	*t = tmp
+	return nil
+}
+
+// MarshalJSON returns the JSON encoding
+func (t ICECredentialType) MarshalJSON() ([]byte, error) {
+	return json.Marshal(t.String())
+}
diff --git a/iceserver.go b/iceserver.go
index b83a9e8b..a1d3be1b 100644
--- a/iceserver.go
+++ b/iceserver.go
@@ -4,6 +4,8 @@
 package webrtc
 
 import (
+	"encoding/json"
+
 	"github.com/pion/ice/v2"
 	"github.com/pion/webrtc/v3/pkg/rtcerr"
 )
@@ -67,3 +69,111 @@ func (s ICEServer) urls() ([]*ice.URL, error) {
 
 	return urls, nil
 }
+
+func iceserverUnmarshalUrls(val interface{}) (*[]string, error) {
+	s, ok := val.([]interface{})
+	if !ok {
+		return nil, errInvalidICEServer
+	}
+	out := make([]string, len(s))
+	for idx, url := range s {
+		out[idx], ok = url.(string)
+		if !ok {
+			return nil, errInvalidICEServer
+		}
+	}
+	return &out, nil
+}
+
+func iceserverUnmarshalOauth(val interface{}) (*OAuthCredential, error) {
+	c, ok := val.(map[string]interface{})
+	if !ok {
+		return nil, errInvalidICEServer
+	}
+	MACKey, ok := c["MACKey"].(string)
+	if !ok {
+		return nil, errInvalidICEServer
+	}
+	AccessToken, ok := c["AccessToken"].(string)
+	if !ok {
+		return nil, errInvalidICEServer
+	}
+	return &OAuthCredential{
+		MACKey:      MACKey,
+		AccessToken: AccessToken,
+	}, nil
+}
+
+func (s *ICEServer) iceserverUnmarshalFields(m map[string]interface{}) error {
+	if val, ok := m["urls"]; ok {
+		u, err := iceserverUnmarshalUrls(val)
+		if err != nil {
+			return err
+		}
+		s.URLs = *u
+	} else {
+		s.URLs = []string{}
+	}
+
+	if val, ok := m["username"]; ok {
+		s.Username, ok = val.(string)
+		if !ok {
+			return errInvalidICEServer
+		}
+	}
+	if val, ok := m["credentialType"]; ok {
+		ct, ok := val.(string)
+		if !ok || (newICECredentialType(ct) == ICECredentialType(Unknown)) {
+			return errInvalidICEServer
+		}
+		s.CredentialType = newICECredentialType(ct)
+	} else {
+		s.CredentialType = ICECredentialType(Unknown)
+	}
+	if val, ok := m["credential"]; ok {
+		switch s.CredentialType {
+		case ICECredentialType(Unknown):
+			s.Credential = val
+		case ICECredentialTypePassword:
+			s.Credential = val
+		case ICECredentialTypeOauth:
+			c, err := iceserverUnmarshalOauth(val)
+			if err != nil {
+				return err
+			}
+			s.Credential = *c
+		default:
+			return errInvalidICECredentialTypeString
+		}
+	}
+	return nil
+}
+
+// UnmarshalJSON parses the JSON-encoded data and stores the result
+func (s *ICEServer) UnmarshalJSON(b []byte) error {
+	var tmp interface{}
+	err := json.Unmarshal(b, &tmp)
+	if err != nil {
+		return err
+	}
+	if m, ok := tmp.(map[string]interface{}); ok {
+		return s.iceserverUnmarshalFields(m)
+	}
+	return errInvalidICEServer
+}
+
+// MarshalJSON returns the JSON encoding
+func (s ICEServer) MarshalJSON() ([]byte, error) {
+	m := make(map[string]interface{})
+	m["urls"] = s.URLs
+	if s.Username != "" {
+		m["username"] = s.Username
+	}
+	if s.Credential != nil {
+		m["credential"] = s.Credential
+	}
+	if s.CredentialType != ICECredentialType(Unknown) {
+		m["credentialType"] = s.CredentialType
+	}
+	return json.Marshal(m)
+}
diff --git a/peerconnection_js.go b/peerconnection_js.go
index ef6b0227..4b0d538e 100644
--- a/peerconnection_js.go
+++ b/peerconnection_js.go
@@ -566,13 +566,33 @@ func iceServersToValue(iceServers []ICEServer) js.Value {
 	return js.ValueOf(maps)
 }
 
+func oauthCredentialToValue(o OAuthCredential) js.Value {
+	out := map[string]interface{}{
+		"MACKey":      o.MACKey,
+		"AccessToken": o.AccessToken,
+	}
+	return js.ValueOf(out)
+}
+
 func iceServerToValue(server ICEServer) js.Value {
-	return js.ValueOf(map[string]interface{}{
-		"urls":           stringsToValue(server.URLs), // required
-		"username":       stringToValueOrUndefined(server.Username),
-		"credential":     interfaceToValueOrUndefined(server.Credential),
-		"credentialType": stringEnumToValueOrUndefined(server.CredentialType.String()),
-	})
+	out := map[string]interface{}{
+		"urls": stringsToValue(server.URLs), // required
+	}
+	if server.Username != "" {
+		out["username"] = stringToValueOrUndefined(server.Username)
+	}
+	if server.Credential != nil {
+		switch t := server.Credential.(type) {
+		case string:
+			out["credential"] = stringToValueOrUndefined(t)
+		case OAuthCredential:
+			out["credential"] = oauthCredentialToValue(t)
+		}
+	}
+	if server.CredentialType != ICECredentialType(Unknown) {
+		out["credentialType"] = stringEnumToValueOrUndefined(server.CredentialType.String())
+	}
+	return js.ValueOf(out)
 }
 
 func valueToConfiguration(configValue js.Value) Configuration {
@@ -603,14 +623,36 @@ func valueToICEServers(iceServersValue js.Value) []ICEServer {
 	return iceServers
 }
 
+func valueToICECredential(iceCredentialValue js.Value) interface{} {
+	if iceCredentialValue.IsNull() || iceCredentialValue.IsUndefined() {
+		return nil
+	}
+	if iceCredentialValue.Type() == js.TypeString {
+		return iceCredentialValue.String()
+	}
+	if iceCredentialValue.Type() == js.TypeObject {
+		return OAuthCredential{
+			MACKey:      iceCredentialValue.Get("MACKey").String(),
+			AccessToken: iceCredentialValue.Get("AccessToken").String(),
+		}
+	}
+	return nil
+}
+
 func valueToICEServer(iceServerValue js.Value) ICEServer {
-	return ICEServer{
+	s := ICEServer{
 		URLs:     valueToStrings(iceServerValue.Get("urls")), // required
 		Username: valueToStringOrZero(iceServerValue.Get("username")),
 		// Note: Credential and CredentialType are not currently supported.
-		// Credential: iceServerValue.Get("credential"),
-		// CredentialType: newICECredentialType(valueToStringOrZero(iceServerValue.Get("credentialType"))),
+		Credential:     valueToICECredential(iceServerValue.Get("credential")),
+		CredentialType: newICECredentialType(valueToStringOrZero(iceServerValue.Get("credentialType"))),
+	}
+
+	// default to password
+	if s.CredentialType == ICECredentialType(Unknown) {
+		s.CredentialType = ICECredentialTypePassword
 	}
+	return s
 }
 
 func valueToICECandidate(val js.Value) *ICECandidate {


Test patch:
diff --git a/configuration_test.go b/configuration_test.go
index ce7c088d..f8f2bdcd 100644
--- a/configuration_test.go
+++ b/configuration_test.go
@@ -41,7 +41,7 @@ func TestConfiguration_getICEServers(t *testing.T) {
 
 func TestConfigurationJSON(t *testing.T) {
 	j := `{
-    "iceServers": [{"URLs": ["turn:turn.example.org"],
+    "iceServers": [{"urls": ["turn:turn.example.org"],
                     "username": "jch",
                     "credential": "topsecret"
                   }],
diff --git a/icecredentialtype_test.go b/icecredentialtype_test.go
index fd10c19a..a6fb4985 100644
--- a/icecredentialtype_test.go
+++ b/icecredentialtype_test.go
@@ -1,6 +1,7 @@
 package webrtc
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -41,3 +42,59 @@ func TestICECredentialType_String(t *testing.T) {
 		)
 	}
 }
+
+func TestICECredentialType_new(t *testing.T) {
+	testCases := []struct {
+		credentialType ICECredentialType
+		expectedString string
+	}{
+		{ICECredentialTypePassword, "password"},
+		{ICECredentialTypeOauth, "oauth"},
+	}
+
+	for i, testCase := range testCases {
+		assert.Equal(t,
+			newICECredentialType(testCase.expectedString),
+			testCase.credentialType,
+			"testCase: %d %v", i, testCase,
+		)
+	}
+}
+
+func TestICECredentialType_Json(t *testing.T) {
+	testCases := []struct {
+		credentialType     ICECredentialType
+		jsonRepresentation []byte
+	}{
+		{ICECredentialTypePassword, []byte("\"password\"")},
+		{ICECredentialTypeOauth, []byte("\"oauth\"")},
+	}
+
+	for i, testCase := range testCases {
+		m, err := json.Marshal(testCase.credentialType)
+		assert.NoError(t, err)
+		assert.Equal(t,
+			testCase.jsonRepresentation,
+			m,
+			"Marshal testCase: %d %v", i, testCase,
+		)
+		var ct ICECredentialType
+		err = json.Unmarshal(testCase.jsonRepresentation, &ct)
+		assert.NoError(t, err)
+		assert.Equal(t,
+			testCase.credentialType,
+			ct,
+			"Unmarshal testCase: %d %v", i, testCase,
+		)
+	}
+
+	{
+		ct := ICECredentialType(1000)
+		err := json.Unmarshal([]byte("\"invalid\""), &ct)
+		assert.Error(t, err)
+		assert.Equal(t, ct, ICECredentialType(1000))
+		err = json.Unmarshal([]byte("\"invalid"), &ct)
+		assert.Error(t, err)
+		assert.Equal(t, ct, ICECredentialType(1000))
+	}
+}
diff --git a/iceserver_test.go b/iceserver_test.go
index dafe1561..94e22374 100644
--- a/iceserver_test.go
+++ b/iceserver_test.go
@@ -4,6 +4,7 @@
 package webrtc
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/pion/ice/v2"
@@ -41,7 +42,13 @@ func TestICEServer_validate(t *testing.T) {
 		}
 
 		for i, testCase := range testCases {
-			_, err := testCase.iceServer.urls()
+			var iceServer ICEServer
+			jsonobj, err := json.Marshal(testCase.iceServer)
+			assert.NoError(t, err)
+			err = json.Unmarshal(jsonobj, &iceServer)
+			assert.NoError(t, err)
+			assert.Equal(t, iceServer, testCase.iceServer)
+			_, err = testCase.iceServer.urls()
 			assert.Nil(t, err, "testCase: %d %v", i, testCase)
 		}
 	})
@@ -88,4 +95,20 @@ func TestICEServer_validate(t *testing.T) {
 			)
 		}
 	})
+	t.Run("JsonFailure", func(t *testing.T) {
+		testCases := [][]byte{
+			[]byte(`{"urls":"NOTAURL","username":"unittest","credential":"placeholder","credentialType":"password"}`),
+			[]byte(`{"urls":["turn:[2001:db8:1234:5678::1]?transport=udp"],"username":"unittest","credential":"placeholder","credentialType":"invalid"}`),
+			[]byte(`{"urls":["turn:[2001:db8:1234:5678::1]?transport=udp"],"username":6,"credential":"placeholder","credentialType":"password"}`),
+			[]byte(`{"urls":["turn:192.158.29.39?transport=udp"],"username":"unittest","credential":{"Bad Object": true},"credentialType":"oauth"}`),
+			[]byte(`{"urls":["turn:192.158.29.39?transport=udp"],"username":"unittest","credential":{"MACKey":"WmtzanB3ZW9peFhtdm42NzUzNG0=","AccessToken":null,"credentialType":"oauth"}`),
+			[]byte(`{"urls":["turn:192.158.29.39?transport=udp"],"username":"unittest","credential":{"MACKey":"WmtzanB3ZW9peFhtdm42NzUzNG0=","AccessToken":null,"credentialType":"password"}`),
+			[]byte(`{"urls":["turn:192.158.29.39?transport=udp"],"username":"unittest","credential":{"MACKey":1337,"AccessToken":"AAwg3kPHWPfvk9bDFL936wYvkoctMADzQ5VhNDgeMR3+ZlZ35byg972fW8QjpEl7bx91YLBPFsIhsxloWcXPhA=="},"credentialType":"oauth"}`),
+		}
+		for i, testCase := range testCases {
+			var tc ICEServer
+			err := json.Unmarshal(testCase, &tc)
+			assert.Error(t, err, "testCase: %d %v", i, string(testCase))
+		}
+	})
 }
diff --git a/peerconnection_js_test.go b/peerconnection_js_test.go
index 6c7d8ebb..79ad0d63 100644
--- a/peerconnection_js_test.go
+++ b/peerconnection_js_test.go
@@ -69,3 +69,35 @@ func TestValueToICECandidate(t *testing.T) {
 		assert.Equal(t, testCase.expect, val)
 	}
 }
+
+func TestValueToICEServer(t *testing.T) {
+	testCases := []ICEServer{
+		{
+			URLs:           []string{"turn:192.158.29.39?transport=udp"},
+			Username:       "unittest",
+			Credential:     "placeholder",
+			CredentialType: ICECredentialTypePassword,
+		},
+		{
+			URLs:           []string{"turn:[2001:db8:1234:5678::1]?transport=udp"},
+			Username:       "unittest",
+			Credential:     "placeholder",
+			CredentialType: ICECredentialTypePassword,
+		},
+		{
+			URLs:     []string{"turn:192.158.29.39?transport=udp"},
+			Username: "unittest",
+			Credential: OAuthCredential{
+				MACKey:      "WmtzanB3ZW9peFhtdm42NzUzNG0=",
+				AccessToken: "AAwg3kPHWPfvk9bDFL936wYvkoctMADzQ5VhNDgeMR3+ZlZ35byg972fW8QjpEl7bx91YLBPFsIhsxloWcXPhA==",
+			},
+			CredentialType: ICECredentialTypeOauth,
+		},
+	}
+
+	for _, testCase := range testCases {
+		v := iceServerToValue(testCase)
+		s := valueToICEServer(v)
+		assert.Equal(t, testCase, s)
+	}
+}