WebRTC: URL class can't parse ICE helper server urls properly

• Component: WebRTC
• Status: NEW ---

[Adam Bergkvist]

WebKit's URL class can only parse the scheme part of a ICE helper server url. These urls have a host and a port but are not hierarchical [1].

The OpenWebRTC WebRTC backend (MediaEndpointOwr) currently extracts the original url string from the URL object and parses it again.

[1] https://tools.ietf.org/html/rfc7064#section-3.1

[Alex Christensen]

I just noticed this after having rewritten the URLParser.  Could you give an example of such a URL?

[Adam Bergkvist]

The URI schemas are defined here [1][2].

A simple example is:
stun:stun1.example.net

[1] https://tools.ietf.org/html/rfc7064#section-3
[2] https://tools.ietf.org/html/rfc7065#section-3

[Alex Christensen]

<script>
try {
    var u = new URL("stun:stun1.example.net");
    alert("scheme " + u.scheme);
    alert("host " + u.host);
    alert("pathname " + u.pathname);
} catch (e) { alert("threw"); }
</script>

This behaves the same in Chrome, Firefox, and Safari.  Are you expecting the host to be stun1.example.net?  What problems are actually caused by this?  It's possible we will need to revise either the URL grammar or the grammars defined in those RFCs.  We definitely need to keep the URL object's behavior for schemes like mailto, though.

[Adam Bergkvist]

I was a bit unclear in the bug title, but I was referring to the cpp class URL.

[Radar WebKit Bug Importer]

<rdar://problem/34134444>

[Jon Lee]

Is this still a valid bug?

[Adam Bergkvist]

I don't have a build available to test at the moment.

For example in [1], the ICE helper server urls are parsed to WebKit URLs. Host and port are not parsed correctly.

[1] https://github.com/WebKit/webkit/blob/master/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp#L305

[Anne van Kesteren]

This is tracked by https://github.com/w3c/webrtc-pc/issues/2660 standards-wise.

How does OpenWebRTC parse it?

I agree that RTCPeerConnection::iceServersFromConfiguration doesn't robust in the face of how these URLs are parsed by the URL parser.

(Note that the cpp class and JavaScript class should behave equivalently. We don't want them to diverge.)

[Philippe Normand]

(In reply to Anne van Kesteren from comment #8)
> This is tracked by https://github.com/w3c/webrtc-pc/issues/2660
> standards-wise.
> 
> How does OpenWebRTC parse it?

OpenWebRTC is no longer maintained. The corresponding backend was removed from webKit soon after the migration to libwebrtc was completed.

The WPE/GTK ports rely on a different backend, based on GstWebRTC.
In GStreamerMediaEndpoint::setConfiguration() we replace `stun:` with `stun://` (same for turn:) and pass the result to the WebKit URL parser. This is a workaround indeed :)

[Anne van Kesteren]

That will do the wrong thing most likely for IPv4 and a variety of non-ASCII or percent-encoded host names.

I think what we want to do is something like this:

* Let inputURL be the result of URL parsing input.
* If inputURL is failure, then return failure.
* If inputURL's scheme is not "stun" or "turn", then return failure.
* If inputURL's path is not an opaque path, then return failure.
* If inputURL's query or fragment is non-null, then return failure.
* Let hostAndPortString be inputURL's path.
* Let hostAndPortURL be the result of URL parsing the concatenation of "https://" and hostAndPortURL.
* If hostAndPortURL is failure, then return failure.
* Return (hostAndPortURL's host, hostAndPortURL's port).

(The URL Standard should probably offer a primitive for the hostAndPort bit, that's not exactly pretty.)

[Anne van Kesteren]

https://w3c.github.io/webrtc-pc/#set-pc-configuration (step 4) defines parsing now in a way that is compatible with URLs.