URLParser should fail when parsing invalid relative URLs with no schemes
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 21 Sep 2016 20:19:07 +0000 (20:19 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 21 Sep 2016 20:19:07 +0000 (20:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=162355

Reviewed by Tim Horton.

Source/WebCore:

Covered by new API tests.

* platform/URLParser.cpp:
(WebCore::copyASCIIStringUntil):
When copying from a null String, is8Bit dereferences a null pointer.  We don't want to do that.
(WebCore::URLParser::parse):
What the spec calls a "null" URL matches !url.isValid(), not url.isNull().
The former reflects whether the parsing succeeded,
the latter whether the contained String (which could be an invalid URL) is null.

Tools:

* TestWebKitAPI/Tests/WebCore/URLParser.cpp:
(TestWebKitAPI::TEST_F):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@206231 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/URLParser.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp

index 3873d53..da42df7 100644 (file)
@@ -1,3 +1,20 @@
+2016-09-21  Alex Christensen  <achristensen@webkit.org>
+
+        URLParser should fail when parsing invalid relative URLs with no schemes
+        https://bugs.webkit.org/show_bug.cgi?id=162355
+
+        Reviewed by Tim Horton.
+
+        Covered by new API tests.
+
+        * platform/URLParser.cpp:
+        (WebCore::copyASCIIStringUntil):
+        When copying from a null String, is8Bit dereferences a null pointer.  We don't want to do that.
+        (WebCore::URLParser::parse):
+        What the spec calls a "null" URL matches !url.isValid(), not url.isNull().
+        The former reflects whether the parsing succeeded, 
+        the latter whether the contained String (which could be an invalid URL) is null.
+
 2016-09-21  Antti Koivisto  <antti@apple.com>
 
         Document::styleResolverChanged simplification
index 7e0dbd9..a8cf8d5 100644 (file)
@@ -705,6 +705,11 @@ size_t URLParser::urlLengthUntilPart(const URL& url, URLPart part)
 
 inline static void copyASCIIStringUntil(Vector<LChar>& destination, const String& string, size_t lengthIf8Bit, size_t lengthIf16Bit)
 {
+    if (string.isNull()) {
+        ASSERT(!lengthIf8Bit);
+        ASSERT(!lengthIf16Bit);
+        return;
+    }
     ASSERT(destination.isEmpty());
     if (string.is8Bit()) {
         RELEASE_ASSERT(lengthIf8Bit <= string.length());
@@ -1069,7 +1074,7 @@ URL URLParser::parse(const CharacterType* input, const unsigned length, const UR
             break;
         case State::NoScheme:
             LOG_STATE("NoScheme");
-            if (base.isNull() || (base.m_cannotBeABaseURL && *c != '#'))
+            if (!base.isValid() || (base.m_cannotBeABaseURL && *c != '#'))
                 return failure(input, length);
             if (base.m_cannotBeABaseURL && *c == '#') {
                 copyURLPartsUntil(base, URLPart::QueryEnd);
@@ -1240,7 +1245,7 @@ URL URLParser::parse(const CharacterType* input, const unsigned length, const UR
                 ++c;
                 break;
             case '?':
-                if (!base.isNull() && base.protocolIs("file"))
+                if (base.isValid() && base.protocolIs("file"))
                     copyURLPartsUntil(base, URLPart::PathEnd);
                 m_asciiBuffer.append("///?", 4);
                 m_url.m_userStart = m_asciiBuffer.size() - 2;
@@ -1254,7 +1259,7 @@ URL URLParser::parse(const CharacterType* input, const unsigned length, const UR
                 ++c;
                 break;
             case '#':
-                if (!base.isNull() && base.protocolIs("file"))
+                if (base.isValid() && base.protocolIs("file"))
                     copyURLPartsUntil(base, URLPart::QueryEnd);
                 m_asciiBuffer.append("///#", 4);
                 m_url.m_userStart = m_asciiBuffer.size() - 2;
@@ -1269,7 +1274,7 @@ URL URLParser::parse(const CharacterType* input, const unsigned length, const UR
                 ++c;
                 break;
             default:
-                if (!base.isNull() && base.protocolIs("file") && shouldCopyFileURL<serialized>(c))
+                if (base.isValid() && base.protocolIs("file") && shouldCopyFileURL<serialized>(c))
                     copyURLPartsUntil(base, URLPart::PathAfterLastSlash);
                 else {
                     m_asciiBuffer.append("///", 3);
@@ -1299,7 +1304,7 @@ URL URLParser::parse(const CharacterType* input, const unsigned length, const UR
                 state = State::FileHost;
                 break;
             }
-            if (!base.isNull() && base.protocolIs("file")) {
+            if (base.isValid() && base.protocolIs("file")) {
                 // FIXME: This String copy is unnecessary.
                 String basePath = base.path();
                 if (basePath.length() >= 2) {
@@ -1459,7 +1464,7 @@ URL URLParser::parse(const CharacterType* input, const unsigned length, const UR
     switch (state) {
     case State::SchemeStart:
         LOG_FINAL_STATE("SchemeStart");
-        if (!m_asciiBuffer.size() && !base.isNull())
+        if (!m_asciiBuffer.size() && base.isValid())
             return base;
         return failure(input, length);
     case State::Scheme:
@@ -1544,7 +1549,7 @@ URL URLParser::parse(const CharacterType* input, const unsigned length, const UR
         break;
     case State::File:
         LOG_FINAL_STATE("File");
-        if (!base.isNull() && base.protocolIs("file")) {
+        if (base.isValid() && base.protocolIs("file")) {
             copyURLPartsUntil(base, URLPart::QueryEnd);
             m_asciiBuffer.append(':');
         }
@@ -2103,7 +2108,7 @@ bool URLParser::parseHostAndPort(CodePointIterator<CharacterType> iterator)
         }
     }
     
-    ASSERT(!serialized || m_hostHasPercentOrNonASCII);
+    ASSERT(!serialized || !m_hostHasPercentOrNonASCII);
     if (!m_hostHasPercentOrNonASCII) {
         auto hostIterator = iterator;
         for (; !iterator.atEnd(); ++iterator) {
index 840b673..ef9b4a2 100644 (file)
@@ -1,3 +1,13 @@
+2016-09-21  Alex Christensen  <achristensen@webkit.org>
+
+        URLParser should fail when parsing invalid relative URLs with no schemes
+        https://bugs.webkit.org/show_bug.cgi?id=162355
+
+        Reviewed by Tim Horton.
+
+        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+        (TestWebKitAPI::TEST_F):
+
 2016-09-21  Keith Miller  <keith_miller@apple.com>
 
         Fix build for future versions of Clang.
index 80fdb5f..55c93fe 100644 (file)
@@ -311,6 +311,7 @@ TEST_F(URLParserTest, ParseRelative)
     checkRelativeURL("notspecial:/", "about:blank", {"notspecial", "", "", "", 0, "/", "", "", "notspecial:/"});
     checkRelativeURL("notspecial:/", "http://host", {"notspecial", "", "", "", 0, "/", "", "", "notspecial:/"});
     checkRelativeURL("foo:/", "http://example.org/foo/bar", {"foo", "", "", "", 0, "/", "", "", "foo:/"});
+    checkRelativeURL("://:0/", "http://webkit.org/", {"http", "", "", "webkit.org", 0, "/://:0/", "", "", "http://webkit.org/://:0/"});
 
     // The checking of slashes in SpecialAuthoritySlashes needed to get this to pass contradicts what is in the spec,
     // but it is included in the web platform tests.
@@ -710,6 +711,9 @@ TEST_F(URLParserTest, ParserFailures)
     shouldFail("~");
     shouldFail("~", "about:blank");
     shouldFail("~~~");
+    shouldFail("://:0/");
+    shouldFail("://:0/", "");
+    shouldFail("://:0/", "about:blank");
 }
 
 // These are in the spec but not in the web platform tests.