-
Notifications
You must be signed in to change notification settings - Fork 114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use the url spec to parse ice server urls (Take 2). #2853
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The username/password thing needs updating. As to the appropriateness of running the URL algorithm twice, once upon a synthetic URL, I'll let @annevk have an opinion on that.
<li> | ||
<p>Let <var>hostAndPortURL</var> be result of | ||
<a data-cite="!url#concept-url-parser">parsing</a> the concatenation of | ||
`"https://"` and <var>parsedURL</var>'s [=url/path=].</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a decidedldy odd way of parsing the arguments to the URL.
It assumes that 7064 and 7065 are completely consistent with the relevant parts of the URL spec. I'd prefer to have the URL spec learn to parse TURN URLs - @annevk may have opinions on the proper way to do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of whatwg/url#767? I think that's essentially the missing piece to be able to define a "stun/turn URL processor", right?
And essentially that comes down to a sound way of separating the port from the host.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@annevk may have opinions on the proper way to do this.
This is based on https://bugs.webkit.org/show_bug.cgi?id=164508#c10 from him.
I'd prefer to have the URL spec learn to parse TURN URLs
This PR moves more of the logic there, which seems a step in the right direction.
Specifically: RFC 7064 section 3.1 says: "<host> and <port> are specified in [RFC3986]." and the URL spec goals say: "Align RFC 3986 and RFC 3987 with contemporary implementations and obsolete the RFCs in the process"
see w3c/webrtc-pc#2853 and related discussions in w3c/webrtc-pc#2997 (comment)
Complete integration of URL parser from #2853 see also #2997 (comment) This aligns with the constraints set in the respective RFC (and thus with the current WebRTC Rec)
Complete integration of URL parser from #2853 see also #2997 (comment) This aligns with the constraints set in the respective RFC (and thus with the current WebRTC Rec)
Complete integration of URL parser from #2853 see also #2997 (comment) This aligns with the constraints set in the respective RFC (and thus with the current WebRTC Rec)
Fixes #2660. @annevk PTAL.
Preview | Diff