Changeset 206758 in webkit


Ignore:
Timestamp:
Oct 3, 2016, 3:55:46 PM (9 years ago)
Author:
achristensen@apple.com
Message:

Source/WebCore:
URLParser should strip tabs at all locations
ā€‹https://bugs.webkit.org/show_bug.cgi?id=162836

Reviewed by Geoffrey Garen.

Covered by adding tabs to each location of each API test
except tests that test the encoding of surrogate pairs,
because inserting a tab between the pairs changes the encoding.

  • platform/URLParser.cpp:

(WebCore::URLParser::takesTwoAdvancesUntilEnd):
(WebCore::URLParser::parse):
(WebCore::URLParser::parseIPv4Number):
(WebCore::URLParser::parseIPv4Host):

  • platform/URLParser.h:

Tools:
URLParser should ignore tabs at all locations
ā€‹https://bugs.webkit.org/show_bug.cgi?id=162836

Reviewed by Geoffrey Garen.

  • TestWebKitAPI/Tests/WebCore/URLParser.cpp:

(TestWebKitAPI::checkURL):
(TestWebKitAPI::checkRelativeURL):
(TestWebKitAPI::checkURLDifferences):
(TestWebKitAPI::checkRelativeURLDifferences):
(TestWebKitAPI::TEST_F):

Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r206753 r206758  
     12016-10-03  Alex Christensen  <achristensen@webkit.org>
     2
     3        URLParser should strip tabs at all locations
     4        https://bugs.webkit.org/show_bug.cgi?id=162836
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Covered by adding tabs to each location of each API test
     9        except tests that test the encoding of surrogate pairs,
     10        because inserting a tab between the pairs changes the encoding.
     11
     12        * platform/URLParser.cpp:
     13        (WebCore::URLParser::takesTwoAdvancesUntilEnd):
     14        (WebCore::URLParser::parse):
     15        (WebCore::URLParser::parseIPv4Number):
     16        (WebCore::URLParser::parseIPv4Host):
     17        * platform/URLParser.h:
     18
    1192016-10-03  Antti Koivisto  <antti@apple.com>
    220

  • trunk/Source/WebCore/platform/URLParser.cpp

    r206749 r206758  
    427427
    428428template<typename CharacterType>
     429bool URLParser::takesTwoAdvancesUntilEnd(CodePointIterator<CharacterType> iterator)
     430{
     431    if (iterator.atEnd())
     432        return false;
     433    advance<CharacterType, ReportSyntaxViolation::No>(iterator);
     434    if (iterator.atEnd())
     435        return false;
     436    advance<CharacterType, ReportSyntaxViolation::No>(iterator);
     437    return iterator.atEnd();
     438}
     439
     440template<typename CharacterType>
    429441ALWAYS_INLINE bool URLParser::isWindowsDriveLetter(CodePointIterator<CharacterType> iterator)
    430442{
    … …  
    15431555                LOG_STATE("FileHost");
    15441556                if (isSlashQuestionOrHash(*c)) {
    1545                     bool windowsQuirk = c.codeUnitsSince(authorityOrHostBegin) == 2 && isWindowsDriveLetter(authorityOrHostBegin);
     1557                    bool windowsQuirk = takesTwoAdvancesUntilEnd(CodePointIterator<CharacterType>(authorityOrHostBegin, c))
     1558                        && isWindowsDriveLetter(authorityOrHostBegin);
    15461559                    if (windowsQuirk) {
    15471560                        syntaxViolation(authorityOrHostBegin);
    … …  
    21012114        return Nullopt;
    21022115    while (!iterator.atEnd()) {
     2116        if (isTabOrNewline(*iterator)) {
     2117            didSeeSyntaxViolation = true;
     2118            ++iterator;
     2119            continue;
     2120        }
    21032121        if (*iterator == '.') {
    21042122            ASSERT(!value.hasOverflowed());
    … …  
    21742192    bool didSeeSyntaxViolation = false;
    21752193    while (!iterator.atEnd()) {
     2194        if (isTabOrNewline(*iterator)) {
     2195            didSeeSyntaxViolation = true;
     2196            ++iterator;
     2197            continue;
     2198        }
    21762199        if (items.size() >= 4)
    21772200            return Nullopt;

  • trunk/Source/WebCore/platform/URLParser.h

    r206735 r206758  
    7272    template<typename CharacterType, ReportSyntaxViolation = ReportSyntaxViolation::Yes>
    7373    void advance(CodePointIterator<CharacterType>&, const CodePointIterator<CharacterType>& iteratorForSyntaxViolationPosition);
     74    template<typename CharacterType> bool takesTwoAdvancesUntilEnd(CodePointIterator<CharacterType>);
    7475    template<typename CharacterType> void syntaxViolation(const CodePointIterator<CharacterType>&);
    7576    template<typename CharacterType> void fragmentSyntaxViolation(const CodePointIterator<CharacterType>&);

  • trunk/Tools/ChangeLog

    r206749 r206758  
     12016-10-03  Alex Christensen  <achristensen@webkit.org>
     2
     3        URLParser should ignore tabs at all locations
     4        https://bugs.webkit.org/show_bug.cgi?id=162836
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
     9        (TestWebKitAPI::checkURL):
     10        (TestWebKitAPI::checkRelativeURL):
     11        (TestWebKitAPI::checkURLDifferences):
     12        (TestWebKitAPI::checkRelativeURLDifferences):
     13        (TestWebKitAPI::TEST_F):
     14
    1152016-10-03  Alex Christensen  <achristensen@webkit.org>
    216

  • trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp

    r206749 r206758  
    7070}
    7171
    72 static void checkURL(const String& urlString, const ExpectedParts& parts, bool checkTabs = true)
     72static String insertTabAtLocation(const String& string, size_t location)
     73{
     74    ASSERT(location <= string.length());
     75    return makeString(string.substring(0, location), "\t", string.substring(location));
     76}
     77
     78static ExpectedParts invalidParts(const String& urlStringWithTab)
     79{
     80    return {"", "", "", "", 0, "" , "", "", urlStringWithTab};
     81}
     82
     83enum class TestTabs { No, Yes };
     84
     85// Inserting tabs between surrogate pairs changes the encoded value instead of being skipped by the URLParser.
     86const TestTabs testTabsValueForSurrogatePairs = TestTabs::No;
     87
     88static void checkURL(const String& urlString, const ExpectedParts& parts, TestTabs testTabs = TestTabs::Yes)
    7389{
    7490    bool wasEnabled = URLParser::enabled();
    … …  
    103119    EXPECT_TRUE(URLParser::internalValuesConsistent(oldURL));
    104120
    105     if (checkTabs) {
     121    if (testTabs == TestTabs::Yes) {
    106122        for (size_t i = 0; i < urlString.length(); ++i) {
    107             String urlStringWithTab = makeString(urlString.substring(0, i), "\t", urlString.substring(i));
    108             ExpectedParts invalidPartsWithTab = {"", "", "", "", 0, "" , "", "", urlStringWithTab};
    109             checkURL(urlStringWithTab, parts.isInvalid() ? invalidPartsWithTab : parts, false);
     123            String urlStringWithTab = insertTabAtLocation(urlString, i);
     124            checkURL(urlStringWithTab,
     125                parts.isInvalid() ? invalidParts(urlStringWithTab) : parts,
     126                TestTabs::No);
    110127        }
    111128    }
    … …  
    274291    checkURL("http://./", {"http", "", "", ".", 0, "/", "", "", "http://./"});
    275292    checkURL("http://.", {"http", "", "", ".", 0, "/", "", "", "http://./"});
    276     checkURL("http://123\t.256/", {"http", "", "", "123.256", 0, "/", "", "", "http://123.256/"});
    277     checkURL("http://123.\t256/", {"http", "", "", "123.256", 0, "/", "", "", "http://123.256/"});
    278293    checkURL("notspecial:/a", {"notspecial", "", "", "", 0, "/a", "", "", "notspecial:/a"});
    279294    checkURL("notspecial:", {"notspecial", "", "", "", 0, "", "", "", "notspecial:"});
    … …  
    306321}
    307322
    308 static void checkRelativeURL(const String& urlString, const String& baseURLString, const ExpectedParts& parts, bool checkTabs = true)
     323static void checkRelativeURL(const String& urlString, const String& baseURLString, const ExpectedParts& parts, TestTabs testTabs = TestTabs::Yes)
    309324{
    310325    bool wasEnabled = URLParser::enabled();
    … …  
    339354    EXPECT_TRUE(URLParser::internalValuesConsistent(oldURL));
    340355   
    341     if (checkTabs) {
     356    if (testTabs == TestTabs::Yes) {
    342357        for (size_t i = 0; i < urlString.length(); ++i) {
    343             String urlStringWithTab = makeString(urlString.substring(0, i), "\t", urlString.substring(i));
    344             ExpectedParts invalidPartsWithTab = {"", "", "", "", 0, "" , "", "", urlStringWithTab};
    345             checkRelativeURL(urlStringWithTab, baseURLString, parts.isInvalid() ? invalidPartsWithTab : parts, false);
     358            String urlStringWithTab = insertTabAtLocation(urlString, i);
     359            checkRelativeURL(urlStringWithTab,
     360                baseURLString,
     361                parts.isInvalid() ? invalidParts(urlStringWithTab) : parts,
     362                TestTabs::No);
    346363        }
    347364    }
    … …  
    423440}
    424441
    425 static void checkURLDifferences(const String& urlString, const ExpectedParts& partsNew, const ExpectedParts& partsOld)
     442static void checkURLDifferences(const String& urlString, const ExpectedParts& partsNew, const ExpectedParts& partsOld, TestTabs testTabs = TestTabs::Yes)
    426443{
    427444    bool wasEnabled = URLParser::enabled();
    … …  
    456473    EXPECT_TRUE(URLParser::internalValuesConsistent(oldURL));
    457474   
    458     // FIXME: check tabs here like we do for checkURL and checkRelativeURL.
    459 }
    460 
    461 static void checkRelativeURLDifferences(const String& urlString, const String& baseURLString, const ExpectedParts& partsNew, const ExpectedParts& partsOld)
     475    if (testTabs == TestTabs::Yes) {
     476        for (size_t i = 0; i < urlString.length(); ++i) {
     477            String urlStringWithTab = insertTabAtLocation(urlString, i);
     478            checkURLDifferences(urlStringWithTab,
     479                partsNew.isInvalid() ? invalidParts(urlStringWithTab) : partsNew,
     480                partsOld.isInvalid() ? invalidParts(urlStringWithTab) : partsOld,
     481                TestTabs::No);
     482        }
     483    }
     484}
     485
     486static void checkRelativeURLDifferences(const String& urlString, const String& baseURLString, const ExpectedParts& partsNew, const ExpectedParts& partsOld, TestTabs testTabs = TestTabs::Yes)
    462487{
    463488    bool wasEnabled = URLParser::enabled();
    … …  
    492517    EXPECT_TRUE(URLParser::internalValuesConsistent(oldURL));
    493518
    494     // FIXME: check tabs here like we do for checkURL and checkRelativeURL.
     519    if (testTabs == TestTabs::Yes) {
     520        for (size_t i = 0; i < urlString.length(); ++i) {
     521            String urlStringWithTab = insertTabAtLocation(urlString, i);
     522            checkRelativeURLDifferences(urlStringWithTab, baseURLString,
     523                partsNew.isInvalid() ? invalidParts(urlStringWithTab) : partsNew,
     524                partsOld.isInvalid() ? invalidParts(urlStringWithTab) : partsOld,
     525                TestTabs::No);
     526        }
     527    }
    495528}
    496529
    … …  
    618651        {"http", "", "", "example.org", 0, "/example.com/", "", "", "http://example.org/example.com/"},
    619652        {"http", "", "", "example.com", 0, "/", "", "", "http://example.com/"});
    620    
     653
    621654    // This behavior matches Chrome and Firefox, but not WebKit using URL::parse.
    622655    // The behavior of URL::parse is clearly wrong because reparsing file://path would make path the host.
    … …  
    648681    checkRelativeURLDifferences(utf16String(u"http://foo:šŸ’©@example.com/bar"), "http://other.com/",
    649682        {"http", "foo", utf16String(u"šŸ’©"), "example.com", 0, "/bar", "", "", "http://foo:%F0%9F%92%A9@example.com/bar"},
    650         {"", "", "", "", 0, "", "", "", utf16String(u"http://foo:šŸ’©@example.com/bar")});
     683        {"", "", "", "", 0, "", "", "", utf16String(u"http://foo:šŸ’©@example.com/bar")}, testTabsValueForSurrogatePairs);
    651684    checkRelativeURLDifferences("http://&a:foo(b]c@d:2/", "http://example.org/foo/bar",
    652685        {"http", "&a", "foo(b]c", "d", 2, "/", "", "", "http://&a:foo(b%5Dc@d:2/"},
    … …  
    702735    checkURLDifferences(utf16String(u"http://host?ƟšŸ˜#ƟšŸ˜"),
    703736        {"http", "", "", "host", 0, "/", "%C3%9F%F0%9F%98%8D", utf16String(u"ƟšŸ˜"), utf16String(u"http://host/?%C3%9F%F0%9F%98%8D#ƟšŸ˜")},
    704         {"http", "", "", "host", 0, "/", "%C3%9F%F0%9F%98%8D", "%C3%9F%F0%9F%98%8D", "http://host/?%C3%9F%F0%9F%98%8D#%C3%9F%F0%9F%98%8D"});
     737        {"http", "", "", "host", 0, "/", "%C3%9F%F0%9F%98%8D", "%C3%9F%F0%9F%98%8D", "http://host/?%C3%9F%F0%9F%98%8D#%C3%9F%F0%9F%98%8D"}, testTabsValueForSurrogatePairs);
    705738    checkURLDifferences(utf16String(u"http://host/path#šŸ’©\tšŸ’©"),
    706739        {"http", "", "", "host", 0, "/path", "", utf16String(u"šŸ’©šŸ’©"), utf16String(u"http://host/path#šŸ’©šŸ’©")},
    … …  
    9761009    const wchar_t invalidSurrogateEnd = 'A';
    9771010    checkURL(utf16String<12>({'h', 't', 't', 'p', ':', '/', '/', 'w', '/', surrogateBegin, validSurrogateEnd, '\0'}),
    978         {"http", "", "", "w", 0, "/%F0%90%85%95", "", "", "http://w/%F0%90%85%95"}, false);
     1011        {"http", "", "", "w", 0, "/%F0%90%85%95", "", "", "http://w/%F0%90%85%95"}, testTabsValueForSurrogatePairs);
    9791012
    9801013    // URLParser matches Chrome and Firefox but not URL::parse.
    … …  
    9961029}
    9971030
    998 static void checkURL(const String& urlString, const TextEncoding& encoding, const ExpectedParts& parts, bool checkTabs = true)
     1031static void checkURL(const String& urlString, const TextEncoding& encoding, const ExpectedParts& parts, TestTabs testTabs = TestTabs::Yes)
    9991032{
    10001033    URLParser parser(urlString, { }, encoding);
    … …  
    10101043    EXPECT_TRUE(eq(parts.string, url.string()));
    10111044
    1012     if (checkTabs) {
     1045    if (testTabs == TestTabs::Yes) {
    10131046        for (size_t i = 0; i < urlString.length(); ++i) {
    1014             String urlStringWithTab = makeString(urlString.substring(0, i), "\t", urlString.substring(i));
    1015             ExpectedParts invalidPartsWithTab = {"", "", "", "", 0, "" , "", "", urlStringWithTab};
    1016             checkURL(urlStringWithTab, encoding, parts.isInvalid() ? invalidPartsWithTab : parts, false);
     1047            String urlStringWithTab = insertTabAtLocation(urlString, i);
     1048            checkURL(urlStringWithTab, encoding,
     1049                parts.isInvalid() ? invalidParts(urlStringWithTab) : parts,
     1050                TestTabs::No);
    10171051        }
    10181052    }
    … …  
    10211055TEST_F(URLParserTest, QueryEncoding)
    10221056{
    1023     checkURL(utf16String(u"http://host?ƟšŸ˜#ƟšŸ˜"), UTF8Encoding(), {"http", "", "", "host", 0, "/", "%C3%9F%F0%9F%98%8D", utf16String(u"ƟšŸ˜"), utf16String(u"http://host/?%C3%9F%F0%9F%98%8D#ƟšŸ˜")}, false);
    1024     checkURL(utf16String(u"http://host?ƟšŸ˜#ƟšŸ˜"), UTF8Encoding(), {"http", "", "", "host", 0, "/", "%C3%9F%F0%9F%98%8D", utf16String(u"ƟšŸ˜"), utf16String(u"http://host/?%C3%9F%F0%9F%98%8D#ƟšŸ˜")}, false);
     1057    checkURL(utf16String(u"http://host?ƟšŸ˜#ƟšŸ˜"), UTF8Encoding(), {"http", "", "", "host", 0, "/", "%C3%9F%F0%9F%98%8D", utf16String(u"ƟšŸ˜"), utf16String(u"http://host/?%C3%9F%F0%9F%98%8D#ƟšŸ˜")}, testTabsValueForSurrogatePairs);
     1058    checkURL(utf16String(u"http://host?ƟšŸ˜#ƟšŸ˜"), UTF8Encoding(), {"http", "", "", "host", 0, "/", "%C3%9F%F0%9F%98%8D", utf16String(u"ƟšŸ˜"), utf16String(u"http://host/?%C3%9F%F0%9F%98%8D#ƟšŸ˜")}, testTabsValueForSurrogatePairs);
    10251059
    10261060    TextEncoding latin1(String("latin1"));
Note: See TracChangeset for help on using the changeset viewer.