Changeset 206162 in webkit


Ignore:
Timestamp:
Sep 20, 2016 11:46:26 AM (8 years ago)
Author:
achristensen@apple.com
Message:

Align URLParser with web platform tests when parsing non-special relative URLs ending in AuthorityOrHost state
​https://bugs.webkit.org/show_bug.cgi?id=162251

Reviewed by Tim Horton.

Source/WebCore:

Covered by new and updated API tests.

  • platform/URLParser.cpp:

(WebCore::URLParser::parse):
Fix parsing of non-special URLs that end after scheme:// with no authority.
We used to assume that parsing non-special schemes would never end with just scheme:// but a string can indeed end right there.
When a non-special relative URL contains just scheme:// we need the resulting URL to be valid to conform with the web platform tests.
(WebCore::URLParser::parseHostAndPort):
Renamed to reflect what the function actually does.
(WebCore::URLParser::internalValuesConsistent):
Add utility function for testing.
(WebCore::URLParser::parseHost): Deleted.

  • platform/URLParser.h:

Tools:

  • TestWebKitAPI/Tests/WebCore/URLParser.cpp:

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

Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r206161 r206162  
     12016-09-20  Alex Christensen  <achristensen@webkit.org>
     2
     3        Align URLParser with web platform tests when parsing non-special relative URLs ending in AuthorityOrHost state
     4        https://bugs.webkit.org/show_bug.cgi?id=162251
     5
     6        Reviewed by Tim Horton.
     7
     8        Covered by new and updated API tests.
     9
     10        * platform/URLParser.cpp:
     11        (WebCore::URLParser::parse):
     12        Fix parsing of non-special URLs that end after scheme:// with no authority.
     13        We used to assume that parsing non-special schemes would never end with just scheme:// but a string can indeed end right there.
     14        When a non-special relative URL contains just scheme:// we need the resulting URL to be valid to conform with the web platform tests.
     15        (WebCore::URLParser::parseHostAndPort):
     16        Renamed to reflect what the function actually does.
     17        (WebCore::URLParser::internalValuesConsistent):
     18        Add utility function for testing.
     19        (WebCore::URLParser::parseHost): Deleted.
     20        * platform/URLParser.h:
     21
    1222016-09-20  Javier Fernandez  <jfernandez@igalia.com>
    223

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

    r206159 r206162  
    10001000                        state = State::SpecialAuthoritySlashes;
    10011001                } else {
    1002                     m_url.m_userStart = m_asciiBuffer.size();
    1003                     m_url.m_userEnd = m_url.m_userStart;
    1004                     m_url.m_passwordEnd = m_url.m_userStart;
    1005                     m_url.m_hostEnd = m_url.m_userStart;
    1006                     m_url.m_portEnd = m_url.m_userStart;
    10071002                    auto maybeSlash = c;
    10081003                    incrementIteratorSkippingTabAndNewLine<serialized>(maybeSlash);
    10091004                    if (!maybeSlash.atEnd() && *maybeSlash == '/') {
    10101005                        m_asciiBuffer.append('/');
    1011                         m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
     1006                        m_url.m_userStart = m_asciiBuffer.size();
    10121007                        state = State::PathOrAuthority;
    10131008                        c = maybeSlash;
    10141009                        ASSERT(*c == '/');
    10151010                    } else {
     1011                        m_url.m_userStart = m_asciiBuffer.size();
     1012                        m_url.m_userEnd = m_url.m_userStart;
     1013                        m_url.m_passwordEnd = m_url.m_userStart;
     1014                        m_url.m_hostEnd = m_url.m_userStart;
     1015                        m_url.m_portEnd = m_url.m_userStart;
    10161016                        m_url.m_pathAfterLastSlash = m_url.m_userStart;
    10171017                        m_url.m_cannotBeABaseURL = true;
    … …  
    10771077                ++c;
    10781078                authorityOrHostBegin = c;
    1079             } else
     1079            } else {
     1080                ASSERT(m_asciiBuffer.last() == '/');
     1081                m_url.m_userStart = m_asciiBuffer.size() - 1;
     1082                m_url.m_userEnd = m_url.m_userStart;
     1083                m_url.m_passwordEnd = m_url.m_userStart;
     1084                m_url.m_hostEnd = m_url.m_userStart;
     1085                m_url.m_portEnd = m_url.m_userStart;
     1086                m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
    10801087                state = State::Path;
     1088            }
    10811089            break;
    10821090        case State::Relative:
    … …  
    11631171                    m_url.m_userEnd = m_asciiBuffer.size();
    11641172                    m_url.m_passwordEnd = m_url.m_userEnd;
    1165                     if (!parseHost<serialized>(CodePointIterator<CharacterType>(authorityOrHostBegin, c)))
     1173                    if (!parseHostAndPort<serialized>(CodePointIterator<CharacterType>(authorityOrHostBegin, c)))
    11661174                        return failure(input, length);
    11671175                    if (!isSlash) {
    … …  
    11801188            LOG_STATE("Host");
    11811189            if (*c == '/' || *c == '?' || *c == '#') {
    1182                 if (!parseHost<serialized>(CodePointIterator<CharacterType>(authorityOrHostBegin, c)))
     1190                if (!parseHostAndPort<serialized>(CodePointIterator<CharacterType>(authorityOrHostBegin, c)))
    11831191                    return failure(input, length);
    11841192                state = State::Path;
    … …  
    13101318                    break;
    13111319                }
    1312                 if (!parseHost<serialized>(CodePointIterator<CharacterType>(authorityOrHostBegin, c)))
     1320                if (!parseHostAndPort<serialized>(CodePointIterator<CharacterType>(authorityOrHostBegin, c)))
    13131321                    return failure(input, length);
    13141322               
    … …  
    14341442    case State::PathOrAuthority:
    14351443        LOG_FINAL_STATE("PathOrAuthority");
     1444        m_url.m_userEnd = m_asciiBuffer.size();
     1445        m_url.m_passwordEnd = m_url.m_userEnd;
     1446        m_url.m_hostEnd = m_url.m_userEnd;
     1447        m_url.m_portEnd = m_url.m_userEnd;
     1448        m_url.m_pathAfterLastSlash = m_url.m_userEnd;
    14361449        m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
    14371450        m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
    … …  
    14711484        m_url.m_userEnd = m_asciiBuffer.size();
    14721485        m_url.m_passwordEnd = m_url.m_userEnd;
    1473         FALLTHROUGH;
     1486        if (authorityOrHostBegin.atEnd()) {
     1487            m_url.m_hostEnd = m_url.m_userEnd;
     1488            m_url.m_portEnd = m_url.m_userEnd;
     1489        } else if (!parseHostAndPort<serialized>(authorityOrHostBegin))
     1490            return failure(input, length);
     1491        m_asciiBuffer.append('/');
     1492        m_url.m_pathEnd = m_url.m_portEnd + 1;
     1493        m_url.m_pathAfterLastSlash = m_url.m_pathEnd;
     1494        m_url.m_queryEnd = m_url.m_pathEnd;
     1495        m_url.m_fragmentEnd = m_url.m_pathEnd;
     1496        break;
    14741497    case State::Host:
    1475         if (state == State::Host)
    1476             LOG_FINAL_STATE("Host");
    1477         if (!parseHost<serialized>(authorityOrHostBegin))
     1498        LOG_FINAL_STATE("Host");
     1499        if (!parseHostAndPort<serialized>(authorityOrHostBegin))
    14781500            return failure(input, length);
    14791501        m_asciiBuffer.append('/');
    … …  
    15291551        }
    15301552
    1531         if (!parseHost<serialized>(CodePointIterator<CharacterType>(authorityOrHostBegin, c)))
     1553        if (!parseHostAndPort<serialized>(CodePointIterator<CharacterType>(authorityOrHostBegin, c)))
    15321554            return failure(input, length);
    15331555
    … …  
    15841606    m_url.m_isValid = true;
    15851607    LOG(URLParser, "Parsed URL <%s>", m_url.m_string.utf8().data());
     1608    ASSERT(internalValuesConsistent(m_url));
    15861609    return m_url;
    15871610}
    … …  
    20072030
    20082031template<bool serialized, typename CharacterType>
    2009 bool URLParser::parseHost(CodePointIterator<CharacterType> iterator)
     2032bool URLParser::parseHostAndPort(CodePointIterator<CharacterType> iterator)
    20102033{
    20112034    if (iterator.atEnd())
    … …  
    22252248}
    22262249
     2250bool URLParser::internalValuesConsistent(const URL& url)
     2251{   
     2252    return url.m_schemeEnd <= url.m_userStart
     2253        && url.m_userStart <= url.m_userEnd
     2254        && url.m_userEnd <= url.m_passwordEnd
     2255        && url.m_passwordEnd <= url.m_hostEnd
     2256        && url.m_hostEnd <= url.m_hostEnd
     2257        && url.m_portEnd <= url.m_pathAfterLastSlash
     2258        && url.m_pathAfterLastSlash <= url.m_pathEnd
     2259        && url.m_pathEnd <= url.m_queryEnd
     2260        && url.m_queryEnd <= url.m_fragmentEnd
     2261        && (url.m_isValid ? url.m_fragmentEnd == url.m_string.length() : !url.m_fragmentEnd);
     2262    // FIXME: Why do we even store m_fragmentEnd?
     2263    // It should be able to be deduced from m_isValid and m_string.length() to save memory.
     2264}
     2265
    22272266static bool urlParserEnabled = false;
    22282267

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

    r206125 r206162  
    3939    WEBCORE_EXPORT URL parse(const String&, const URL& = { }, const TextEncoding& = UTF8Encoding());
    4040    WEBCORE_EXPORT URL parseSerializedURL(const String&);
     41
    4142    WEBCORE_EXPORT static bool allValuesEqual(const URL&, const URL&);
     43    WEBCORE_EXPORT static bool internalValuesConsistent(const URL&);
    4244
    4345    WEBCORE_EXPORT static bool enabled();
    … …  
    5759    template<bool serialized, typename CharacterType> URL parse(const CharacterType*, const unsigned length, const URL&, const TextEncoding&);
    5860    template<bool serialized, typename CharacterType> void parseAuthority(CodePointIterator<CharacterType>);
    59     template<bool serialized, typename CharacterType> bool parseHost(CodePointIterator<CharacterType>);
     61    template<bool serialized, typename CharacterType> bool parseHostAndPort(CodePointIterator<CharacterType>);
    6062    template<bool serialized, typename CharacterType> bool parsePort(CodePointIterator<CharacterType>&);
    6163    template<typename CharacterType> URL failure(const CharacterType*, unsigned length);

  • trunk/Tools/ChangeLog

    r206159 r206162  
     12016-09-20  Alex Christensen  <achristensen@webkit.org>
     2
     3        Align URLParser with web platform tests when parsing non-special relative URLs ending in AuthorityOrHost state
     4        https://bugs.webkit.org/show_bug.cgi?id=162251
     5
     6        Reviewed by Tim Horton.
     7
     8        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
     9        (TestWebKitAPI::checkURL):
     10        (TestWebKitAPI::TEST_F):
     11        (TestWebKitAPI::checkRelativeURL):
     12        (TestWebKitAPI::checkURLDifferences):
     13        (TestWebKitAPI::checkRelativeURLDifferences):
     14
    1152016-09-20  Alex Christensen  <achristensen@webkit.org>
    216

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

    r206159 r206162  
    8383   
    8484    EXPECT_TRUE(URLParser::allValuesEqual(url, oldURL));
     85    EXPECT_TRUE(URLParser::internalValuesConsistent(url));
     86    EXPECT_TRUE(URLParser::internalValuesConsistent(oldURL));
    8587}
    8688
    … …  
    202204    checkURL("sc:/pa", {"sc", "", "", "", 0, "/pa", "", "", "sc:/pa"});
    203205    checkURL("sc:/pa/", {"sc", "", "", "", 0, "/pa/", "", "", "sc:/pa/"});
     206    checkURL("notspecial:/notuser:notpassword@nothost", {"notspecial", "", "", "", 0, "/notuser:notpassword@nothost", "", "", "notspecial:/notuser:notpassword@nothost"});
    204207    checkURL("sc://pa/", {"sc", "", "", "pa", 0, "/", "", "", "sc://pa/"});
    205208    checkURL("http://host   \a   ", {"http", "", "", "host", 0, "/", "", "", "http://host/"});
    206     checkURL("notspecial:/", {"notspecial", "", "", "", 0, "/", "", "", "notspecial:/"});
    207209    checkURL("notspecial:/a", {"notspecial", "", "", "", 0, "/a", "", "", "notspecial:/a"});
    208210    checkURL("notspecial:", {"notspecial", "", "", "", 0, "", "", "", "notspecial:"});
    … …  
    211213    checkURL("http://256./", {"http", "", "", "256.", 0, "/", "", "", "http://256./"});
    212214    checkURL("http://123.256/", {"http", "", "", "123.256", 0, "/", "", "", "http://123.256/"});
     215    checkURL("notspecial:/a", {"notspecial", "", "", "", 0, "/a", "", "", "notspecial:/a"});
     216    checkURL("notspecial:", {"notspecial", "", "", "", 0, "", "", "", "notspecial:"});
    213217    // FIXME: Fix and add a test with an invalid surrogate pair at the end with a space as the second code unit.
    214218
    … …  
    247251
    248252    EXPECT_TRUE(URLParser::allValuesEqual(url, oldURL));
     253    EXPECT_TRUE(URLParser::internalValuesConsistent(url));
     254    EXPECT_TRUE(URLParser::internalValuesConsistent(oldURL));
    249255}
    250256
    … …  
    288294    checkRelativeURL(" \a baz", "http://example.org/foo/bar", {"http", "", "", "example.org", 0, "/foo/baz", "", "", "http://example.org/foo/baz"});
    289295    checkRelativeURL("~", "http://example.org", {"http", "", "", "example.org", 0, "/~", "", "", "http://example.org/~"});
    290     checkRelativeURL("notspecial:/", "about:blank", {"notspecial", "", "", "", 0, "/", "", "", "notspecial:/"});
    291296    checkRelativeURL("notspecial:", "about:blank", {"notspecial", "", "", "", 0, "", "", "", "notspecial:"});
    292     checkRelativeURL("notspecial:/", "http://host", {"notspecial", "", "", "", 0, "/", "", "", "notspecial:/"});
    293297    checkRelativeURL("notspecial:", "http://host", {"notspecial", "", "", "", 0, "", "", "", "notspecial:"});
    294298    checkRelativeURL("http:", "http://host", {"http", "", "", "host", 0, "/", "", "", "http://host/"});
    … …  
    325329   
    326330    EXPECT_FALSE(URLParser::allValuesEqual(url, oldURL));
     331    EXPECT_TRUE(URLParser::internalValuesConsistent(url));
     332    EXPECT_TRUE(URLParser::internalValuesConsistent(oldURL));
    327333}
    328334
    … …  
    356362   
    357363    EXPECT_FALSE(URLParser::allValuesEqual(url, oldURL));
     364    EXPECT_TRUE(URLParser::internalValuesConsistent(url));
     365    EXPECT_TRUE(URLParser::internalValuesConsistent(oldURL));
    358366}
    359367
    … …  
    548556        {"https", "@test@test", "", "example", 800, "/", "", "", "https://%40test%40test@example:800/"},
    549557        {"", "", "", "", 0, "", "", "", "https://@test@test@example:800/"});
     558    checkRelativeURLDifferences("foo://", "http://example.org/foo/bar",
     559        {"foo", "", "", "", 0, "/", "", "", "foo:///"},
     560        {"foo", "", "", "", 0, "//", "", "", "foo://"});
     561
     562    // This matches the spec and web platform tests, but not Chrome, Firefox, or URL::parse.
     563    checkRelativeURLDifferences("notspecial:/", "about:blank",
     564        {"notspecial", "", "", "", 0, "", "", "", "notspecial:/"},
     565        {"notspecial", "", "", "", 0, "/", "", "", "notspecial:/"});
     566    checkRelativeURLDifferences("notspecial:/", "http://host",
     567        {"notspecial", "", "", "", 0, "", "", "", "notspecial:/"},
     568        {"notspecial", "", "", "", 0, "/", "", "", "notspecial:/"});
     569    checkURLDifferences("notspecial:/",
     570        {"notspecial", "", "", "", 0, "", "", "", "notspecial:/"},
     571        {"notspecial", "", "", "", 0, "/", "", "", "notspecial:/"});
    550572}
    551573
Note: See TracChangeset for help on using the changeset viewer.