Repository: pion/webrtc (go)

Issue:
ICEServer and RTCConfiguration cannot be marshaled to JSON
Consider the following:
```
func TestConfigurationJSONMarshal(t *testing.T) {
	j := `{
    "iceServers": [{"URLs": ["turn:turn.example.org"],
                    "username": "jch",
                    "credential": "topsecret",
                  }],
    "iceTransportPolicy": "relay",
    "bundlePolicy": "balanced",
    "rtcpMuxPolicy": "require"
}`

	conf := Configuration{
		ICEServers: []ICEServer{
			{
				URLs:       []string{"turn:turn.example.org"},
				Username:   "jch",
				Credential: "topsecret",
			},
		},
		ICETransportPolicy: ICETransportPolicyRelay,
		BundlePolicy:       BundlePolicyBalanced,
		RTCPMuxPolicy:      RTCPMuxPolicyRequire,
	}

	var conf2 Configuration
	err := json.Unmarshal([]byte(j), &conf2)
	assert.NoError(t, err)
	assert.Equal(t, conf, conf2)
}
```
This fails horribly:
```
--- FAIL: TestConfigurationJSONMarshal (0.00s)
    configuration_test.go:68: 
        	Error Trace:	configuration_test.go:68
        	Error:      	Received unexpected error:
        	            	invalid character '}' looking for beginning of object key string
        	Test:       	TestConfigurationJSONMarshal
    configuration_test.go:69: 
        	Error Trace:	configuration_test.go:69
        	Error:      	Not equal: 
        	            	expected: webrtc.Configuration{ICEServers:[]webrtc.ICEServer{webrtc.ICEServer{URLs:[]string{"turn:turn.example.org"}, Username:"jch", Credential:"topsecret", CredentialType:0}}, ICETransportPolicy:1, BundlePolicy:1, RTCPMuxPolicy:2, PeerIdentity:"", Certificates:[]webrtc.Certificate(nil), ICECandidatePoolSize:0x0, SDPSemantics:0}
        	            	actual  : webrtc.Configuration{ICEServers:[]webrtc.ICEServer(nil), ICETransportPolicy:0, BundlePolicy:0, RTCPMuxPolicy:0, PeerIdentity:"", Certificates:[]webrtc.Certificate(nil), ICECandidatePoolSize:0x0, SDPSemantics:0}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,15 +1,6 @@
        	            	 (webrtc.Configuration) {
        	            	- ICEServers: ([]webrtc.ICEServer) (len=1) {
        	            	-  (webrtc.ICEServer) {
        	            	-   URLs: ([]string) (len=1) {
        	            	-    (string) (len=21) "turn:turn.example.org"
        	            	-   },
        	            	-   Username: (string) (len=3) "jch",
        	            	-   Credential: (string) (len=9) "topsecret",
        	            	-   CredentialType: (webrtc.ICECredentialType) 0
        	            	-  }
        	            	- },
        	            	- ICETransportPolicy: (webrtc.ICETransportPolicy) 1,
        	            	- BundlePolicy: (webrtc.BundlePolicy) 1,
        	            	- RTCPMuxPolicy: (webrtc.RTCPMuxPolicy) 2,
        	            	+ ICEServers: ([]webrtc.ICEServer) <nil>,
        	            	+ ICETransportPolicy: (webrtc.ICETransportPolicy) 0,
        	            	+ BundlePolicy: (webrtc.BundlePolicy) 0,
        	            	+ RTCPMuxPolicy: (webrtc.RTCPMuxPolicy) 0,
        	            	  PeerIdentity: (string) "",
        	Test:       	TestConfigurationJSONMarshal
FAIL
```
There are two issues here:
  * the types `Configuration` and `ICEServer` do not have the required `json` tags;
  * a number of fields are numeric instead of being strings.

How do people want this fixed?  I can either make the relevant fields into strings, or write custom marshallers/unmarshallers.  The alternative would be to work around this issue in client code, but I find that distasteful.

Pull request:
JSON marshalling/unmarshalling for Configuration
Add UnmarshalJSON and MarshalJSON implementations and tests

