Split ICE and DTLS related SDP parsing out

Move this stuff out of SetRemoteDescription so it will be easier to test

Relates to #1023

errors.go

@@ -91,4 +91,24 @@ var (
 	// ErrSenderNotCreatedByConnection indicates RemoveTrack was called with a RtpSender not created
 	// by this PeerConnection
 	ErrSenderNotCreatedByConnection = errors.New("RtpSender not created by this PeerConnection")
+
+	// ErrSessionDescriptionNoFingerprint indicates SetRemoteDescription was called with a SessionDescription that has no
+	// fingerprint
+	ErrSessionDescriptionNoFingerprint = errors.New("SetRemoteDescription called with no fingerprint")
+
+	// ErrSessionDescriptionInvalidFingerprint indicates SetRemoteDescription was called with a SessionDescription that
+	// has an invalid fingerprint
+	ErrSessionDescriptionInvalidFingerprint = errors.New("SetRemoteDescription called with an invalid fingerprint")
+
+	// ErrSessionDescriptionConflictingFingerprints indicates SetRemoteDescription was called with a SessionDescription that
+	// has an conflicting fingerprints
+	ErrSessionDescriptionConflictingFingerprints = errors.New("SetRemoteDescription called with multiple conflicting fingerprint")
+
+	// ErrSessionDescriptionMissingIceUfrag indicates SetRemoteDescription was called with a SessionDescription that
+	// is missing an ice-ufrag value
+	ErrSessionDescriptionMissingIceUfrag = errors.New("SetRemoteDescription called with no ice-ufrag")
+
+	// ErrSessionDescriptionMissingIcePwd indicates SetRemoteDescription was called with a SessionDescription that
+	// is missing an ice-pwd value
+	ErrSessionDescriptionMissingIcePwd = errors.New("SetRemoteDescription called with no ice-pwd")
 )

peerconnection.go

