Skip to content

Commit

Permalink
Implement permissive STUN URL parsing
Browse files Browse the repository at this point in the history
The parser included in the ice package strictly adheres to RFC 7064,
however, this can be problematic when attempting to use ICE server URLs
that were automatically generated by a third party.

Twilio, for example, has been seen to send the following in its list of
ICE servers:

    stun:global.stun.twilio.com:3478?transport=udp

which would require user intervention to sanitise before passing to
Pion.
This patch side-steps this aspect of ice.ParseURL by pre-stripping any
queries that may be present so that we can allow URLs of this form.
  • Loading branch information
mchlrhw committed Jul 17, 2019
1 parent 632530b commit 7243561
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 36 deletions.
18 changes: 0 additions & 18 deletions configuration.go
Expand Up @@ -2,10 +2,6 @@

package webrtc

import (
"github.com/pion/ice"
)

// Configuration defines a set of parameters to configure how the
// peer-to-peer communication via PeerConnection is established or
// re-established.
Expand Down Expand Up @@ -51,17 +47,3 @@ type Configuration struct {
// SDP answers generated by the PeerConnection.
SDPSemantics SDPSemantics
}

func (c Configuration) getICEServers() (*[]*ice.URL, error) {
var iceServers []*ice.URL
for _, server := range c.ICEServers {
for _, rawURL := range server.URLs {
url, err := ice.ParseURL(rawURL)
if err != nil {
return nil, err
}
iceServers = append(iceServers, url)
}
}
return &iceServers, nil
}
29 changes: 29 additions & 0 deletions configuration_common.go
@@ -0,0 +1,29 @@
package webrtc

import (
"strings"

"github.com/pion/ice"
)

// getICEServers side-steps the strict parsing mode of the ice package
// (as defined in https://tools.ietf.org/html/rfc7064) by stripping any
// erroneous queries from "stun(s):" URLs before parsing.
func (c Configuration) getICEServers() (*[]*ice.URL, error) {
var iceServers []*ice.URL
for _, server := range c.ICEServers {
for _, rawURL := range server.URLs {
if strings.HasPrefix(rawURL, "stun") {
// strip the query from "stun(s):" if present
parts := strings.Split(rawURL, "?")
rawURL = parts[0]
}
url, err := ice.ParseURL(rawURL)
if err != nil {
return nil, err
}
iceServers = append(iceServers, url)
}
}
return &iceServers, nil
}
18 changes: 0 additions & 18 deletions configuration_js.go
Expand Up @@ -2,10 +2,6 @@

package webrtc

import (
"github.com/pion/ice"
)

// Configuration defines a set of parameters to configure how the
// peer-to-peer communication via PeerConnection is established or
// re-established.
Expand Down Expand Up @@ -37,17 +33,3 @@ type Configuration struct {
// ICECandidatePoolSize describes the size of the prefetched ICE pool.
ICECandidatePoolSize uint8
}

func (c Configuration) getICEServers() (*[]*ice.URL, error) {
var iceServers []*ice.URL
for _, server := range c.ICEServers {
for _, rawURL := range server.URLs {
url, err := ice.ParseURL(rawURL)
if err != nil {
return nil, err
}
iceServers = append(iceServers, url)
}
}
return &iceServers, nil
}
17 changes: 17 additions & 0 deletions configuration_test.go
Expand Up @@ -35,4 +35,21 @@ func TestConfiguration_getICEServers(t *testing.T) {
_, err := cfg.getICEServers()
assert.NotNil(t, err)
})

t.Run("Success", func(t *testing.T) {
// ignore the fact that stun URLs shouldn't have a query
serverStr := "stun:global.stun.twilio.com:3478?transport=udp"
expectedServerStr := "stun:global.stun.twilio.com:3478"
cfg := Configuration{
ICEServers: []ICEServer{
{
URLs: []string{serverStr},
},
},
}

parsedURLs, err := cfg.getICEServers()
assert.Nil(t, err)
assert.Equal(t, expectedServerStr, (*parsedURLs)[0].String())
})
}

0 comments on commit 7243561

Please sign in to comment.