URLParser should handle relative URLs that start with //

• Component: New Bugs
• Status: RESOLVED FIXED

[Alex Christensen]

URLParser should handle relative URLs that start with //

[Alex Christensen]

Created attachment 287357 [details]
Patch

[Alex Christensen]

An unspecified difference in URL behavior can be seen by executing this in all browsers:
alert(new URL("//", "https://example.com/path1/path2/index.html"))

[Alex Christensen]

Comment on attachment 287357 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=287357&action=review

> Source/WebCore/platform/URLParser.cpp:300
> +                copyURLPartsUntil(base, URLPart::SchemeEnd);

This is not in the spec, but it needs to be.  Such URLs are quite common.

> Source/WebCore/platform/URLParser.cpp:301
> +                m_buffer.append("://");

I'm not sure if this is "correct" behavior, or if "correct" behavior exists.

[Alex Christensen]

The web platform tests have a similar test for file URLs:
    "input": "//",
    "base": "file:///tmp/mock/path",
    ...
File URLs have special states in the state machine, but there are no such tests for non-file URLs.

[Anne van Kesteren]

Going through the state machine of https://url.spec.whatwg.org/#concept-basic-url-parser I get:

* https://url.spec.whatwg.org/#scheme-start-state "/" -> no scheme state 
* https://url.spec.whatwg.org/#no-scheme-state "/" -> relative state
* https://url.spec.whatwg.org/#relative-state "/" consumed -> relative slash state
* https://url.spec.whatwg.org/#relative-slash-state "/" consumed -> special authority ignore slashes state
* https://url.spec.whatwg.org/#special-authority-ignore-slashes-state EOF -> authority state
* https://url.spec.whatwg.org/#authority-state EOF -> host state
* https://url.spec.whatwg.org/#host-state -> EOF and /url/ is special -> failure

This seems to match Firefox, which throws.

[Alex Christensen]

I think we need to change the spec in this case.  Copying the base url's scheme is not in the spec, but it's needed to successfully search anything on google.com.

By the way, Anne, I'm collecting a bunch of feedback on the url spec.  I'll let you know once I have a parser working well enough to be more sure about my requested changes.  Consider this the first.

[Alex Christensen]

I'm going to go ahead and assume the spec will be changed to match my code and land this so I can move ahead with finding other problems.  If it is changed differently, I'll change my new URLParser, which isn't used by default yet.
http://trac.webkit.org/changeset/205194

[Alex Christensen]

(In reply to comment #7)
> I'm going to go ahead and assume the spec will be changed to match my code
> and land this so I can move ahead with finding other problems.  If it is
> changed differently, I'll change my new URLParser, which isn't used by
> default yet.
This could be interpreted in a way that is antagonizing.  This isn't in harmony with the cooperative nature of open web standards, and that was not the intent.  My work so far is experimental, and I hope to contribute positively towards tests, standards, and implementations.

[Anne van Kesteren]

(In reply to comment #6)
> I think we need to change the spec in this case.  Copying the base url's
> scheme is not in the spec, but it's needed to successfully search anything
> on google.com.

That happens first thing in https://url.spec.whatwg.org/#relative-state, no?


> By the way, Anne, I'm collecting a bunch of feedback on the url spec.  I'll
> let you know once I have a parser working well enough to be more sure about
> my requested changes.  Consider this the first.

Sounds great, looking forward to it. Please do file them as issues at https://github.com/whatwg/url/issues/new so they don't get lost. Really pleased to see some work in this area.

Note https://bugzilla.mozilla.org/show_bug.cgi?id=1064700#c7 for changes Firefox is considering relative to the specification, that would move Firefox away from the specification & Safari, but closer to Chrome & Edge. The change seems relatively sensible (more normalization) so I'm hoping you all would be on board with that too.

[Alex Christensen]

(In reply to comment #9)
> (In reply to comment #6)
> > I think we need to change the spec in this case.  Copying the base url's
> > scheme is not in the spec, but it's needed to successfully search anything
> > on google.com.
> 
> That happens first thing in https://url.spec.whatwg.org/#relative-state, no?
Yep, I had overlooked that and done this in a different place, which had a similar effect for the few test cases I'm using so far.
> 
> 
> > By the way, Anne, I'm collecting a bunch of feedback on the url spec.  I'll
> > let you know once I have a parser working well enough to be more sure about
> > my requested changes.  Consider this the first.
> 
> Sounds great, looking forward to it. Please do file them as issues at
> https://github.com/whatwg/url/issues/new so they don't get lost. Really
> pleased to see some work in this area.
> 
> Note https://bugzilla.mozilla.org/show_bug.cgi?id=1064700#c7 for changes
> Firefox is considering relative to the specification, that would move
> Firefox away from the specification & Safari, but closer to Chrome & Edge.
> The change seems relatively sensible (more normalization) so I'm hoping you
> all would be on board with that too.
I think it's a little too early for me to respond to that Firefox change right now.