Changeset 205667 in webkit

Timestamp:

Sep 8, 2016 3:15:07 PM (8 years ago)

Author:

achristensen@apple.com

Message:

URLParser should handle URLs with empty authority
​https://bugs.webkit.org/show_bug.cgi?id=161711

Reviewed by Brady Eidson.

Source/WebCore:

Covered by new API tests.

• platform/URLParser.cpp:

(WebCore::URLParser::parse):
(WebCore::URLParser::parseAuthority):

Tools:

• TestWebKitAPI/Tests/WebCore/URLParser.cpp:

(TestWebKitAPI::TEST_F):

Location:

trunk

Files:

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

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-09-08  Alex Christensen  <achristensen@webkit.org>
+
+        URLParser should handle URLs with empty authority
+        https://bugs.webkit.org/show_bug.cgi?id=161711
+
+        Reviewed by Brady Eidson.
+
+        Covered by new API tests.
+
+        * platform/URLParser.cpp:
+        (WebCore::URLParser::parse):
+        (WebCore::URLParser::parseAuthority):
+
 2016-09-08  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/platform/URLParser.cpp

@@ -858 +858 @@
     case State::SpecialAuthorityIgnoreSlashes:
         LOG_FINAL_STATE("SpecialAuthorityIgnoreSlashes");
-        break;
+        return { };
     case State::AuthorityOrHost:
         LOG_FINAL_STATE("AuthorityOrHost");
@@ -975 +975 @@
 void URLParser::parseAuthority(StringView::CodePoints::Iterator& iterator, const StringView::CodePoints::Iterator& end)
 {
+    if (iterator == end) {
+        m_url.m_userEnd = m_buffer.length();
+        m_url.m_passwordEnd = m_url.m_userEnd;
+        return;
+    }
     for (; iterator != end; ++iterator) {
-        m_buffer.append(*iterator);
         if (*iterator == ':') {
             ++iterator;
-            m_url.m_userEnd = m_buffer.length() - 1;
-            break;
-        }
+            m_url.m_userEnd = m_buffer.length();
+            if (iterator == end) {
+                m_url.m_passwordEnd = m_url.m_userEnd;
+                if (m_url.m_userEnd > m_url.m_userStart)
+                    m_buffer.append('@');
+                return;
+            }
+            m_buffer.append(':');
+            break;
+        }
+        m_buffer.append(*iterator);
     }
     for (; iterator != end; ++iterator)

trunk/Tools/ChangeLog

@@ +1 @@
+2016-09-08  Alex Christensen  <achristensen@webkit.org>
+
+        URLParser should handle URLs with empty authority
+        https://bugs.webkit.org/show_bug.cgi?id=161711
+
+        Reviewed by Brady Eidson.
+
+        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+        (TestWebKitAPI::TEST_F):
+
 2016-09-08  Alex Christensen  <achristensen@webkit.org>
 

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

@@ -179 +179 @@
     checkURL("http://host/a%20B", {"http", "", "", "host", 0, "/a%20B", "", "", "http://host/a%20B"});
     checkURL("http://host?q=@ <>!#fragment", {"http", "", "", "host", 0, "/", "q=@%20%3C%3E!", "fragment", "http://host/?q=@%20%3C%3E!#fragment"});
+    checkURL("http://user:@host", {"http", "user", "", "host", 0, "/", "", "", "http://user@host/"});
+
+    // This disagrees with the web platform test for http://:@www.example.com but agrees with Chrome and URL::parse,
+    // and Firefox fails the web platform test differently. Maybe the web platform test ought to be changed.
+    checkURL("http://:@host", {"http", "", "", "host", 0, "/", "", "", "http://host/"});
 }
 
@@ -231 +236 @@
     checkRelativeURL("#?", "http://example.org/foo/bar", {"http", "", "", "example.org", 0, "/foo/bar", "", "?", "http://example.org/foo/bar#?"});
     checkRelativeURL("/", "http://example.org/foo/bar", {"http", "", "", "example.org", 0, "/", "", "", "http://example.org/"});
+    checkRelativeURL("http://@host", "about:blank", {"http", "", "", "host", 0, "/", "", "", "http://host/"});
+    checkRelativeURL("http://:@host", "about:blank", {"http", "", "", "host", 0, "/", "", "", "http://host/"});
 }
 
@@ -355 +362 @@
         {"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"});
-
-    // FIXME: This behavior ought to be specified in the standard.
-    // With the existing URL::parse, WebKit returns "https:/", Firefox returns "https:///", and Chrome throws an error.
+    checkURLDifferences("http://@",
+        {"", "", "", "", 0, "", "", "", ""},
+        {"", "", "", "", 0, "", "", "", "http://@"});
+    checkURLDifferences("http://",
+        {"", "", "", "", 0, "", "", "", ""},
+        {"http", "", "", "", 0, "/", "", "", "http:/"});
     checkRelativeURLDifferences("//", "https://www.webkit.org/path",
-        {"https", "", "", "", 0, "", "", "", "https://"},
+        {"", "", "", "", 0, "", "", "", ""},
         {"https", "", "", "", 0, "/", "", "", "https:/"});
     
@@ -437 +447 @@
     
     // 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@"});
 }