[webkit-changes] [267837] trunk

achristensen Thu, 01 Oct 2020 10:06:45 -0700

Title: [267837] trunk

Revision: 267837
Author: achristen...@apple.com
Date: 2020-10-01 10:05:41 -0700 (Thu, 01 Oct 2020)

Log Message

Non-special URLs are not idempotent
https://bugs.webkit.org/show_bug.cgi?id=215762

Reviewed by Tim Horton.

LayoutTests/imported/w3c:

* web-platform-tests/url/a-element-expected.txt:
* web-platform-tests/url/a-element-xhtml-expected.txt:
* web-platform-tests/url/url-constructor-expected.txt:
* web-platform-tests/url/url-setters-expected.txt:

Source/WTF:

https://github.com/whatwg/url/pull/505 added an interesting edge case to the URL serialization:
"If url’s host is null, url’s path’s size is greater than 1, and url’s path[0] is the empty string, then append U+002F (/) followed by U+002E (.) to output."
The problem was that URLs like "a:/a/..//a" would be parsed into "a://a" with a pathname of "//a" and an empty host. If "a://a" was then reparsed, it would again have an href of "a://a"
but its host would be "a" and it would have an empty path. There is consensus that URL parsing should be idempotent, so we need to do something different here.
According to https://github.com/whatwg/url/issues/415#issuecomment-419197290 this follows what Edge did (and then subsequently abandoned when they switched to Chromium)
to make URL parsing idempotent by adding "/." before the path in the edge case of a URL with a non-special scheme (not http, https, wss, etc.) and a null host and a non-empty path that
has an empty first segment. All the members of the URL remain unchanged except the full serialization (href). This is not important in practice, but important in theory.

Our URL parser tries very hard to use the exact same WTF::String object given as input if it can. However, this step is better implemented as a post-processing step that will almost never happen
because otherwise we would have to parse the entire path twice to find out if we need to add "./" or if the "./" that may have already been there needs to stay. This is illustrated with the test URL
"t:/.//p/../../../..//x" which does need the "./".

In the common case, this adds one well-predicted branch to URL parsing, so I expect performance to be unaffected. Since this is such a rare edge case of URLs, I expect no compatibility problems.

* wtf/URL.cpp:
(WTF::URL::pathStart const):
* wtf/URL.h:
(WTF::URL::pathStart const): Deleted.
* wtf/URLParser.cpp:
(WTF::URLParser::copyURLPartsUntil):
(WTF::URLParser::URLParser):
(WTF::URLParser::needsNonSpecialDotSlash const):
(WTF::URLParser::addNonSpecialDotSlash):
* wtf/URLParser.h:

Tools:

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

Modified Paths

trunk/LayoutTests/imported/w3c/ChangeLog
trunk/LayoutTests/imported/w3c/web-platform-tests/url/a-element-expected.txt
trunk/LayoutTests/imported/w3c/web-platform-tests/url/a-element-xhtml-expected.txt
trunk/LayoutTests/imported/w3c/web-platform-tests/url/url-constructor-expected.txt
trunk/LayoutTests/imported/w3c/web-platform-tests/url/url-setters-expected.txt
trunk/Source/WTF/ChangeLog
trunk/Source/WTF/wtf/URL.cpp
trunk/Source/WTF/wtf/URL.h
trunk/Source/WTF/wtf/URLParser.cpp
trunk/Source/WTF/wtf/URLParser.h
trunk/Tools/ChangeLog
trunk/Tools/TestWebKitAPI/Tests/WTF/URLParser.cpp

