[webkit-changes] [205312] trunk

Thu, 01 Sep 2016 13:33:32 -0700

Title: [205312] trunk
Revision
205312
Author
achristen...@apple.com
Date
2016-09-01 13:33:31 -0700 (Thu, 01 Sep 2016)

Log Message

URLParser should handle . and .. in URL paths
https://bugs.webkit.org/show_bug.cgi?id=161443

Reviewed by Brady Eidson.

Source/WebCore:

Covered by new API tests.

* platform/URLParser.cpp:
(WebCore::isSingleDotPathSegment):
(WebCore::isDoubleDotPathSegment):
(WebCore::consumeSingleDotPathSegment):
(WebCore::consumeDoubleDotPathSegment):
(WebCore::URLParser::parse):
(WebCore::URLParser::copyURLPartsUntil): Deleted.

Tools:

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

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (205311 => 205312)


--- trunk/Source/WebCore/ChangeLog	2016-09-01 20:17:02 UTC (rev 205311)
+++ trunk/Source/WebCore/ChangeLog	2016-09-01 20:33:31 UTC (rev 205312)
@@ -1,5 +1,22 @@
 2016-09-01  Alex Christensen  <achristen...@webkit.org>
 
+        URLParser should handle . and .. in URL paths
+        https://bugs.webkit.org/show_bug.cgi?id=161443
+
+        Reviewed by Brady Eidson.
+
+        Covered by new API tests.
+
+        * platform/URLParser.cpp:
+        (WebCore::isSingleDotPathSegment):
+        (WebCore::isDoubleDotPathSegment):
+        (WebCore::consumeSingleDotPathSegment):
+        (WebCore::consumeDoubleDotPathSegment):
+        (WebCore::URLParser::parse):
+        (WebCore::URLParser::copyURLPartsUntil): Deleted.
+
+2016-09-01  Alex Christensen  <achristen...@webkit.org>
+
         Fix Mac CMake build.
 
         * PlatformMac.cmake:

Modified: trunk/Source/WebCore/platform/URLParser.cpp (205311 => 205312)


--- trunk/Source/WebCore/platform/URLParser.cpp	2016-09-01 20:17:02 UTC (rev 205311)
+++ trunk/Source/WebCore/platform/URLParser.cpp	2016-09-01 20:33:31 UTC (rev 205312)
@@ -127,7 +127,111 @@
         m_url.m_schemeEnd = base.m_schemeEnd;
     }
 }
+
+static const char* dotASCIICode = "2e";
+
+static bool isSingleDotPathSegment(StringView::CodePoints::Iterator c, const StringView::CodePoints::Iterator& end)
+{
+    if (c == end)
+        return false;
+    if (*c == '.') {
+        ++c;
+        return c == end || *c == '/' || *c == '\\' || *c == '?' || *c == '#';
+    }
+    if (*c != '%')
+        return false;
+    ++c;
+    if (c == end || *c != dotASCIICode[0])
+        return false;
+    ++c;
+    if (c == end)
+        return false;
+    if (toASCIILower(*c) == dotASCIICode[1]) {
+        ++c;
+        return c == end || *c == '/' || *c == '\\' || *c == '?' || *c == '#';
+    }
+    return false;
+}
     
+static bool isDoubleDotPathSegment(StringView::CodePoints::Iterator c, const StringView::CodePoints::Iterator& end)
+{
+    if (c == end)
+        return false;
+    if (*c == '.') {
+        ++c;
+        return isSingleDotPathSegment(c, end);
+    }
+    if (*c != '%')
+        return false;
+    ++c;
+    if (c == end || *c != dotASCIICode[0])
+        return false;
+    ++c;
+    if (c == end)
+        return false;
+    if (toASCIILower(*c) == dotASCIICode[1]) {
+        ++c;
+        return isSingleDotPathSegment(c, end);
+    }
+    return false;
+}
+
+static void consumeSingleDotPathSegment(StringView::CodePoints::Iterator& c, const StringView::CodePoints::Iterator end)
+{
+    ASSERT(isSingleDotPathSegment(c, end));
+    if (*c == '.') {
+        ++c;
+        if (c != end) {
+            if (*c == '/' || *c == '\\')
+                ++c;
+            else
+                ASSERT(*c == '?' || *c == '#');
+        }
+    } else {
+        ASSERT(*c == '%');
+        ++c;
+        ASSERT(*c == dotASCIICode[0]);
+        ++c;
+        ASSERT(toASCIILower(*c) == dotASCIICode[1]);
+        ++c;
+        if (c != end) {
+            if (*c == '/' || *c == '\\')
+                ++c;
+            else
+                ASSERT(*c == '?' || *c == '#');
+        }
+    }
+}
+
+static void consumeDoubleDotPathSegment(StringView::CodePoints::Iterator& c, const StringView::CodePoints::Iterator end)
+{
+    ASSERT(isDoubleDotPathSegment(c, end));
+    if (*c == '.')
+        ++c;
+    else {
+        ASSERT(*c == '%');
+        ++c;
+        ASSERT(*c == dotASCIICode[0]);
+        ++c;
+        ASSERT(toASCIILower(*c) == dotASCIICode[1]);
+        ++c;
+    }
+    consumeSingleDotPathSegment(c, end);
+}
+
+void URLParser::popPath()
+{
+    if (m_url.m_pathAfterLastSlash > m_url.m_portEnd + 1) {
+        m_url.m_pathAfterLastSlash--;
+        if (m_buffer[m_url.m_pathAfterLastSlash] == '/')
+            m_url.m_pathAfterLastSlash--;
+        while (m_url.m_pathAfterLastSlash > m_url.m_portEnd && m_buffer[m_url.m_pathAfterLastSlash] != '/')
+            m_url.m_pathAfterLastSlash--;
+        m_url.m_pathAfterLastSlash++;
+    }
+    m_buffer.resize(m_url.m_pathAfterLastSlash);
+}
+
 URL URLParser::parse(const String& input, const URL& base, const TextEncoding&)
 {
     LOG(URLParser, "Parsing URL <%s> base <%s>", input.utf8().data(), base.string().utf8().data());
@@ -385,26 +489,27 @@
                 m_buffer.append('/');
                 m_url.m_pathAfterLastSlash = m_buffer.length();
                 ++c;
-                while (c != end && isTabOrNewline(*c))
-                    ++c;
-                if (c == end)
+                break;
+            }
+            if (m_buffer.length() && m_buffer[m_buffer.length() - 1] == '/') {
+                if (isDoubleDotPathSegment(c, end)) {
+                    consumeDoubleDotPathSegment(c, end);
+                    popPath();
                     break;
-                if (*c == '.') {
-                    ++c;
-                    while (c != end && isTabOrNewline(*c))
-                        ++c;
-                    if (c == end)
-                        return { };
-                    if (*c == '.')
-                        notImplemented();
-                    notImplemented();
                 }
-            } else if (*c == '?') {
+                if (m_buffer[m_buffer.length() - 1] == '/' && isSingleDotPathSegment(c, end)) {
+                    consumeSingleDotPathSegment(c, end);
+                    break;
+                }
+            }
+            if (*c == '?') {
                 m_url.m_pathEnd = m_buffer.length();
                 state = State::Query;
                 break;
-            } else if (*c == '#') {
+            }
+            if (*c == '#') {
                 m_url.m_pathEnd = m_buffer.length();
+                m_url.m_queryEnd = m_url.m_pathEnd;
                 state = State::Fragment;
                 break;
             }

Modified: trunk/Source/WebCore/platform/URLParser.h (205311 => 205312)


--- trunk/Source/WebCore/platform/URLParser.h	2016-09-01 20:17:02 UTC (rev 205311)
+++ trunk/Source/WebCore/platform/URLParser.h	2016-09-01 20:33:31 UTC (rev 205312)
@@ -49,6 +49,7 @@
     enum class URLPart;
     void copyURLPartsUntil(const URL& base, URLPart);
     static size_t urlLengthUntilPart(const URL&, URLPart);
+    void popPath();
 };
 
 }

