Bug 162646 - URLParser: Handle windows drive letters after two slashes in relative URLs according to spec
Summary: URLParser: Handle windows drive letters after two slashes in relative URLs ac...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-27 17:06 PDT by Alex Christensen
Modified: 2016-09-27 17:34 PDT (History)
0 users

See Also:


Attachments
Patch (7.90 KB, patch)
2016-09-27 17:06 PDT, Alex Christensen
saam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2016-09-27 17:06:08 PDT
URLParser: Handle windows drive letters after two slashes in relative URLs according to spec
Comment 1 Alex Christensen 2016-09-27 17:06:55 PDT
Created attachment 290028 [details]
Patch
Comment 2 Saam Barati 2016-09-27 17:30:05 PDT
Comment on attachment 290028 [details]
Patch

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

r=me

> Source/WebCore/platform/URLParser.cpp:1457
> +                bool windowsQuirk = c.codeUnitsSince(authorityOrHostBegin) == 2 && isWindowsDriveLetter(authorityOrHostBegin);

Isn't the ==2 check redundant here since isWindowsDriveLetter only returns true if you've moved 2 points along?
Or is the goal to quickly rule out the isWindowsDriveLetter(...) check?

> Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp:643
> +    checkRelativeURLDifferences("//C|/foo/bar", "file:///tmp/mock/path",

I never knew "|" is OK there.
Comment 3 Alex Christensen 2016-09-27 17:33:48 PDT
Comment on attachment 290028 [details]
Patch

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

>> Source/WebCore/platform/URLParser.cpp:1457
>> +                bool windowsQuirk = c.codeUnitsSince(authorityOrHostBegin) == 2 && isWindowsDriveLetter(authorityOrHostBegin);
> 
> Isn't the ==2 check redundant here since isWindowsDriveLetter only returns true if you've moved 2 points along?
> Or is the goal to quickly rule out the isWindowsDriveLetter(...) check?

It's needed for correctness.  isWindowsDriveLetter(iterator) returns true if the String the iterator is pointing to starts with C: but I need it to fail if it contains more than just C:
This is covered by the test with //C:asdf/foo/bar

>> Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp:643
>> +    checkRelativeURLDifferences("//C|/foo/bar", "file:///tmp/mock/path",
> 
> I never knew "|" is OK there.

It's equivalent to a : in this case, but when canonicalized it becomes a :, so it's a syntax violation.
Comment 4 Alex Christensen 2016-09-27 17:34:15 PDT
http://trac.webkit.org/changeset/206477