Resolves #1616 



Patch:
diff --git a/bundlepolicy.go b/bundlepolicy.go
index 91e6f979..6d39a277 100644
--- a/bundlepolicy.go
+++ b/bundlepolicy.go
@@ -1,5 +1,9 @@
 package webrtc
 
+import (
+	"encoding/json"
+)
+
 // BundlePolicy affects which media tracks are negotiated if the remote
 // endpoint is not bundle-aware, and what ICE candidates are gathered. If the
 // remote endpoint is bundle-aware, all media tracks and data channels are
@@ -56,3 +60,19 @@ func (t BundlePolicy) String() string {
 		return ErrUnknownType.Error()
 	}
 }
+
+// UnmarshalJSON parses the JSON-encoded data and stores the result
+func (t *BundlePolicy) UnmarshalJSON(b []byte) error {
+	var val string
+	if err := json.Unmarshal(b, &val); err != nil {
+		return err
+	}
+
+	*t = newBundlePolicy(val)
+	return nil
+}
+
+// MarshalJSON returns the JSON encoding
+func (t BundlePolicy) MarshalJSON() ([]byte, error) {
+	return json.Marshal(t.String())
+}
diff --git a/configuration.go b/configuration.go
index 11addcf7..056fec5f 100644
--- a/configuration.go
+++ b/configuration.go
@@ -10,24 +10,24 @@ package webrtc
 type Configuration struct {
 	// ICEServers defines a slice describing servers available to be used by
 	// ICE, such as STUN and TURN servers.
-	ICEServers []ICEServer
+	ICEServers []ICEServer `json:"iceServers,omitempty"`
 
 	// ICETransportPolicy indicates which candidates the ICEAgent is allowed
 	// to use.
-	ICETransportPolicy ICETransportPolicy
+	ICETransportPolicy ICETransportPolicy `json:"iceTransportPolicy,omitempty"`
 
 	// BundlePolicy indicates which media-bundling policy to use when gathering
 	// ICE candidates.
-	BundlePolicy BundlePolicy
+	BundlePolicy BundlePolicy `json:"bundlePolicy,omitempty"`
 
 	// RTCPMuxPolicy indicates which rtcp-mux policy to use when gathering ICE
 	// candidates.
-	RTCPMuxPolicy RTCPMuxPolicy
+	RTCPMuxPolicy RTCPMuxPolicy `json:"rtcpMuxPolicy,omitempty"`
 
 	// PeerIdentity sets the target peer identity for the PeerConnection.
 	// The PeerConnection will not establish a connection to a remote peer
 	// unless it can be successfully authenticated with the provided name.
-	PeerIdentity string
+	PeerIdentity string `json:"peerIdentity"`
 
 	// Certificates describes a set of certificates that the PeerConnection
 	// uses to authenticate. Valid values for this parameter are created
@@ -40,12 +40,12 @@ type Configuration struct {
 	// used for a given connection; how certificates are selected is outside
 	// the scope of this specification. If this value is absent, then a default
 	// set of certificates is generated for each PeerConnection instance.
-	Certificates []Certificate
+	Certificates []Certificate `json:"certificates,omitempty"`
 
 	// ICECandidatePoolSize describes the size of the prefetched ICE pool.
-	ICECandidatePoolSize uint8
+	ICECandidatePoolSize uint8 `json:"iceCandidatePoolSize,omitempty"`
 
 	// SDPSemantics controls the type of SDP offers accepted by and
 	// SDP answers generated by the PeerConnection.
-	SDPSemantics SDPSemantics
+	SDPSemantics SDPSemantics `json:"sdpSemantics,omitempty"`
 }
diff --git a/iceserver.go b/iceserver.go
index abb51994..76146ece 100644
--- a/iceserver.go
+++ b/iceserver.go
@@ -10,10 +10,10 @@ import (
 // ICEServer describes a single STUN and TURN server that can be used by
 // the ICEAgent to establish a connection with a peer.
 type ICEServer struct {
-	URLs           []string
-	Username       string
-	Credential     interface{}
-	CredentialType ICECredentialType
+	URLs           []string          `json:"urls"`
+	Username       string            `json:"username,omitempty"`
+	Credential     interface{}       `json:"credential,omitempty"`
+	CredentialType ICECredentialType `json:"credentialType,omitempty"`
 }
 
 func (s ICEServer) parseURL(i int) (*ice.URL, error) {
diff --git a/iceserver_js.go b/iceserver_js.go
index c7f249e0..07b2dcba 100644
--- a/iceserver_js.go
+++ b/iceserver_js.go
@@ -13,9 +13,9 @@ import (
 type ICEServer struct {
 	URLs     []string
 	Username string
-	// Note: Credential and CredentialType are not supported.
-	// Credential     interface{}
-	// CredentialType ICECredentialType
+	// Note: TURN is not supported in the WASM bindings yet
+	Credential     interface{}
+	CredentialType ICECredentialType
 }
 
 func (s ICEServer) parseURL(i int) (*ice.URL, error) {
@@ -32,27 +32,6 @@ func (s ICEServer) validate() ([]*ice.URL, error) {
 		}
 
 		if url.Scheme == ice.SchemeTypeTURN || url.Scheme == ice.SchemeTypeTURNS {
-			// // https://www.w3.org/TR/webrtc/#set-the-configuration (step #11.3.2)
-			// if s.Username == "" || s.Credential == nil {
-			// 	return nil, &rtcerr.InvalidAccessError{Err: ErrNoTurnCredentials}
-			// }
-
-			// switch s.CredentialType {
-			// case ICECredentialTypePassword:
-			// 	// https://www.w3.org/TR/webrtc/#set-the-configuration (step #11.3.3)
-			// 	if _, ok := s.Credential.(string); !ok {
-			// 		return nil, &rtcerr.InvalidAccessError{Err: ErrTurnCredentials}
-			// 	}
-
-			// case ICECredentialTypeOauth:
-			// 	// https://www.w3.org/TR/webrtc/#set-the-configuration (step #11.3.4)
-			// 	if _, ok := s.Credential.(OAuthCredential); !ok {
-			// 		return nil, &rtcerr.InvalidAccessError{Err: ErrTurnCredentials}
-			// 	}
-
-			// default:
-			// 	return nil, &rtcerr.InvalidAccessError{Err: ErrTurnCredentials}
-			// }
 			return nil, errors.New("TURN is not currently supported in the JavaScript/Wasm bindings")
 		}
 
diff --git a/icetransportpolicy.go b/icetransportpolicy.go
index 679de978..16273f56 100644
--- a/icetransportpolicy.go
+++ b/icetransportpolicy.go
@@ -1,5 +1,9 @@
 package webrtc
 
+import (
+	"encoding/json"
+)
+
 // ICETransportPolicy defines the ICE candidate policy surface the
 // permitted candidates. Only these candidates are used for connectivity checks.
 type ICETransportPolicy int
@@ -44,3 +48,18 @@ func (t ICETransportPolicy) String() string {
 		return ErrUnknownType.Error()
 	}
 }
+
+// UnmarshalJSON parses the JSON-encoded data and stores the result
+func (t *ICETransportPolicy) UnmarshalJSON(b []byte) error {
+	var val string
+	if err := json.Unmarshal(b, &val); err != nil {
+		return err
+	}
+	*t = NewICETransportPolicy(val)
+	return nil
+}
+
+// MarshalJSON returns the JSON encoding
+func (t ICETransportPolicy) MarshalJSON() ([]byte, error) {
+	return json.Marshal(t.String())
+}
diff --git a/rtcpmuxpolicy.go b/rtcpmuxpolicy.go
index d3fece1a..f74e440b 100644
--- a/rtcpmuxpolicy.go
+++ b/rtcpmuxpolicy.go
@@ -1,5 +1,9 @@
 package webrtc
 
+import (
+	"encoding/json"
+)
+
 // RTCPMuxPolicy affects what ICE candidates are gathered to support
 // non-multiplexed RTCP.
 type RTCPMuxPolicy int
@@ -44,3 +48,19 @@ func (t RTCPMuxPolicy) String() string {
 		return ErrUnknownType.Error()
 	}
 }
+
+// UnmarshalJSON parses the JSON-encoded data and stores the result
+func (t *RTCPMuxPolicy) UnmarshalJSON(b []byte) error {
+	var val string
+	if err := json.Unmarshal(b, &val); err != nil {
+		return err
+	}
+
+	*t = newRTCPMuxPolicy(val)
+	return nil
+}
+
+// MarshalJSON returns the JSON encoding
+func (t RTCPMuxPolicy) MarshalJSON() ([]byte, error) {
+	return json.Marshal(t.String())
+}
diff --git a/sdpsemantics.go b/sdpsemantics.go
index 4d4fce80..b8d396c9 100644
--- a/sdpsemantics.go
+++ b/sdpsemantics.go
@@ -1,5 +1,9 @@
 package webrtc
 
+import (
+	"encoding/json"
+)
+
 // SDPSemantics determines which style of SDP offers and answers
 // can be used
 type SDPSemantics int
@@ -27,6 +31,19 @@ const (
 	sdpSemanticsPlanB                   = "plan-b"
 )
 
+func newSDPSemantics(raw string) SDPSemantics {
+	switch raw {
+	case sdpSemanticsUnifiedPlan:
+		return SDPSemanticsUnifiedPlan
+	case sdpSemanticsPlanB:
+		return SDPSemanticsPlanB
+	case sdpSemanticsUnifiedPlanWithFallback:
+		return SDPSemanticsUnifiedPlanWithFallback
+	default:
+		return SDPSemantics(Unknown)
+	}
+}
+
 func (s SDPSemantics) String() string {
 	switch s {
 	case SDPSemanticsUnifiedPlanWithFallback:
@@ -39,3 +56,19 @@ func (s SDPSemantics) String() string {
 		return ErrUnknownType.Error()
 	}
 }
+
+// UnmarshalJSON parses the JSON-encoded data and stores the result
+func (s *SDPSemantics) UnmarshalJSON(b []byte) error {
+	var val string
+	if err := json.Unmarshal(b, &val); err != nil {
+		return err
+	}
+
+	*s = newSDPSemantics(val)
+	return nil
+}
+
+// MarshalJSON returns the JSON encoding
+func (s SDPSemantics) MarshalJSON() ([]byte, error) {
+	return json.Marshal(s.String())
+}


Test patch:
diff --git a/configuration_test.go b/configuration_test.go
index 9f767298..ce7c088d 100644
--- a/configuration_test.go
+++ b/configuration_test.go
@@ -1,6 +1,7 @@
 package webrtc
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -37,3 +38,39 @@ func TestConfiguration_getICEServers(t *testing.T) {
 		assert.Equal(t, expectedServerStr, parsedURLs[0].URLs[0])
 	})
 }
+
+func TestConfigurationJSON(t *testing.T) {
+	j := `{
+    "iceServers": [{"URLs": ["turn:turn.example.org"],
+                    "username": "jch",
+                    "credential": "topsecret"
+                  }],
+    "iceTransportPolicy": "relay",
+    "bundlePolicy": "balanced",
+    "rtcpMuxPolicy": "require"
+}`
+
+	conf := Configuration{
+		ICEServers: []ICEServer{
+			{
+				URLs:       []string{"turn:turn.example.org"},
+				Username:   "jch",
+				Credential: "topsecret",
+			},
+		},
+		ICETransportPolicy: ICETransportPolicyRelay,
+		BundlePolicy:       BundlePolicyBalanced,
+		RTCPMuxPolicy:      RTCPMuxPolicyRequire,
+	}
+
+	var conf2 Configuration
+	assert.NoError(t, json.Unmarshal([]byte(j), &conf2))
+	assert.Equal(t, conf, conf2)
+
+	j2, err := json.Marshal(conf2)
+	assert.NoError(t, err)
+
+	var conf3 Configuration
+	assert.NoError(t, json.Unmarshal(j2, &conf3))
+	assert.Equal(t, conf2, conf3)
+}