Modified: trunk/Tools/ChangeLog (205311 => 205312)


--- trunk/Tools/ChangeLog	2016-09-01 20:17:02 UTC (rev 205311)
+++ trunk/Tools/ChangeLog	2016-09-01 20:33:31 UTC (rev 205312)
@@ -1,3 +1,14 @@
+2016-09-01  Alex Christensen  <achristen...@webkit.org>
+
+        URLParser should handle . and .. in URL paths
+        https://bugs.webkit.org/show_bug.cgi?id=161443
+
+        Reviewed by Brady Eidson.
+
+        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+        (TestWebKitAPI::eq):
+        (TestWebKitAPI::TEST_F):
+
 2016-09-01  Commit Queue  <commit-qu...@webkit.org>
 
         Unreviewed, rolling out r205295 and r205303.

Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp (205311 => 205312)


--- trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp	2016-09-01 20:17:02 UTC (rev 205311)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp	2016-09-01 20:33:31 UTC (rev 205312)
@@ -102,6 +102,43 @@
     checkURL("http://[0:f::f:f:0:0]", {"http", "", "", "[0:f::f:f:0:0]", 0, "/", "", "", "http://[0:f::f:f:0:0]/"});
     checkURL("http://[0:f:0:0:f::]", {"http", "", "", "[0:f:0:0:f::]", 0, "/", "", "", "http://[0:f:0:0:f::]/"});
     checkURL("http://[::f:0:0:f:0:0]", {"http", "", "", "[::f:0:0:f:0:0]", 0, "/", "", "", "http://[::f:0:0:f:0:0]/"});
