-
Notifications
You must be signed in to change notification settings - Fork 141
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
Restrict setting protocol to "file" #269
Conversation
As file URLs cannot have username/password/port we don’t want to allow changing the scheme of a URL that contains one or more of those components. Fixes #259.
This looks good to me to fix the issue, but I'm worried about the change seems a new behaviour that browsers don't have yet. |
Tests at web-platform-tests/wpt#5112. Yeah, we'll need to file browser bugs. What browsers currently do doesn't make any sense, so hopefully this can still change. |
and <var>buffer</var> is "<code>file</code>", then return. | ||
|
||
<li><p>If <var>url</var>'s <a for=url>scheme</a> is "<code>file</code>" and its | ||
<a for=url>host</a> is an <a>empty host</a> or null, then return. |
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.
I think the last item must be:
4. If url's host is an empty host or null, and buffer is a non-"file" special scheme, then return.
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.
It's only "file" that can come here and hit this. Other non-special schemes cannot be set to special schemes due to the earlier steps.
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.
Yes you are right.
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.
I guess I could remove the check for scheme being "file" though, since only file can have an empty/null 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.
No, that would be bad, since that would also block non-special URLs without host from changing schemes. Oops.
Since file URLs can not have `username/password/port`, the specification was updated to restrict setting protocol to "file". Refs: whatwg/url#269 Fixes: nodejs#11785
Since file URLs can not have `username/password/port`, the specification was updated to restrict setting protocol to "file". Refs: whatwg/url#269 Fixes: #11785 PR-URL: #11887 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Since file URLs can not have `username/password/port`, the specification was updated to restrict setting protocol to "file". Refs: whatwg/url#269 Fixes: nodejs#11785 PR-URL: nodejs#11887 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Since file URLs can not have `username/password/port`, the specification was updated to restrict setting protocol to "file". Refs: whatwg/url#269 Fixes: #11785 PR-URL: #11887 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
As file URLs cannot have username/password/port we don’t want to allow
changing the scheme of a URL that contains one or more of those
components.
Fixes #259.
Preview | Diff