Bug 162836 – URLParser should ignore tabs at all locations

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

[Alex Christensen]

URLParser should ignore tabs at all locations

[Alex Christensen]

Created attachment 290421 [details]
Patch

[Alex Christensen]

Comment on attachment 290421 [details]
Patch

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

Geoff said "r=me"

> Source/WebCore/ChangeLog:3
> +        URLParser should ignore tabs at all locations

s/ignore/strip/

> Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp:72
> +enum class CheckTabs { No, Yes };

enum class TestTabs?

> Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp:73
> +const CheckTabs containsEncodedSurrogatePairs = CheckTabs::No;

Maybe "checkTabsValueForSurrogatePairs"? And then a comment here that says we can't auto-insert tabs because they would ruin the encoding.

> Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp:455
> +    if (checkTabs == CheckTabs::Yes) {
> +        for (size_t i = 0; i < urlString.length(); ++i) {
> +            String urlStringWithTab = makeString(urlString.substring(0, i), "\t", urlString.substring(i));
> +            ExpectedParts invalidPartsWithTab = {"", "", "", "", 0, "" , "", "", urlStringWithTab};
> +            checkURLDifferences(urlStringWithTab, partsNew.isInvalid() ? invalidPartsWithTab : partsNew, partsOld.isInvalid() ? invalidPartsWithTab : partsOld, CheckTabs::No);
> +        }
> +    }

Maybe some helper function action for tab checking?

[Alex Christensen]

I'm also going to remove the incorrect tests for "http://123\t.256/" and "http://123.\t256/" canonicalizing to "http://123.256/".  Those tests were verifying incorrect behavior that this patch fixed.  There is already a test that canonicalizes it correctly to "http://123.0.1.0/" in the new URLParser.

[Alex Christensen]

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