Diff

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (267836 => 267837)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2020-10-01 17:03:03 UTC (rev 267836)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2020-10-01 17:05:41 UTC (rev 267837)
@@ -1,3 +1,15 @@
+2020-10-01  Alex Christensen  <achristen...@webkit.org>
+
+        Non-special URLs are not idempotent
+        https://bugs.webkit.org/show_bug.cgi?id=215762
+
+        Reviewed by Tim Horton.
+
+        * web-platform-tests/url/a-element-expected.txt:
+        * web-platform-tests/url/a-element-xhtml-expected.txt:
+        * web-platform-tests/url/url-constructor-expected.txt:
+        * web-platform-tests/url/url-setters-expected.txt:
+
 2020-10-01  Youenn Fablet  <you...@apple.com>
 
         MediaRecorder should support MediaRecorderOptions.mimeType

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/url/a-element-expected.txt (267836 => 267837)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/url/a-element-expected.txt	2020-10-01 17:03:03 UTC (rev 267836)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/url/a-element-expected.txt	2020-10-01 17:05:41 UTC (rev 267837)
@@ -511,18 +511,18 @@
 PASS Parsing: <git+https://github.com/foo/bar> against <about:blank>
 PASS Parsing: <urn:ietf:rfc:2648> against <about:blank>
 PASS Parsing: <tag:j...@example.org,2001:foo/bar> against <about:blank>
-FAIL Parsing: <non-spec:/.//> against <about:blank> assert_equals: href expected "non-spec:/.//" but got "non-spec://"
-FAIL Parsing: <non-spec:/..//> against <about:blank> assert_equals: href expected "non-spec:/.//" but got "non-spec://"
-FAIL Parsing: <non-spec:/a/..//> against <about:blank> assert_equals: href expected "non-spec:/.//" but got "non-spec://"
-FAIL Parsing: <non-spec:/.//path> against <about:blank> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: <non-spec:/..//path> against <about:blank> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: <non-spec:/a/..//path> against <about:blank> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: </.//path> against <non-spec:/p> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: </..//path> against <non-spec:/p> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: <..//path> against <non-spec:/p> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: <a/..//path> against <non-spec:/p> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: <> against <non-spec:/..//p> assert_equals: href expected "non-spec:/.//p" but got "non-spec://p"
-FAIL Parsing: <path> against <non-spec:/..//p> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
+PASS Parsing: <non-spec:/.//> against <about:blank>
+PASS Parsing: <non-spec:/..//> against <about:blank>
+PASS Parsing: <non-spec:/a/..//> against <about:blank>
+PASS Parsing: <non-spec:/.//path> against <about:blank>
+PASS Parsing: <non-spec:/..//path> against <about:blank>
+PASS Parsing: <non-spec:/a/..//path> against <about:blank>
+PASS Parsing: </.//path> against <non-spec:/p>
+PASS Parsing: </..//path> against <non-spec:/p>
+PASS Parsing: <..//path> against <non-spec:/p>
+PASS Parsing: <a/..//path> against <non-spec:/p>
+PASS Parsing: <> against <non-spec:/..//p>
+PASS Parsing: <path> against <non-spec:/..//p>
 PASS Parsing: <../path> against <non-spec:/.//p>
 PASS Parsing: <non-special://%E2%80%A0/> against <about:blank>
 PASS Parsing: <non-special://H%4fSt/path> against <about:blank>

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/url/a-element-xhtml-expected.txt (267836 => 267837)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/url/a-element-xhtml-expected.txt	2020-10-01 17:03:03 UTC (rev 267836)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/url/a-element-xhtml-expected.txt	2020-10-01 17:05:41 UTC (rev 267837)
@@ -511,18 +511,18 @@
 PASS Parsing: <git+https://github.com/foo/bar> against <about:blank>
 PASS Parsing: <urn:ietf:rfc:2648> against <about:blank>
 PASS Parsing: <tag:j...@example.org,2001:foo/bar> against <about:blank>
