Changeset 206231 in webkit

Timestamp:

Sep 21, 2016 1:19:07 PM (8 years ago)

Author:

achristensen@apple.com

Message:

URLParser should fail when parsing invalid relative URLs with no schemes
​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):

Location:

trunk

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/URLParser.cpp (modified) (9 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/platform/URLParser.cpp

@@ -706 +706 @@
 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()) {
@@ -1070 +1075 @@
         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 == '#') {
@@ -1241 +1246 @@
                 break;
             case '?':
-                if (!base.isNull() && base.protocolIs("file"))
+                if (base.isValid() && base.protocolIs("file"))
                     copyURLPartsUntil(base, URLPart::PathEnd);
                 m_asciiBuffer.append("///?", 4);
@@ -1255 +1260 @@
                 break;
             case '#':
-                if (!base.isNull() && base.protocolIs("file"))
+                if (base.isValid() && base.protocolIs("file"))
                     copyURLPartsUntil(base, URLPart::QueryEnd);
                 m_asciiBuffer.append("///#", 4);
@@ -1270 +1275 @@
                 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 {
@@ -1300 +1305 @@
                 break;
             }
-            if (!base.isNull() && base.protocolIs("file")) {
+            if (base.isValid() && base.protocolIs("file")) {
                 // FIXME: This String copy is unnecessary.
                 String basePath = base.path();
@@ -1460 +1465 @@
     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);
@@ -1545 +1550 @@
     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(':');
@@ -2104 +2109 @@
     }
     
-    ASSERT(!serialized || m_hostHasPercentOrNonASCII);
+    ASSERT(!serialized || !m_hostHasPercentOrNonASCII);
     if (!m_hostHasPercentOrNonASCII) {
         auto hostIterator = iterator;

trunk/Tools/ChangeLog

@@ +1 @@
+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>
 

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

@@ -312 +312 @@
     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,
@@ -711 +712 @@
     shouldFail("~", "about:blank");
     shouldFail("~~~");
+    shouldFail("://:0/");
+    shouldFail("://:0/", "");
+    shouldFail("://:0/", "about:blank");
 }