WebRTC: URL class can't parse ICE helper server urls properly
https://bugs.webkit.org/show_bug.cgi?id=164508
Summary WebRTC: URL class can't parse ICE helper server urls properly
Adam Bergkvist
Reported 2016-11-08 00:37:33 PST
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
Attachments
Alex Christensen
Comment 1 2017-02-02 00:17:40 PST
I just noticed this after having rewritten the URLParser. Could you give an example of such a URL?
Adam Bergkvist
Comment 2 2017-02-09 04:38:59 PST
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
Comment 3 2017-02-09 10:28:40 PST
<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
Comment 4 2017-02-10 00:11:41 PST
I was a bit unclear in the bug title, but I was referring to the cpp class URL.
Radar WebKit Bug Importer
Comment 5 2017-08-29 09:52:01 PDT
Jon Lee
Comment 6 2017-08-29 15:44:38 PDT
Is this still a valid bug?
Adam Bergkvist
Comment 7 2017-08-29 23:57:21 PDT
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
Comment 8 2023-04-01 01:06:03 PDT
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
Comment 9 2023-04-01 07:35:05 PDT
(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
Comment 10 2023-04-02 08:28:13 PDT
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
Comment 11 2023-06-12 09:04:32 PDT
https://w3c.github.io/webrtc-pc/#set-pc-configuration (step 4) defines parsing now in a way that is compatible with URLs.
Note You need to log in before you can comment on or make changes to this bug.