-FAIL Parsing: <non-spec:/.//> against <about:blank> assert_equals: href expected "non-spec:/.//" but got "non-spec://"
-FAIL Parsing: <non-spec:/..//> against <about:blank> assert_equals: href expected "non-spec:/.//" but got "non-spec://"
-FAIL Parsing: <non-spec:/a/..//> against <about:blank> assert_equals: href expected "non-spec:/.//" but got "non-spec://"
-FAIL Parsing: <non-spec:/.//path> against <about:blank> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: <non-spec:/..//path> against <about:blank> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: <non-spec:/a/..//path> against <about:blank> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: </.//path> against <non-spec:/p> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: </..//path> against <non-spec:/p> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: <..//path> against <non-spec:/p> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: <a/..//path> against <non-spec:/p> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: <> against <non-spec:/..//p> assert_equals: href expected "non-spec:/.//p" but got "non-spec://p"
-FAIL Parsing: <path> against <non-spec:/..//p> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
+PASS Parsing: <non-spec:/.//> against <about:blank>
+PASS Parsing: <non-spec:/..//> against <about:blank>
+PASS Parsing: <non-spec:/a/..//> against <about:blank>
+PASS Parsing: <non-spec:/.//path> against <about:blank>
+PASS Parsing: <non-spec:/..//path> against <about:blank>
+PASS Parsing: <non-spec:/a/..//path> against <about:blank>
+PASS Parsing: </.//path> against <non-spec:/p>
+PASS Parsing: </..//path> against <non-spec:/p>
+PASS Parsing: <..//path> against <non-spec:/p>
+PASS Parsing: <a/..//path> against <non-spec:/p>
+PASS Parsing: <> against <non-spec:/..//p>
+PASS Parsing: <path> against <non-spec:/..//p>
 PASS Parsing: <../path> against <non-spec:/.//p>
 PASS Parsing: <non-special://%E2%80%A0/> against <about:blank>
 PASS Parsing: <non-special://H%4fSt/path> against <about:blank>

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/url/url-constructor-expected.txt (267836 => 267837)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/url/url-constructor-expected.txt	2020-10-01 17:03:03 UTC (rev 267836)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/url/url-constructor-expected.txt	2020-10-01 17:05:41 UTC (rev 267837)
@@ -514,18 +514,18 @@
 PASS Parsing: <git+https://github.com/foo/bar> against <about:blank>
 PASS Parsing: <urn:ietf:rfc:2648> against <about:blank>
 PASS Parsing: <tag:j...@example.org,2001:foo/bar> against <about:blank>
-FAIL Parsing: <non-spec:/.//> against <about:blank> assert_equals: href expected "non-spec:/.//" but got "non-spec://"
-FAIL Parsing: <non-spec:/..//> against <about:blank> assert_equals: href expected "non-spec:/.//" but got "non-spec://"
-FAIL Parsing: <non-spec:/a/..//> against <about:blank> assert_equals: href expected "non-spec:/.//" but got "non-spec://"
-FAIL Parsing: <non-spec:/.//path> against <about:blank> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: <non-spec:/..//path> against <about:blank> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: <non-spec:/a/..//path> against <about:blank> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: </.//path> against <non-spec:/p> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: </..//path> against <non-spec:/p> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: <..//path> against <non-spec:/p> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: <a/..//path> against <non-spec:/p> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
-FAIL Parsing: <> against <non-spec:/..//p> assert_equals: href expected "non-spec:/.//p" but got "non-spec://p"
-FAIL Parsing: <path> against <non-spec:/..//p> assert_equals: href expected "non-spec:/.//path" but got "non-spec://path"
+PASS Parsing: <non-spec:/.//> against <about:blank>
+PASS Parsing: <non-spec:/..//> against <about:blank>
+PASS Parsing: <non-spec:/a/..//> against <about:blank>
+PASS Parsing: <non-spec:/.//path> against <about:blank>
+PASS Parsing: <non-spec:/..//path> against <about:blank>
+PASS Parsing: <non-spec:/a/..//path> against <about:blank>
+PASS Parsing: </.//path> against <non-spec:/p>
+PASS Parsing: </..//path> against <non-spec:/p>
+PASS Parsing: <..//path> against <non-spec:/p>
+PASS Parsing: <a/..//path> against <non-spec:/p>
+PASS Parsing: <> against <non-spec:/..//p>
+PASS Parsing: <path> against <non-spec:/..//p>
 PASS Parsing: <../path> against <non-spec:/.//p>
 PASS Parsing: <non-special://%E2%80%A0/> against <about:blank>
 PASS Parsing: <non-special://H%4fSt/path> against <about:blank>

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/url/url-setters-expected.txt (267836 => 267837)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/url/url-setters-expected.txt	2020-10-01 17:03:03 UTC (rev 267836)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/url/url-setters-expected.txt	2020-10-01 17:05:41 UTC (rev 267837)
@@ -429,9 +429,9 @@
 PASS URL: Setting <non-spec:/.//p>.hostname = 'h' Drop /. from path
 PASS <a>: Setting <non-spec:/.//p>.hostname = 'h' Drop /. from path
 PASS <area>: Setting <non-spec:/.//p>.hostname = 'h' Drop /. from path
