Bug 162646 – URLParser: Handle windows drive letters after two slashes in relative URLs according to spec

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

URLParser: Handle windows drive letters after two slashes in relative URLs according to spec

[Alex Christensen]

Created attachment 290028 [details]
Patch

[Saam Barati]

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.

[Alex Christensen]

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.

[Alex Christensen]

http://trac.webkit.org/changeset/206477