Title: [205678] trunk
Revision
205678
Author
[email protected]
Date
2016-09-08 17:43:50 -0700 (Thu, 08 Sep 2016)

Log Message

URLParser failures should preserve the original input string
https://bugs.webkit.org/show_bug.cgi?id=161769

Reviewed by Tim Horton.

Source/WebCore:

No new tests, but covered by updates to API tests.
This also represents many newly passing web platform tests when using URLParser.

* platform/URLParser.cpp:
(WebCore::URLParser::failure):
(WebCore::URLParser::parse):
* platform/URLParser.h:

Tools:

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

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (205677 => 205678)


--- trunk/Source/WebCore/ChangeLog	2016-09-09 00:05:36 UTC (rev 205677)
+++ trunk/Source/WebCore/ChangeLog	2016-09-09 00:43:50 UTC (rev 205678)
@@ -1,5 +1,20 @@
 2016-09-08  Alex Christensen  <[email protected]>
 
+        URLParser failures should preserve the original input string
+        https://bugs.webkit.org/show_bug.cgi?id=161769
+
+        Reviewed by Tim Horton.
+
+        No new tests, but covered by updates to API tests.
+        This also represents many newly passing web platform tests when using URLParser.
+
+        * platform/URLParser.cpp:
+        (WebCore::URLParser::failure):
+        (WebCore::URLParser::parse):
+        * platform/URLParser.h:
+
+2016-09-08  Alex Christensen  <[email protected]>
+
         URLParser should parse URLs with a user but no password
         https://bugs.webkit.org/show_bug.cgi?id=161773
 

Modified: trunk/Source/WebCore/platform/URLParser.cpp (205677 => 205678)


--- trunk/Source/WebCore/platform/URLParser.cpp	2016-09-09 00:05:36 UTC (rev 205677)
+++ trunk/Source/WebCore/platform/URLParser.cpp	2016-09-09 00:43:50 UTC (rev 205678)
@@ -358,6 +358,25 @@
     m_buffer.resize(m_url.m_pathAfterLastSlash);
 }
 
+URL URLParser::failure(const String& input)
+{
+    URL url;
+    url.m_isValid = false;
+    url.m_protocolIsInHTTPFamily = false;
+    url.m_schemeEnd = 0;
+    url.m_userStart = 0;
+    url.m_userEnd = 0;
+    url.m_passwordEnd = 0;
+    url.m_hostEnd = 0;
+    url.m_portEnd = 0;
+    url.m_pathAfterLastSlash = 0;
+    url.m_pathEnd = 0;
+    url.m_queryEnd = 0;
+    url.m_fragmentEnd = 0;
+    url.m_string = input;
+    return url;
+}
+
 URL URLParser::parse(const String& input, const URL& base, const TextEncoding& encoding)
 {
     LOG(URLParser, "Parsing URL <%s> base <%s>", input.utf8().data(), base.string().utf8().data());
@@ -483,7 +502,7 @@
                     state = State::Fragment;
                     ++c;
                 } else