-FAIL URL: Setting <non-spec:/.//p>.hostname = '' assert_equals: expected "non-spec:////p" but got "non-spec://p"
-FAIL <a>: Setting <non-spec:/.//p>.hostname = '' assert_equals: expected "non-spec:////p" but got "non-spec://p"
-FAIL <area>: Setting <non-spec:/.//p>.hostname = '' assert_equals: expected "non-spec:////p" but got "non-spec://p"
+FAIL URL: Setting <non-spec:/.//p>.hostname = '' assert_equals: expected "non-spec:////p" but got "non-spec:/.//p"
+FAIL <a>: Setting <non-spec:/.//p>.hostname = '' assert_equals: expected "non-spec:////p" but got "non-spec:/.//p"
+FAIL <area>: Setting <non-spec:/.//p>.hostname = '' assert_equals: expected "non-spec:////p" but got "non-spec:/.//p"
 PASS URL: Setting <http://example.net>.port = '8080'
 PASS <a>: Setting <http://example.net>.port = '8080'
 PASS <area>: Setting <http://example.net>.port = '8080'
@@ -543,12 +543,12 @@
 FAIL URL: Setting <file:///unicorn>.pathname = '//monkey/..//' File URLs and (back)slashes assert_equals: expected "file:///" but got "file://///"
 FAIL <a>: Setting <file:///unicorn>.pathname = '//monkey/..//' File URLs and (back)slashes assert_equals: expected "file:///" but got "file://///"
 FAIL <area>: Setting <file:///unicorn>.pathname = '//monkey/..//' File URLs and (back)slashes assert_equals: expected "file:///" but got "file://///"
-FAIL URL: Setting <non-spec:/>.pathname = '/.//p' Serialize /. in path assert_equals: expected "non-spec:/.//p" but got "non-spec://p"
-FAIL <a>: Setting <non-spec:/>.pathname = '/.//p' Serialize /. in path assert_equals: expected "non-spec:/.//p" but got "non-spec://p"
-FAIL <area>: Setting <non-spec:/>.pathname = '/.//p' Serialize /. in path assert_equals: expected "non-spec:/.//p" but got "non-spec://p"
-FAIL URL: Setting <non-spec:/>.pathname = '/..//p' assert_equals: expected "non-spec:/.//p" but got "non-spec://p"
-FAIL <a>: Setting <non-spec:/>.pathname = '/..//p' assert_equals: expected "non-spec:/.//p" but got "non-spec://p"
-FAIL <area>: Setting <non-spec:/>.pathname = '/..//p' assert_equals: expected "non-spec:/.//p" but got "non-spec://p"
+PASS URL: Setting <non-spec:/>.pathname = '/.//p' Serialize /. in path
+PASS <a>: Setting <non-spec:/>.pathname = '/.//p' Serialize /. in path
+PASS <area>: Setting <non-spec:/>.pathname = '/.//p' Serialize /. in path
+PASS URL: Setting <non-spec:/>.pathname = '/..//p'
+PASS <a>: Setting <non-spec:/>.pathname = '/..//p'
+PASS <area>: Setting <non-spec:/>.pathname = '/..//p'
 FAIL URL: Setting <non-spec:/>.pathname = '//p' assert_equals: expected "non-spec:/.//p" but got "non-spec://p"
 FAIL <a>: Setting <non-spec:/>.pathname = '//p' assert_equals: expected "non-spec:/.//p" but got "non-spec://p"
 FAIL <area>: Setting <non-spec:/>.pathname = '//p' assert_equals: expected "non-spec:/.//p" but got "non-spec://p"

Modified: trunk/Source/WTF/ChangeLog (267836 => 267837)


