WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
161364
URLParser should handle relative URLs that start with //
https://bugs.webkit.org/show_bug.cgi?id=161364
Summary
URLParser should handle relative URLs that start with //
Alex Christensen
Reported
2016-08-29 17:17:24 PDT
URLParser should handle relative URLs that start with //
Attachments
Patch
(5.39 KB, patch)
2016-08-29 17:24 PDT
,
Alex Christensen
darin
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Alex Christensen
Comment 1
2016-08-29 17:24:37 PDT
Created
attachment 287357
[details]
Patch
Alex Christensen
Comment 2
2016-08-29 17:25:32 PDT
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 3
2016-08-29 17:27:16 PDT
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
Comment 4
2016-08-29 21:08:04 PDT
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
Comment 5
2016-08-30 01:06:54 PDT
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
Comment 6
2016-08-30 12:26:02 PDT
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
Comment 7
2016-08-30 12:36:57 PDT
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
Comment 8
2016-08-30 18:52:03 PDT
(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
Comment 9
2016-08-31 10:30:12 PDT
(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
Comment 10
2016-08-31 14:27:17 PDT
(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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug