Bug 21974 – KURL.setProtocol does not re-interpret the URL properly

• Component: Platform
• Status: RESOLVED INVALID

[Brett Wilson (Google)]

KURL.setProtocol technically requires that the URL be completely re-parsed. For example, setting the protocol from http to file would trigger different handling. javascript and data URLs have different backslash handling.

This could mean that creating a URL in this way would produce a different parsing than giving the resulting string to KURL's constructor. For example,  changing "data:foo/bar" to "http:foo/bar".

This isn't a large issue in practice because setProtocol is not often (or maybe ever?) used to change the type of URLs like this, but rather to build up URLs from component parts. However, this function is potentially dangerous or misleading.

[Brett Wilson (Google)]

Here's another case:

original URL:
  http://localhost/e:/foo/
then doing:
  url.setProtocol("file");
should yield:
  file:///e:/

Instead, it yields:
  file://localhost/e:/foo/
which is wrong. If you feed this to KURL again, you'll get it correctly canonicalized to:
  file:///e:/foo/

Its scary to have cases where:
  KURL(kurl.string) != kurl;

[Brett Wilson (Google)]

Note that Firefox and Chrome both get my case in comment 2 correct. IE8 gives some weird error for my testcase that I didn't bother to figure out.

[Alexey Proskuryakov]

> should yield:
>   file:///e:/

Why?

> Instead, it yields:
>   file://localhost/e:/foo/

With ToT WebKit, I'm getting "file:///e:/foo/".

[Brett Wilson (Google)]

(In reply to comment #3)
> > should yield:
> >   file:///e:/
> 
> Why?
> 
> > Instead, it yields:
> >   file://localhost/e:/foo/
> 
> With ToT WebKit, I'm getting "file:///e:/foo/".

Sorry, my example was wrong (should have had "foo") To clarify setProtocol should trigger the code that removes "localhost" from the file URLs when you set it to file. I used my Windows Safari to test, which is probably out of date.

In fact, looking at the current code, it looks like this entire bug should be fixed. Do you agree?

[Alexey Proskuryakov]

Not sure - your original examples seem to still fail, if I understand them correctly. But if they fail indeed, and fail for a different reason, it could be better to track them in a new bug.

[Anne van Kesteren]

We no longer use KURL.