--- trunk/Source/WTF/ChangeLog	2020-10-01 17:03:03 UTC (rev 267836)
+++ trunk/Source/WTF/ChangeLog	2020-10-01 17:05:41 UTC (rev 267837)
@@ -1,3 +1,35 @@
+2020-10-01  Alex Christensen  <achristen...@webkit.org>
+
+        Non-special URLs are not idempotent
+        https://bugs.webkit.org/show_bug.cgi?id=215762
+
+        Reviewed by Tim Horton.
+
+        https://github.com/whatwg/url/pull/505 added an interesting edge case to the URL serialization:
+        "If url’s host is null, url’s path’s size is greater than 1, and url’s path[0] is the empty string, then append U+002F (/) followed by U+002E (.) to output."
+        The problem was that URLs like "a:/a/..//a" would be parsed into "a://a" with a pathname of "//a" and an empty host.  If "a://a" was then reparsed, it would again have an href of "a://a"
+        but its host would be "a" and it would have an empty path.  There is consensus that URL parsing should be idempotent, so we need to do something different here.
+        According to https://github.com/whatwg/url/issues/415#issuecomment-419197290 this follows what Edge did (and then subsequently abandoned when they switched to Chromium)
+        to make URL parsing idempotent by adding "/." before the path in the edge case of a URL with a non-special scheme (not http, https, wss, etc.) and a null host and a non-empty path that
+        has an empty first segment.  All the members of the URL remain unchanged except the full serialization (href).  This is not important in practice, but important in theory.
+
+        Our URL parser tries very hard to use the exact same WTF::String object given as input if it can.  However, this step is better implemented as a post-processing step that will almost never happen
+        because otherwise we would have to parse the entire path twice to find out if we need to add "./" or if the "./" that may have already been there needs to stay.  This is illustrated with the test URL
+        "t:/.//p/../../../..//x" which does need the "./".
+
+        In the common case, this adds one well-predicted branch to URL parsing, so I expect performance to be unaffected.  Since this is such a rare edge case of URLs, I expect no compatibility problems.
+
+        * wtf/URL.cpp:
+        (WTF::URL::pathStart const):
+        * wtf/URL.h:
+        (WTF::URL::pathStart const): Deleted.
+        * wtf/URLParser.cpp:
+        (WTF::URLParser::copyURLPartsUntil):
+        (WTF::URLParser::URLParser):
+        (WTF::URLParser::needsNonSpecialDotSlash const):
+        (WTF::URLParser::addNonSpecialDotSlash):
+        * wtf/URLParser.h:
+
 2020-09-30  Commit Queue  <commit-qu...@webkit.org>
 
         Unreviewed, reverting r267795.

Modified: trunk/Source/WTF/wtf/URL.cpp (267836 => 267837)


--- trunk/Source/WTF/wtf/URL.cpp	2020-10-01 17:03:03 UTC (rev 267836)
+++ trunk/Source/WTF/wtf/URL.cpp	2020-10-01 17:05:41 UTC (rev 267837)
@@ -93,6 +93,16 @@
     return StringView(m_string).substring(start, end - start + 1);
 }
 
+unsigned URL::pathStart() const
+{
+    unsigned start = m_hostEnd + m_portLength;
+    if (start == m_schemeEnd + 1
+        && start + 1 < m_string.length()
+        && m_string[start] == '/' && m_string[start + 1] == '.')
+        start += 2;
+    return start;
+}
+
 StringView URL::protocol() const
 {
     if (!m_isValid)

Modified: trunk/Source/WTF/wtf/URL.h (267836 => 267837)


--- trunk/Source/WTF/wtf/URL.h	2020-10-01 17:03:03 UTC (rev 267836)
+++ trunk/Source/WTF/wtf/URL.h	2020-10-01 17:05:41 UTC (rev 267837)
@@ -167,7 +167,7 @@
 
     WTF_EXPORT_PRIVATE static bool hostIsIPAddress(StringView);
 
-    unsigned pathStart() const;
+    WTF_EXPORT_PRIVATE unsigned pathStart() const;
     unsigned pathEnd() const;
     unsigned pathAfterLastSlash() const;
 
@@ -370,11 +370,6 @@
     return m_protocolIsInHTTPFamily;
 }
 
-inline unsigned URL::pathStart() const
-{
-    return m_hostEnd + m_portLength;
-}
-
 inline unsigned URL::pathEnd() const
 {
     return m_pathEnd;

Modified: trunk/Source/WTF/wtf/URLParser.cpp (267836 => 267837)


--- trunk/Source/WTF/wtf/URLParser.cpp	2020-10-01 17:03:03 UTC (rev 267836)
+++ trunk/Source/WTF/wtf/URLParser.cpp	2020-10-01 17:05:41 UTC (rev 267837)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2020 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -807,6 +807,7 @@
         m_url.m_protocolIsInHTTPFamily = base.m_protocolIsInHTTPFamily;
         m_url.m_schemeEnd = base.m_schemeEnd;
     }