+    checkURL("http://example.com/path1/path2/.", {"http", "", "", "example.com", 0, "/path1/path2/", "", "", "http://example.com/path1/path2/"});
+    checkURL("http://example.com/path1/path2/..", {"http", "", "", "example.com", 0, "/path1/", "", "", "http://example.com/path1/"});
+    checkURL("http://example.com/path1/path2/./path3", {"http", "", "", "example.com", 0, "/path1/path2/path3", "", "", "http://example.com/path1/path2/path3"});
+    checkURL("http://example.com/path1/path2/.\\path3", {"http", "", "", "example.com", 0, "/path1/path2/path3", "", "", "http://example.com/path1/path2/path3"});
+    checkURL("http://example.com/path1/path2/../path3", {"http", "", "", "example.com", 0, "/path1/path3", "", "", "http://example.com/path1/path3"});
+    checkURL("http://example.com/path1/path2/..\\path3", {"http", "", "", "example.com", 0, "/path1/path3", "", "", "http://example.com/path1/path3"});
+    checkURL("http://example.com/.", {"http", "", "", "example.com", 0, "/", "", "", "http://example.com/"});
+    checkURL("http://example.com/..", {"http", "", "", "example.com", 0, "/", "", "", "http://example.com/"});
+    checkURL("http://example.com/./path1", {"http", "", "", "example.com", 0, "/path1", "", "", "http://example.com/path1"});
+    checkURL("http://example.com/../path1", {"http", "", "", "example.com", 0, "/path1", "", "", "http://example.com/path1"});
+    checkURL("http://example.com/../path1/../../path2/path3/../path4", {"http", "", "", "example.com", 0, "/path2/path4", "", "", "http://example.com/path2/path4"});
+    checkURL("http://example.com/path1/.%2", {"http", "", "", "example.com", 0, "/path1/.%2", "", "", "http://example.com/path1/.%2"});
+    checkURL("http://example.com/path1/%2", {"http", "", "", "example.com", 0, "/path1/%2", "", "", "http://example.com/path1/%2"});
+    checkURL("http://example.com/path1/%", {"http", "", "", "example.com", 0, "/path1/%", "", "", "http://example.com/path1/%"});
+    checkURL("http://example.com/path1/.%", {"http", "", "", "example.com", 0, "/path1/.%", "", "", "http://example.com/path1/.%"});
+    checkURL("http://example.com//.", {"http", "", "", "example.com", 0, "//", "", "", "http://example.com//"});
+    checkURL("http://example.com//./", {"http", "", "", "example.com", 0, "//", "", "", "http://example.com//"});
+    checkURL("http://example.com//.//", {"http", "", "", "example.com", 0, "///", "", "", "http://example.com///"});
+    checkURL("http://example.com//..", {"http", "", "", "example.com", 0, "/", "", "", "http://example.com/"});
+    checkURL("http://example.com//../", {"http", "", "", "example.com", 0, "/", "", "", "http://example.com/"});
+    checkURL("http://example.com//..//", {"http", "", "", "example.com", 0, "//", "", "", "http://example.com//"});
+    checkURL("http://example.com//..", {"http", "", "", "example.com", 0, "/", "", "", "http://example.com/"});
+    checkURL("http://example.com/.//", {"http", "", "", "example.com", 0, "//", "", "", "http://example.com//"});
+    checkURL("http://example.com/..//", {"http", "", "", "example.com", 0, "//", "", "", "http://example.com//"});
+    checkURL("http://example.com/./", {"http", "", "", "example.com", 0, "/", "", "", "http://example.com/"});
+    checkURL("http://example.com/../", {"http", "", "", "example.com", 0, "/", "", "", "http://example.com/"});
+    checkURL("http://example.com/path1/.../path3", {"http", "", "", "example.com", 0, "/path1/.../path3", "", "", "http://example.com/path1/.../path3"});
+    checkURL("http://example.com/path1/...", {"http", "", "", "example.com", 0, "/path1/...", "", "", "http://example.com/path1/..."});
+    checkURL("http://example.com/path1/.../", {"http", "", "", "example.com", 0, "/path1/.../", "", "", "http://example.com/path1/.../"});
+    checkURL("http://example.com/.path1/", {"http", "", "", "example.com", 0, "/.path1/", "", "", "http://example.com/.path1/"});
+    checkURL("http://example.com/..path1/", {"http", "", "", "example.com", 0, "/..path1/", "", "", "http://example.com/..path1/"});
+    checkURL("http://example.com/path1/.path2", {"http", "", "", "example.com", 0, "/path1/.path2", "", "", "http://example.com/path1/.path2"});
+    checkURL("http://example.com/path1/..path2", {"http", "", "", "example.com", 0, "/path1/..path2", "", "", "http://example.com/path1/..path2"});
+    checkURL("http://example.com/path1/path2/.?query", {"http", "", "", "example.com", 0, "/path1/path2/", "query", "", "http://example.com/path1/path2/?query"});
+    checkURL("http://example.com/path1/path2/..?query", {"http", "", "", "example.com", 0, "/path1/", "query", "", "http://example.com/path1/?query"});
+    checkURL("http://example.com/path1/path2/.#fragment", {"http", "", "", "example.com", 0, "/path1/path2/", "", "fragment", "http://example.com/path1/path2/#fragment"});
+    checkURL("http://example.com/path1/path2/..#fragment", {"http", "", "", "example.com", 0, "/path1/", "", "fragment", "http://example.com/path1/#fragment"});
 }
 
 static void checkRelativeURL(const String& urlString, const String& baseURLString, const ExpectedParts& parts)
@@ -202,6 +239,7 @@
     EXPECT_FALSE(URLParser::allValuesEqual(url, oldURL));
 }
 
