[tor-commits] [snowflake/master] Have util.{Serialize, Deserialize}SessionDescription return an error

[dcf at torproject.org]

commit b48fb781ee15cf033efc61496746a295dc0d63c7
Author: David Fifield <david at bamsoftware.com>
Date:   Mon Apr 27 18:45:10 2020 -0600

    Have util.{Serialize,Deserialize}SessionDescription return an error
    
    https://bugs.torproject.org/33897#comment:4
---
 client/lib/lib_test.go   | 11 ++++++++---
 client/lib/rendezvous.go |  9 ++++++---
 common/util/util.go      | 33 ++++++++++++---------------------
 3 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/client/lib/lib_test.go b/client/lib/lib_test.go
index 50a211d..41d9cf9 100644
--- a/client/lib/lib_test.go
+++ b/client/lib/lib_test.go
@@ -231,7 +231,8 @@ func TestSnowflakeClient(t *testing.T) {
 				So(err, ShouldBeNil)
 
 				c.offerChannel <- nil
-				answer := util.DeserializeSessionDescription(sampleAnswer)
+				answer, err := util.DeserializeSessionDescription(sampleAnswer)
+				So(err, ShouldBeNil)
 				So(answer, ShouldNotBeNil)
 				c.answerChannel <- answer
 				err = c.exchangeSDP()
@@ -256,7 +257,8 @@ func TestSnowflakeClient(t *testing.T) {
 					ctx.So(err, ShouldBeNil)
 					wg.Done()
 				}()
-				answer := util.DeserializeSessionDescription(sampleAnswer)
+				answer, err := util.DeserializeSessionDescription(sampleAnswer)
+				So(err, ShouldBeNil)
 				c.answerChannel <- answer
 				wg.Wait()
 			})
@@ -286,7 +288,10 @@ func TestSnowflakeClient(t *testing.T) {
 			http.StatusOK,
 			[]byte(`{"type":"answer","sdp":"fake"}`),
 		}
-		fakeOffer := util.DeserializeSessionDescription(`{"type":"offer","sdp":"test"}`)
+		fakeOffer, err := util.DeserializeSessionDescription(`{"type":"offer","sdp":"test"}`)
+		if err != nil {
+			panic(err)
+		}
 
 		Convey("Construct BrokerChannel with no front domain", func() {
 			b, err := NewBrokerChannel("test.broker", "", transport, false)
diff --git a/client/lib/rendezvous.go b/client/lib/rendezvous.go
index f236c47..85e25d2 100644
--- a/client/lib/rendezvous.go
+++ b/client/lib/rendezvous.go
@@ -96,7 +96,11 @@ func (bc *BrokerChannel) Negotiate(offer *webrtc.SessionDescription) (
 			SDP:  util.StripLocalAddresses(offer.SDP),
 		}
 	}
-	data := bytes.NewReader([]byte(util.SerializeSessionDescription(offer)))
+	offerSDP, err := util.SerializeSessionDescription(offer)
+	if err != nil {
+		return nil, err
+	}
+	data := bytes.NewReader([]byte(offerSDP))
 	// Suffix with broker's client registration handler.
 	clientURL := bc.url.ResolveReference(&url.URL{Path: "client"})
 	request, err := http.NewRequest("POST", clientURL.String(), data)
@@ -119,8 +123,7 @@ func (bc *BrokerChannel) Negotiate(offer *webrtc.SessionDescription) (
 		if nil != err {
 			return nil, err
 		}
-		answer := util.DeserializeSessionDescription(string(body))
-		return answer, nil
+		return util.DeserializeSessionDescription(string(body))
 	case http.StatusServiceUnavailable:
 		return nil, errors.New(BrokerError503)
 	case http.StatusBadRequest:
diff --git a/common/util/util.go b/common/util/util.go
index fa62fd7..ac254fa 100644
--- a/common/util/util.go
+++ b/common/util/util.go
@@ -2,43 +2,38 @@ package util
 
 import (
 	"encoding/json"
-	"log"
+	"errors"
 	"net"
 
 	"github.com/pion/sdp/v2"
 	"github.com/pion/webrtc/v2"
 )
 
-func SerializeSessionDescription(desc *webrtc.SessionDescription) string {
+func SerializeSessionDescription(desc *webrtc.SessionDescription) (string, error) {
 	bytes, err := json.Marshal(*desc)
-	if nil != err {
-		log.Println(err)
-		return ""
+	if err != nil {
+		return "", err
 	}
-	return string(bytes)
+	return string(bytes), nil
 }
 
-func DeserializeSessionDescription(msg string) *webrtc.SessionDescription {
+func DeserializeSessionDescription(msg string) (*webrtc.SessionDescription, error) {
 	var parsed map[string]interface{}
 	err := json.Unmarshal([]byte(msg), &parsed)
-	if nil != err {
-		log.Println(err)
-		return nil
+	if err != nil {
+		return nil, err
 	}
 	if _, ok := parsed["type"]; !ok {
-		log.Println("Cannot deserialize SessionDescription without type field.")
-		return nil
+		return nil, errors.New("cannot deserialize SessionDescription without type field")
 	}
 	if _, ok := parsed["sdp"]; !ok {
-		log.Println("Cannot deserialize SessionDescription without sdp field.")
-		return nil
+		return nil, errors.New("cannot deserialize SessionDescription without sdp field")
 	}
 
 	var stype webrtc.SDPType
 	switch parsed["type"].(string) {
 	default:
-		log.Println("Unknown SDP type")
-		return nil
+		return nil, errors.New("Unknown SDP type")
 	case "offer":
 		stype = webrtc.SDPTypeOffer
 	case "pranswer":
@@ -49,14 +44,10 @@ func DeserializeSessionDescription(msg string) *webrtc.SessionDescription {
 		stype = webrtc.SDPTypeRollback
 	}
 
-	if err != nil {
-		log.Println(err)
-		return nil
-	}
 	return &webrtc.SessionDescription{
 		Type: stype,
 		SDP:  parsed["sdp"].(string),
-	}
+	}, nil
 }
 
 // Stolen from https://github.com/golang/go/pull/30278