+
     switch (scheme(StringView(m_asciiBuffer.data(), m_url.m_schemeEnd))) {
     case Scheme::WS:
     case Scheme::WSS:
@@ -824,6 +825,16 @@
     case Scheme::NonSpecial:
         m_urlIsSpecial = false;
         nonUTF8QueryEncoding = nullptr;
+        auto pathStart = m_url.m_hostEnd + m_url.m_portLength;
+        if (pathStart + 2 < m_asciiBuffer.size()
+            && m_asciiBuffer[pathStart] == '/'
+            && m_asciiBuffer[pathStart + 1] == '.'
+            && m_asciiBuffer[pathStart + 2] == '/') {
+            m_asciiBuffer.remove(pathStart + 1, 2);
+            m_url.m_pathAfterLastSlash = std::max(2u, m_url.m_pathAfterLastSlash) - 2;
+            m_url.m_pathEnd = std::max(2u, m_url.m_pathEnd) - 2;
+            m_url.m_queryEnd = std::max(2u, m_url.m_queryEnd) - 2;
+        }
         return;
     }
     ASSERT_NOT_REACHED();
@@ -1075,6 +1086,9 @@
             ASSERT(allValuesEqual(parser.result(), m_url));
     }
 #endif // ASSERT_ENABLED
+
+    if (UNLIKELY(needsNonSpecialDotSlash()))
+        addNonSpecialDotSlash();
 }
 
 template<typename CharacterType>
@@ -2434,6 +2448,26 @@
     return output;
 }
 
+bool URLParser::needsNonSpecialDotSlash() const
+{
+    auto pathStart = m_url.m_hostEnd + m_url.m_portLength;
+    return !m_urlIsSpecial
+        && pathStart == m_url.m_schemeEnd + 1
+        && pathStart + 1 < m_url.m_string.length()
+        && m_url.m_string[pathStart] == '/'
+        && m_url.m_string[pathStart + 1] == '/';
+}
+
+void URLParser::addNonSpecialDotSlash()
+{
+    auto oldPathStart = m_url.m_hostEnd + m_url.m_portLength;
+    auto& oldString = m_url.m_string;
+    m_url.m_string = makeString(oldString.substring(0, oldPathStart + 1), "./", oldString.substring(oldPathStart + 1));
+    m_url.m_pathAfterLastSlash += 2;
+    m_url.m_pathEnd += 2;
+    m_url.m_queryEnd += 2;
+}
+
 template<typename CharacterType> Optional<URLParser::LCharBuffer> URLParser::domainToASCII(StringImpl& domain, const CodePointIterator<CharacterType>& iteratorForSyntaxViolationPosition)
 {
     LCharBuffer ascii;

Modified: trunk/Source/WTF/wtf/URLParser.h (267836 => 267837)


--- trunk/Source/WTF/wtf/URLParser.h	2020-10-01 17:03:03 UTC (rev 267836)
+++ trunk/Source/WTF/wtf/URLParser.h	2020-10-01 17:05:41 UTC (rev 267837)
@@ -114,6 +114,9 @@
     StringView parsedDataView(size_t start, size_t length);
     UChar parsedDataView(size_t position);
 
+    bool needsNonSpecialDotSlash() const;
+    void addNonSpecialDotSlash();
+
     using IPv4Address = uint32_t;
     void serializeIPv4(IPv4Address);
     enum class IPv4ParsingError;

Modified: trunk/Tools/ChangeLog (267836 => 267837)


--- trunk/Tools/ChangeLog	2020-10-01 17:03:03 UTC (rev 267836)
+++ trunk/Tools/ChangeLog	2020-10-01 17:05:41 UTC (rev 267837)
@@ -1,3 +1,13 @@
+2020-10-01  Alex Christensen  <achristen...@webkit.org>
+
+        Non-special URLs are not idempotent
+        https://bugs.webkit.org/show_bug.cgi?id=215762
+
+        Reviewed by Tim Horton.
+
+        * TestWebKitAPI/Tests/WTF/URLParser.cpp:
+        (TestWebKitAPI::TEST_F):
+
 2020-10-01  Carlos Garcia Campos  <cgar...@igalia.com>
 
         Unreviewed. Update W3C WebDriver imported tests.

Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/URLParser.cpp (267836 => 267837)


--- trunk/Tools/TestWebKitAPI/Tests/WTF/URLParser.cpp	2020-10-01 17:03:03 UTC (rev 267836)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/URLParser.cpp	2020-10-01 17:05:41 UTC (rev 267837)
@@ -209,6 +209,39 @@
     checkRelativeURL(urlString, baseString, {"", "", "", "", 0, "", "", "", urlString});
 }
 
