File URL reparse bug with percent encoded windows drive letter

[rmisev]

There is a reparse bug, when parsing URL with percent encoded windows drive letter (C|) in the host. URLs to test:

file://%43%7C
file://C%7C
file://%43|

Any of them parses to file://c|/, the latter - to file:///c:/.

Let's see how browsers parse them:

Browser	first pass	second pass
Chrome:	file://c%7C/	file://c%7C/
Firefox:	file:///	file:///
Safari:	file://c|/	file:///c:/

Safari has the same bug as spec. Chrome suggests possible resolution.

Some ways how to fix bug:

1. Percent encode | in the host after host parsing (as Chrome does)
2. Fail if host contains | after host parsing.
3. Check for Windows drive letter after host parsing.

[annevk]

Nice find. I kinda like 3 (and then returning failure or explicitly dropping it). 1 and 2 seem a bit too generic.

[rmisev]

I think, if | is allowed in hosts, then it may be a bit strange why c| host is dropped or failed. So I prefer Chrome's behavior.
Percent encoding lets use c| as host (because percent encoded c| will not be treated as windows drive letter).
To not be too generic, I think | can be percent encoded if 3 succeeds.

Yet another example, which leads to the same bug:

var u = new URL("file://h/")
u.hostname = "c|"
// u.href == "file://c|/"

[achristensen07]

Does your table include Chrome on Linux or Chrome on Windows? There are some functional differences in Chromium's interpretation of Windows drive letters in URLs on different operating systems, as seen by looking for C|/foo/bar on https://wpt.fyi/results/url/url-constructor.html . I think it's important when considering Windows drive letters what happens on Windows. What is the behavior of new and old Edge? What about IE?

While I do think URL parsing should be idempotent, I don't have a strong opinion here. I initially think all those URLs should be canonicalized to file:///c:/ and parse successfully.

[rmisev]

Does your table include Chrome on Linux or Chrome on Windows?

I tested both. The same result: file://c%7C/

What is the behavior of new and old Edge? What about IE?

Browser	first pass	second pass
new Edge:	file://c%7C/	file://c%7C/
old Edge:	file://c%7c/	file://c%7c/
IE:	file://c%7c/	file://c%7c/

[achristensen07]

After tinkering around a bit more, I think we should have file://c%7C/ canonicalize to file:///c:/ and not return failure because file://c|/ canonicalizes to file:///c:/ in IE and Edge and the current specification has us percent-decode file hosts. It's worth making this change to stay idempotent.

[rmisev]

I think I can agree with you, but what result must be here:

var u = new URL("file://h/path")
u.hostname = "c|"

Currently result (u.href) is file://c|/path, and after reparse: file:///c:/path.
But maybe it is better to reject such host name value? I think it may not be acceptable to change path by setting host name.

[achristensen07]

Ah, yes. I think we should reject a windows drive letter or something that would percent decode to a windows drive letter in the hostname setter but not in the main parser.

[alwinb]

Coincidentally, step 1.3.1 in the file host state, states that file-hosts should be parsed as non-special, i.e. as opaque hosts, I think that's a typo.

[alwinb]

It might not be an unreasonable idea to just add | to the set of forbidden host code points. It is not an URL code point, so it is invalid in hostnames as per the current standard already. RFC 3986 and 3987 do not allow it either, so it seems unlikely that there are URLs with a | in the hostname out there.

[achristensen07]

I tried implementing forbidding '|' in hosts in https://bugs.webkit.org/show_bug.cgi?id=220778

[achristensen07]

Is there a consensus on changing the spec to forbid '|' in hosts to solve this issue?

[valenting]

Firefox already forbids | in the hostname.
I expect when we get around to actually allowing hostnames for file:// URLs that will still be the case.
So that's a 👍 from me.

[annevk]

@achristensen07 want to make the change to the specification as well?

[achristensen07]

Github found my PR to this repo. web-platform-tests/wpt#28118 is my PR for the tests.