@@ -820,6 +820,7 @@ func (pc *PeerConnection) SetRemoteDescription(desc SessionDescription) error {
 
 	currentTransceivers := pc.GetTransceivers()
 	haveRemoteDescription := pc.currentRemoteDescription != nil
+
 	desc.parsed = &sdp.SessionDescription{}
 	if err := desc.parsed.Unmarshal([]byte(desc.SDP)); err != nil {
 		return err
@@ -834,57 +835,31 @@ func (pc *PeerConnection) SetRemoteDescription(desc SessionDescription) error {
 	}
 
 	weOffer := true
-	remoteUfrag := ""
-	remotePwd := ""
 	if desc.Type == SDPTypeOffer {
 		weOffer = false
 	}
+
 	remoteIsLite := false
 	if liteValue, haveRemoteIs := desc.parsed.Attribute(sdp.AttrKeyICELite); haveRemoteIs && liteValue == sdp.AttrKeyICELite {
 		remoteIsLite = true
 	}
 
-	fingerprint, haveFingerprint := desc.parsed.Attribute("fingerprint")
+	fingerprint, fingerprintHash, err := extractFingerprint(desc.parsed)
-	for _, m := range pc.RemoteDescription().parsed.MediaDescriptions {
+	if err != nil {
-		if !haveFingerprint {
+		return err
-			fingerprint, haveFingerprint = m.Attribute("fingerprint")
+	}
-		}
 
-		for _, a := range m.Attributes {
+	remoteUfrag, remotePwd, candidates, err := extractICEDetails(desc.parsed)
-			switch {
+	if err != nil {
-			case a.IsICECandidate():
+		return err
-				sdpCandidate, err := a.ToICECandidate()
+	}
-				if err != nil {
-					return err
-				}
 
-				candidate, err := newICECandidateFromSDP(sdpCandidate)
+	for _, c := range candidates {
-				if err != nil {
+		if err = pc.iceTransport.AddRemoteCandidate(c); err != nil {
-					return err
+			return err
-				}
-
-				if err = pc.iceTransport.AddRemoteCandidate(candidate); err != nil {
-					return err
-				}
-			case strings.HasPrefix(*a.String(), "ice-ufrag"):
-				remoteUfrag = (*a.String())[len("ice-ufrag:"):]
-			case strings.HasPrefix(*a.String(), "ice-pwd"):
-				remotePwd = (*a.String())[len("ice-pwd:"):]
-			}
 		}
 	}
 
-	if !haveFingerprint {
-		return fmt.Errorf("could not find fingerprint")
-	}
-
-	parts := strings.Split(fingerprint, " ")
-	if len(parts) != 2 {
-		return fmt.Errorf("invalid fingerprint")
-	}
-	fingerprint = parts[1]
-	fingerprintHash := parts[0]
-
 	iceRole := ICERoleControlled
 	// If one of the agents is lite and the other one is not, the lite agent must be the controlling agent.
 	// If both or neither agents are lite the offering agent is controlling.

sdp.go

@@ -265,3 +265,70 @@ func getPeerDirection(media *sdp.MediaDescription) RTPTransceiverDirection {
 	}
 	return RTPTransceiverDirection(Unknown)
 }
+
+func extractFingerprint(desc *sdp.SessionDescription) (string, string, error) {
+	fingerprints := []string{}
+
+	if fingerprint, haveFingerprint := desc.Attribute("fingerprint"); haveFingerprint {
+		fingerprints = append(fingerprints, fingerprint)
+	}
+
+	for _, m := range desc.MediaDescriptions {
+		if fingerprint, haveFingerprint := m.Attribute("fingerprint"); haveFingerprint {
+			fingerprints = append(fingerprints, fingerprint)
+		}
+	}
+
+	if len(fingerprints) < 1 {
+		return "", "", ErrSessionDescriptionNoFingerprint
+	}
+
+	for _, m := range fingerprints {
+		if m != fingerprints[0] {
+			return "", "", ErrSessionDescriptionConflictingFingerprints
+		}
+	}
+
+	parts := strings.Split(fingerprints[0], " ")
+	if len(parts) != 2 {
+		return "", "", ErrSessionDescriptionInvalidFingerprint
+	}
+	return parts[1], parts[0], nil
+}
+
+func extractICEDetails(desc *sdp.SessionDescription) (string, string, []ICECandidate, error) {
+	candidates := []ICECandidate{}
+	remotePwd := ""
+	remoteUfrag := ""
+
+	for _, m := range desc.MediaDescriptions {
+		for _, a := range m.Attributes {
+			switch {
+			case a.IsICECandidate():
+				sdpCandidate, err := a.ToICECandidate()
+				if err != nil {
+					return "", "", nil, err
+				}
+
+				candidate, err := newICECandidateFromSDP(sdpCandidate)
+				if err != nil {
+					return "", "", nil, err
+				}
+
+				candidates = append(candidates, candidate)
+			case strings.HasPrefix(*a.String(), "ice-ufrag"):
+				remoteUfrag = (*a.String())[len("ice-ufrag:"):]
+			case strings.HasPrefix(*a.String(), "ice-pwd"):
+				remotePwd = (*a.String())[len("ice-pwd:"):]
+			}
+		}
+	}
+
+	if remoteUfrag == "" {
+		return "", "", nil, ErrSessionDescriptionMissingIceUfrag
+	} else if remotePwd == "" {
+		return "", "", nil, ErrSessionDescriptionMissingIcePwd
+	}
+
+	return remoteUfrag, remotePwd, candidates, nil
+}

sdp_test.go

@@ -0,0 +1,88 @@
+// +build !js
+
+package webrtc
+
+import (
+	"testing"
+
+	"github.com/pion/sdp/v2"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestExtractFingerprint(t *testing.T) {
+	t.Run("Good Session Fingerprint", func(t *testing.T) {
+		s := &sdp.SessionDescription{
+			Attributes: []sdp.Attribute{{Key: "fingerprint", Value: "foo bar"}},
+		}
+
+		fingerprint, hash, err := extractFingerprint(s)
+		assert.NoError(t, err)
+		assert.Equal(t, fingerprint, "bar")
+		assert.Equal(t, hash, "foo")
+	})
+
+	t.Run("Good Media Fingerprint", func(t *testing.T) {
+		s := &sdp.SessionDescription{
+			MediaDescriptions: []*sdp.MediaDescription{
+				{Attributes: []sdp.Attribute{{Key: "fingerprint", Value: "foo bar"}}},
+			},
+		}
+
+		fingerprint, hash, err := extractFingerprint(s)
+		assert.NoError(t, err)
+		assert.Equal(t, fingerprint, "bar")
+		assert.Equal(t, hash, "foo")
+	})
+
+	t.Run("No Fingerprint", func(t *testing.T) {
+		s := &sdp.SessionDescription{}
+
+		_, _, err := extractFingerprint(s)
+		assert.Equal(t, ErrSessionDescriptionNoFingerprint, err)
+	})
+
+	t.Run("Invalid Fingerprint", func(t *testing.T) {
+		s := &sdp.SessionDescription{
+			Attributes: []sdp.Attribute{{Key: "fingerprint", Value: "foo"}},
+		}
+
+		_, _, err := extractFingerprint(s)
+		assert.Equal(t, ErrSessionDescriptionInvalidFingerprint, err)
+	})
+
+	t.Run("Conflicting Fingerprint", func(t *testing.T) {
+		s := &sdp.SessionDescription{
+			Attributes: []sdp.Attribute{{Key: "fingerprint", Value: "foo"}},
+			MediaDescriptions: []*sdp.MediaDescription{
+				{Attributes: []sdp.Attribute{{Key: "fingerprint", Value: "foo blah"}}},
+			},
+		}
+
+		_, _, err := extractFingerprint(s)
+		assert.Equal(t, ErrSessionDescriptionConflictingFingerprints, err)
+	})
+}
+
+func TestExtractICEDetails(t *testing.T) {
+	t.Run("Missing ice-pwd", func(t *testing.T) {
+		s := &sdp.SessionDescription{
+			MediaDescriptions: []*sdp.MediaDescription{
+				{Attributes: []sdp.Attribute{{Key: "ice-ufrag", Value: "foobar"}}},
+			},
+		}
+
+		_, _, _, err := extractICEDetails(s)
+		assert.Equal(t, err, ErrSessionDescriptionMissingIcePwd)
+	})
+
+	t.Run("Missing ice-ufrag", func(t *testing.T) {
+		s := &sdp.SessionDescription{
+			MediaDescriptions: []*sdp.MediaDescription{
+				{Attributes: []sdp.Attribute{{Key: "ice-pwd", Value: "foobar"}}},
+			},
+		}
+
+		_, _, _, err := extractICEDetails(s)
+		assert.Equal(t, err, ErrSessionDescriptionMissingIceUfrag)
+	})
+}