+TEST_F(WTF_URLParser, Idempotence)
+{
+    checkURL("a://", {"a", "", "", "", 0, "", "", "", "a://"});
+    checkURL("b:///", {"b", "", "", "", 0, "/", "", "", "b:///"});
+    checkURL("c:/.//", {"c", "", "", "", 0, "//", "", "", "c:/.//"});
+    checkURL("d:/..//", {"d", "", "", "", 0, "//", "", "", "d:/.//"});
+    checkURL("e:/../..//", {"e", "", "", "", 0, "//", "", "", "e:/.//"});
+    checkURL("f:/../../", {"f", "", "", "", 0, "/", "", "", "f:/"});
+    checkURL("g:/././", {"g", "", "", "", 0, "/", "", "", "g:/"});
+    checkURL("h:/./../", {"h", "", "", "", 0, "/", "", "", "h:/"});
+    checkURL("i:/.././", {"i", "", "", "", 0, "/", "", "", "i:/"});
+    checkURL("j:/./..//", {"j", "", "", "", 0, "//", "", "", "j:/.//"});
+    checkURL("k:/.././/", {"k", "", "", "", 0, "//", "", "", "k:/.//"});
+    checkURL("l:/.?", {"l", "", "", "", 0, "/", "", "", "l:/?"});
+    checkURL("m:/./?", {"m", "", "", "", 0, "/", "", "", "m:/?"});
+    checkURL("n:/.//?", {"n", "", "", "", 0, "//", "", "", "n:/.//?"});
+    checkURL("o:/.#", {"o", "", "", "", 0, "/", "", "", "o:/#"});
+    checkURL("p:/%2e//", {"p", "", "", "", 0, "//", "", "", "p:/.//"});
+    checkURL("q:/%2e%2e//", {"q", "", "", "", 0, "//", "", "", "q:/.//"});
+    checkURL("r:/%2e%2e/", {"r", "", "", "", 0, "/", "", "", "r:/"});
+    checkURL("s:/%2e/", {"s", "", "", "", 0, "/", "", "", "s:/"});
+    checkURL("t:/.//p/../../../..//x", {"t", "", "", "", 0, "//x", "", "", "t:/.//x"});
+    checkRelativeURL("../path", "u:/.//p", {"u", "", "", "", 0, "/path", "", "", "u:/path"});
+    checkURL("v:/.//..", {"v", "", "", "", 0, "/", "", "", "v:/"});
+    checkURL("w:/.//..//", {"w", "", "", "", 0, "//", "", "", "w:/.//"});
+    checkURL("x:/.//../a", {"x", "", "", "", 0, "/a", "", "", "x:/a"});
+    checkURL("http://host/./", {"http", "", "", "host", 0, "/", "", "", "http://host/"});
+    checkURL("http://host/../", {"http", "", "", "host", 0, "/", "", "", "http://host/"});
+    checkURL("http://host/.../", {"http", "", "", "host", 0, "/.../", "", "", "http://host/.../"});
+    checkURL("http://host/..", {"http", "", "", "host", 0, "/", "", "", "http://host/"});
+    checkURL("http://host/.", {"http", "", "", "host", 0, "/", "", "", "http://host/"});
+}
+
 TEST_F(WTF_URLParser, Basic)
 {
     checkURL("http://user:p...@webkit.org:123/path?query#fragment", {"http", "user", "pass", "webkit.org", 123, "/path", "query", "fragment", "http://user:p...@webkit.org:123/path?query#fragment"});

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes