Needs to ignore repeated slashes in file URL

[watilde]

Hello!

Since the file state doesn't have a way to jump to the special authority slashes state, once the code enters into the file state, repeated slashes in the beginning of a file URL(e.g. file://///foo/bar should be file:///foo/bar) can't be ignored.

To cover the case, it could be done by the following steps:

1. In scheme state, stop jumping to file state directly before checking special authority slashes state.
2. In special authority ignore slashes state, add file state check.
3. In scheme state, add file state check before jumping to special relative or authority state.

Resources:

• In the bs file, it can be like this: watilde@b587151
• Here is an example impl: watilde/node@247f960

What do you think?

[annevk]

It does seem that Chrome and Edge have this behavior, while Firefox and Safari (likely changed I suppose) do not.

I would support changing this based on that. We'll need new tests as well.

[watilde]

@annevk Thanks for your comment!

Then I will try reading external/google-url(correct?) impl as a reference, and write some tests to WPT. To make a PR to WPT, could I possibility make a PR to this repo with this commit before that?

[annevk]

Yeah, you can do the PRs independently as you wish. We'll link them all up before landing.

As for what to use as reference, google-url seems good, but testing Edge would be good too. As for what to test, switching between \ and / would be good, and whether the eventual non-//\ code point ends up as path or host. Not sure whether there is much else here.

[rmisev]

I think @watilde you must be careful with this issue: also consider case with localhost, for example:
file://localhost///foo/bar

It looks likes Edge and Chrome ignores slashes after localhost. If not, then this can cause a re-parsing issue:

After first pass we will get:
href: file://///foo/bar
pathname: ///foo/bar

If now we try to parse href, then will get:
href: file:///foo/bar
pathname: /foo/bar

Other test cases to try:
file:///localhost///foo/bar
file:////localhost///foo/bar

[watilde]

@rmisev Thanks for your comment, that's a good point indeed! I will try supporting it, and potentially we need to update some logics around the file host state.

I also find an another case that should ignore some slashes:
input: new URL('////////server/file', 'file:///tmp/mock/path')
href: file:///server/file
pathname: /server/file

So it's not only about when the base is localhost but also includes some of the other base hosts.

[annevk]

I have another problem that I don't really know a good solution to. If you have file:/// as input and then you set its pathname to //, that would create a URL that cannot be serialized and then parsed into the same URL. In Chrome this can indeed lead to problems: http://software.hixie.ch/utilities/js/live-dom-viewer/saved/4883.

[yutakahirano]

Let me check my understanding.

According to RFC3986, path can start with multiple slashes only when authority is present. Note that authority can be empty as reg-name can be empty.

      URI         = scheme ":" hier-part [ "?" query ] [ "#" fragment ]

      hier-part   = "//" authority path-abempty
                  / path-absolute
                  / path-rootless
                  / path-empty
...
      authority   = [ userinfo "@" ] host [ ":" port ]
      host        = IP-literal / IPv4address / reg-name
      reg-name    = *( unreserved / pct-encoded / sub-delims )
...
      path-abempty  = *( "/" segment )
      path-absolute = "/" [ segment-nz *( "/" segment ) ]
      path-noscheme = segment-nz-nc *( "/" segment )
      path-rootless = segment-nz *( "/" segment )
      path-empty    = 0<pchar>
      segment       = *pchar
      segment-nz    = 1*pchar
      segment-nz-nc = 1*( unreserved / pct-encoded / sub-delims / "@" )
                    ; non-zero-length segment without any colon ":"

      pchar         = unreserved / pct-encoded / sub-delims / ":" / "@"

So, after seeing "file:",

• If the parser sees "//":
  authority follows (note that it can be empty), followed by path-abempty which is either empty or starting with (possible multiple) slashes.
• Otherwise, if the parser sees "/":
  path-absolute follows (including the "/"). Note that path-absolute cannot start with "//".
• Otherwise:
  ...

Here are some examples:

input	host	path	comment
file:/x/y/z	n/a	/x/y/z	authority is not present
file://x/y/z	x	/y/z
file://x//y/z	x	//y/z
file:///x/y/z		/x/y/z	authority is empty
file:////x/y/z		//x/y/z	authority is empty
file:/localhost/y/z	n/a	/localhost/y/z	authority is not present
file://localhost/y/z	localhost	/y/z
file:///localhost/y/z		/localhost/y/z	authority is empty

Please correct me if I'm wrong.

[annevk]

That is correct per that RFC, but nobody really follows it closely and the compatibility constrains are likely different too.

[yutakahirano]

It looks @watilde is proposing to ignore repeating leading slashes in pathname. If we want to do that, we would also want to remove repeating leading slashes when setting pathname, which will solve your problem.

[annevk]

But only ignore them when authority is empty, or just always ignoring them? I guess file systems do not allow for empty directories, so maybe that is okay?

[yutakahirano]

If we ignore repeating leading slashes in pathname on file://localhost/ (i.e., "file://localhost////////////path" is parsed as "file:///path"), I think we should ignore them even when authority is not empty. In other words, if we ignore repeating leading slahes in pathname only when authority is empty, I think we should not omit 'localhost'.

[annevk]

Sounds reasonable to me. (It's very heard to search for "file systems that support directories without name".)

[annevk]

Always trimming would also work for when you set host to the empty string through the API. (Another case we should make sure to test.)

[watilde]

@annevk @yutakahirano Sorry for my late response, and that sounds what we're looking for. I will catch up now!

[rmisev]

Yet another sample:
file://localhost///a//../..//

Currently it is serialized to:
file://////

It shows what repeated leading slashes must be removed at the end of path parse (path state), when all possible /../ are resolved. At this point localhost is already converted to empty host, so when switching from path state or at EOF:

• If url’s scheme is "file" and url’s host is empty, then remove all leading empty strings from path excluding last path's string.

This also works for pathname setter.

[annevk]

@watilde are you still interested in working on this?

[watilde]

@annevk I may take more time to fix. I would appreciate if you could take a patch :)

[annevk]

Note that in #278 I went with a slightly different solution than @rmisev proposes since I wanted to go with @yutakahirano's proposal of ignoring the leading slashes even when the URL contains a host.

I also ended up finding an unrelated bug and fixed that too.

[annevk]

Bugs filed:

• https://bugs.webkit.org/show_bug.cgi?id=170233
• https://bugs.chromium.org/p/chromium/issues/detail?id=706335
• https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/11435595/
• https://bugzilla.mozilla.org/show_bug.cgi?id=1351603

[watilde]

brilliant! thanks.

[alwinb]

Could someone explain to me the reasoning behind this decision?

I believe that it was motivated by the desire to interpret file:////host as file://host file:///host, making file URLs and other 'special' URLs more alike. But the result is that now paths of file URLs are different from paths of other special URLs, and less expressive.

I've been working on an URL parser that follows the WhatWG behaviour but separates the parsing and the normalization phases. In that light, the following cases from the web platform tests are especially confusing.

• /..//localhost//pig against file://lion/ resolves to file://lion/localhost//pig rather than file://lion//localhost//pig.
• file://localhost//a//../..// resolves to file:/// rather than file:////

Note that /..//localhost//pig against http://lion/ resolves to http://lion//localhost//pig
(and http://localhost//a//../..// resolves to http://localhost///).

Why would you make this trade-off? Is there a motivation that I am missing?

Thank you,

[annevk]

@alwinb did you read through this issue and the links provided? Skimming it the rationale appears to be related to existing implementations and avoiding reparsing issues.

[alwinb]

I did read through it, it seems to have gone from wanting to ignore slashes after file:/// via a reparsing issue to this solution.

I want to make a case for keeping path normalization of file URLs and other 'special' URLs the same.
They have been the same (barring drive letters) until this change was made.

However, none of the browsers (or Edge?) implement the specced behaviour.
I tested Chrome canary, Safari preview and Firefox nightly.

Input	Firefox and Safari	Chrome
/..//localhost//pig against file://lion/	file:////localhost//pig	file://lion//localhost//pig (seems right)
file://localhost//a//../..//	file://///	file://localhost///
file://localhost////foo	file://////foo	file://localhost////foo
file:////foo	file:////foo	file:///foo

• All agree on the pathName except in the fourth case. (I don't know why Safari and Firefox drop the host in the first.)
• Only Chrome (Edge?) match the spec in only the fourth case.
• Firefox and Safari do not ignore slashes and thereby avoid the reparsing issue.
• Chrome (Edge?) only ignores slashes immediately following file:/// and avoids the reparsing issue by keeping localhost.

Some ideas for alternatives:

1. Always replace the empty host with localhost.
2. Do so only in ambiguous cases.
3. Prefix ambiguous pathNames with ./.
4. Don't collapse slashes at all.
5. Come up with something else?

I guess 1, 4 (and 5) seem to be the more reasonable ones.

Thanks,

[annevk]

Hmm, okay, I guess this warrants another look then. Could you file a new issue with the above findings? And maybe add the specified behavior as a column to the table?

[alwinb]

I created a new issue, #405