+// These are differences between the new URLParser and the old URL::parse which make URLParser more standards compliant.
 TEST_F(URLParserTest, ParserDifferences)
 {
     checkURLDifferences("http://127.0.1",
@@ -222,6 +260,42 @@
     checkURLDifferences("http://[0:0:f:0:0:f:0:0]",
         {"http", "", "", "[::f:0:0:f:0:0]", 0, "/", "", "", "http://[::f:0:0:f:0:0]/"},
         {"http", "", "", "[0:0:f:0:0:f:0:0]", 0, "/", "", "", "http://[0:0:f:0:0:f:0:0]/"});
+    checkURLDifferences("http://example.com/path1/.%2e",
+        {"http", "", "", "example.com", 0, "/", "", "", "http://example.com/"},
+        {"http", "", "", "example.com", 0, "/path1/.%2e", "", "", "http://example.com/path1/.%2e"});
+    checkURLDifferences("http://example.com/path1/.%2E",
+        {"http", "", "", "example.com", 0, "/", "", "", "http://example.com/"},
+        {"http", "", "", "example.com", 0, "/path1/.%2E", "", "", "http://example.com/path1/.%2E"});
+    checkURLDifferences("http://example.com/path1/.%2E/",
+        {"http", "", "", "example.com", 0, "/", "", "", "http://example.com/"},
+        {"http", "", "", "example.com", 0, "/path1/.%2E/", "", "", "http://example.com/path1/.%2E/"});
+    checkURLDifferences("http://example.com/path1/%2e.",
+        {"http", "", "", "example.com", 0, "/", "", "", "http://example.com/"},
+        {"http", "", "", "example.com", 0, "/path1/%2e.", "", "", "http://example.com/path1/%2e."});
+    checkURLDifferences("http://example.com/path1/%2E%2e",
+        {"http", "", "", "example.com", 0, "/", "", "", "http://example.com/"},
+        {"http", "", "", "example.com", 0, "/path1/%2E%2e", "", "", "http://example.com/path1/%2E%2e"});
+    checkURLDifferences("http://example.com/path1/%2e",
+        {"http", "", "", "example.com", 0, "/path1/", "", "", "http://example.com/path1/"},
+        {"http", "", "", "example.com", 0, "/path1/%2e", "", "", "http://example.com/path1/%2e"});
+    checkURLDifferences("http://example.com/path1/%2E",
+        {"http", "", "", "example.com", 0, "/path1/", "", "", "http://example.com/path1/"},
+        {"http", "", "", "example.com", 0, "/path1/%2E", "", "", "http://example.com/path1/%2E"});
+    checkURLDifferences("http://example.com/path1/%2E/",
+        {"http", "", "", "example.com", 0, "/path1/", "", "", "http://example.com/path1/"},
+        {"http", "", "", "example.com", 0, "/path1/%2E/", "", "", "http://example.com/path1/%2E/"});
+    checkURLDifferences("http://example.com/path1/path2/%2e?query",
+        {"http", "", "", "example.com", 0, "/path1/path2/", "query", "", "http://example.com/path1/path2/?query"},
+        {"http", "", "", "example.com", 0, "/path1/path2/%2e", "query", "", "http://example.com/path1/path2/%2e?query"});
+    checkURLDifferences("http://example.com/path1/path2/%2e%2e?query",
+        {"http", "", "", "example.com", 0, "/path1/", "query", "", "http://example.com/path1/?query"},
+        {"http", "", "", "example.com", 0, "/path1/path2/%2e%2e", "query", "", "http://example.com/path1/path2/%2e%2e?query"});
+    checkURLDifferences("http://example.com/path1/path2/%2e#fragment",
+        {"http", "", "", "example.com", 0, "/path1/path2/", "", "fragment", "http://example.com/path1/path2/#fragment"},
+        {"http", "", "", "example.com", 0, "/path1/path2/%2e", "", "fragment", "http://example.com/path1/path2/%2e#fragment"});
+    checkURLDifferences("http://example.com/path1/path2/%2e%2e#fragment",
+        {"http", "", "", "example.com", 0, "/path1/", "", "fragment", "http://example.com/path1/#fragment"},
+        {"http", "", "", "example.com", 0, "/path1/path2/%2e%2e", "", "fragment", "http://example.com/path1/path2/%2e%2e#fragment"});
 
     // 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.

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

Reply via email to