-                    return { };
+                    return failure(input);
             } else if (base.protocol() == "file") {
                 copyURLPartsUntil(base, URLPart::SchemeEnd);
                 m_buffer.append(':');
@@ -499,7 +518,7 @@
                 while (c != end && isTabOrNewline(*c))
                     ++c;
                 if (c == end)
-                    return { };
+                    return failure(input);
                 if (*c == '/') {
                     m_buffer.append('/');
                     state = State::SpecialAuthorityIgnoreSlashes;
@@ -564,7 +583,7 @@
                 while (c != end && isTabOrNewline(*c))
                     ++c;
                 if (c == end)
-                    return { };
+                    return failure(input);
                 m_buffer.append('/');
                 if (*c == '/') {
                     m_buffer.append('/');
@@ -597,7 +616,7 @@
                 m_url.m_userEnd = m_buffer.length();
                 m_url.m_passwordEnd = m_url.m_userEnd;
                 if (!parseHost(authorityOrHostBegin, c))
-                    return { };
+                    return failure(input);
                 if (*c != '/') {
                     m_buffer.append('/');
                     m_url.m_pathAfterLastSlash = m_buffer.length();
@@ -611,7 +630,7 @@
             LOG_STATE("Host");
             if (*c == '/' || *c == '?' || *c == '#') {
                 if (!parseHost(authorityOrHostBegin, c))
-                    return { };
+                    return failure(input);
                 state = State::Path;
                 break;
             }
@@ -734,7 +753,7 @@
                     break;
                 }
                 if (!parseHost(authorityOrHostBegin, c))
-                    return { };
+                    return failure(input);
                 
                 // FIXME: Don't allocate a new string for this comparison.
                 if (m_buffer.toString().substring(m_url.m_passwordEnd) == "localhost")  {
@@ -833,7 +852,7 @@
     switch (state) {
     case State::SchemeStart:
         LOG_FINAL_STATE("SchemeStart");
-        return { };
+        return failure(input);
     case State::Scheme:
         LOG_FINAL_STATE("Scheme");
         break;
@@ -867,7 +886,7 @@
         break;
     case State::SpecialAuthorityIgnoreSlashes:
         LOG_FINAL_STATE("SpecialAuthorityIgnoreSlashes");
-        return { };
+        return failure(input);
     case State::AuthorityOrHost:
         LOG_FINAL_STATE("AuthorityOrHost");
         m_url.m_userEnd = m_buffer.length();
@@ -877,7 +896,7 @@
         if (state == State::Host)
             LOG_FINAL_STATE("Host");
         if (!parseHost(authorityOrHostBegin, end))
-            return { };
+            return failure(input);
         m_buffer.append('/');
         m_url.m_pathEnd = m_url.m_portEnd + 1;
         m_url.m_pathAfterLastSlash = m_url.m_pathEnd;
@@ -935,7 +954,7 @@
         m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
         m_url.m_fragmentEnd = m_url.m_pathAfterLastSlash;
         if (!parseHost(authorityOrHostBegin, c))
-            return { };
+            return failure(input);
         
         // FIXME: Don't allocate a new string for this comparison.
         if (m_buffer.toString().substring(m_url.m_passwordEnd) == "localhost")  {

Modified: trunk/Source/WebCore/platform/URLParser.h (205677 => 205678)


--- trunk/Source/WebCore/platform/URLParser.h	2016-09-09 00:05:36 UTC (rev 205677)
+++ trunk/Source/WebCore/platform/URLParser.h	2016-09-09 00:43:50 UTC (rev 205678)
@@ -45,6 +45,7 @@
     void parseAuthority(StringView::CodePoints::Iterator&, const StringView::CodePoints::Iterator& end);
     bool parseHost(StringView::CodePoints::Iterator&, const StringView::CodePoints::Iterator& end);
     bool parsePort(StringView::CodePoints::Iterator&, const StringView::CodePoints::Iterator& end);
+    URL failure(const String& input);
 
     enum class URLPart;
     void copyURLPartsUntil(const URL& base, URLPart);

Modified: trunk/Tools/ChangeLog (205677 => 205678)


--- trunk/Tools/ChangeLog	2016-09-09 00:05:36 UTC (rev 205677)
+++ trunk/Tools/ChangeLog	2016-09-09 00:43:50 UTC (rev 205678)
@@ -1,5 +1,16 @@
 2016-09-08  Alex Christensen  <[email protected]>
 
+        URLParser failures should preserve the original input string
+        https://bugs.webkit.org/show_bug.cgi?id=161769
+
+        Reviewed by Tim Horton.
+
+        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+        (TestWebKitAPI::TEST_F):
+        (TestWebKitAPI::shouldFail):
+
+2016-09-08  Alex Christensen  <[email protected]>
+
         URLParser should parse URLs with a user but no password
         https://bugs.webkit.org/show_bug.cgi?id=161773
 

Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp (205677 => 205678)


--- trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp	2016-09-09 00:05:36 UTC (rev 205677)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp	2016-09-09 00:43:50 UTC (rev 205678)
@@ -378,35 +378,23 @@
     checkRelativeURLDifferences(wideString(L"#β"), "http://example.org/foo/bar",
         {"http", "", "", "example.org", 0, "/foo/bar", "", wideString(L"β"), wideString(L"http://example.org/foo/bar#β")},
         {"http", "", "", "example.org", 0, "/foo/bar", "", "%CE%B2", "http://example.org/foo/bar#%CE%B2"});
-    checkURLDifferences("http://@",
-        {"", "", "", "", 0, "", "", "", ""},
-        {"", "", "", "", 0, "", "", "", "http://@"});
     checkURLDifferences("http://",
-        {"", "", "", "", 0, "", "", "", ""},
+        {"", "", "", "", 0, "", "", "", "http://"},
         {"http", "", "", "", 0, "/", "", "", "http:/"});
     checkRelativeURLDifferences("//", "https://www.webkit.org/path",
-        {"", "", "", "", 0, "", "", "", ""},
+        {"", "", "", "", 0, "", "", "", "//"},
         {"https", "", "", "", 0, "/", "", "", "https:/"});
     checkURLDifferences("http://127.0.0.1:65536/path",
-        {"", "", "", "", 0, "", "", "", ""},
+        {"", "", "", "", 0, "", "", "", "http://127.0.0.1:65536/path"},
         {"http", "", "", "127.0.0.1", 65535, "/path", "", "", "http://127.0.0.1:65536/path"});
-    checkURLDifferences("http://host:abc",
-        {"", "", "", "", 0, "", "", "", ""},
-        {"", "", "", "", 0, "", "", "", "http://host:abc"});
     checkURLDifferences("http://host:65536",
-        {"", "", "", "", 0, "", "", "", ""},
+        {"", "", "", "", 0, "", "", "", "http://host:65536"},
         {"http", "", "", "host", 65535, "/", "", "", "http://host:65536/"});
-    checkURLDifferences("http://127.0.0.1:abc",
-        {"", "", "", "", 0, "", "", "", ""},
-        {"", "", "", "", 0, "", "", "", "http://127.0.0.1:abc"});
     checkURLDifferences("http://127.0.0.1:65536",
-        {"", "", "", "", 0, "", "", "", ""},
+        {"", "", "", "", 0, "", "", "", "http://127.0.0.1:65536"},
         {"http", "", "", "127.0.0.1", 65535, "/", "", "", "http://127.0.0.1:65536/"});
-    checkURLDifferences("http://[0:f::f:f:0:0]:abc",
-        {"", "", "", "", 0, "", "", "", ""},
-        {"", "", "", "", 0, "", "", "", "http://[0:f::f:f:0:0]:abc"});
     checkURLDifferences("http://[0:f::f:f:0:0]:65536",
-        {"", "", "", "", 0, "", "", "", ""},
+        {"", "", "", "", 0, "", "", "", "http://[0:f::f:f:0:0]:65536"},
         {"http", "", "", "[0:f::f:f:0:0]", 65535, "/", "", "", "http://[0:f::f:f:0:0]:65536/"});
     
     // This behavior matches Chrome and Firefox, but not WebKit using URL::parse.
@@ -484,24 +472,6 @@
         {"wss", "", "", "host", 444, "", "", "", "wss://host:444"});
     
     // FIXME: Fix and check unknown schemes with ports, as well as ftps.
-
-    // Firefox returns http://a:@/ Chrome fails, URL::parse fails
-    checkRelativeURLDifferences("http://a:@", "about:blank",
-        {"", "", "", "", 0, "", "", "", ""},
-        {"", "", "", "", 0, "", "", "", "http://a:@"});
-    
-    checkRelativeURLDifferences("http://:@", "about:blank",
-        {"", "", "", "", 0, "", "", "", ""},
-        {"", "", "", "", 0, "", "", "", "http://:@"});
-    checkRelativeURLDifferences("http://:b@", "about:blank",
-        {"", "", "", "", 0, "", "", "", ""},
-        {"", "", "", "", 0, "", "", "", "http://:b@"});
-    checkURLDifferences("http://a:@",
-        {"", "", "", "", 0, "", "", "", ""},
-        {"", "", "", "", 0, "", "", "", "http://a:@"});
-    checkURLDifferences("http://:b@",
-        {"", "", "", "", 0, "", "", "", ""},
-        {"", "", "", "", 0, "", "", "", "http://:b@"});
 }
     
 static void shouldFail(const String& urlString)
@@ -508,13 +478,29 @@
 {
     URLParser parser;
     auto invalidURL = parser.parse(urlString);
-    EXPECT_TRUE(URLParser::allValuesEqual(invalidURL, { }));
+    checkURL(urlString, {"", "", "", "", 0, "", "", "", urlString});
 }
+
+static void shouldFail(const String& urlString, const String& baseString)
+{
+    URLParser parser;
+    auto invalidURL = parser.parse(urlString);
+    checkRelativeURL(urlString, baseString, {"", "", "", "", 0, "", "", "", urlString});
+}
     
 TEST_F(URLParserTest, ParserFailures)
 {
     shouldFail("    ");
     shouldFail("");
+    shouldFail("http://127.0.0.1:abc");
+    shouldFail("http://host:abc");
+    shouldFail("http://a:@", "about:blank");
+    shouldFail("http://:b@", "about:blank");
+    shouldFail("http://:@", "about:blank");
+    shouldFail("http://a:@");
+    shouldFail("http://:b@");
+    shouldFail("http://@");
+    shouldFail("http://[0:f::f:f:0:0]:abc");
 }
 
 } // namespace TestWebKitAPI
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to