Changeset 236674 in webkit

Timestamp:

Oct 1, 2018, 11:16:33 AM (7 years ago)

Author:

achristensen@apple.com

Message:

URL should not use TextEncoding internally
​https://bugs.webkit.org/show_bug.cgi?id=190111

Reviewed by Andy Estes.

Source/WebCore:

That dependency makes it impossible to move or use elsewhere.
Using TextEncoding was overkill because we know the credentials are UTF-8 percent-encoded in a parsed URL.
No change in behavior as verified by new API tests.

• page/SecurityOrigin.cpp:
• page/csp/ContentSecurityPolicySourceList.cpp:
• platform/URL.cpp:

(WebCore::decodeEscapeSequencesFromParsedURL):
(WebCore::URL::user const):
(WebCore::URL::pass const):
(WebCore::URL::fileSystemPath const):
(WebCore::decodeURLEscapeSequences): Deleted.

• platform/URL.h:
• platform/network/DataURLDecoder.cpp:
• platform/text/TextEncoding.cpp:

(WebCore::decodeURLEscapeSequences):

• platform/text/TextEncoding.h:

Source/WebKit:

• UIProcess/WebInspectorProxy.cpp:

Source/WebKitLegacy/mac:

• Misc/WebNSURLExtras.mm:

Tools:

• TestWebKitAPI/Tests/WebCore/URLParser.cpp:

(TestWebKitAPI::testUserPass):
(TestWebKitAPI::TEST_F):

Location:

trunk

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/SecurityOrigin.cpp (modified) (1 diff)
• Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp (modified) (1 diff)
• Source/WebCore/platform/URL.cpp (modified) (5 diffs)
• Source/WebCore/platform/URL.h (modified) (2 diffs)
• Source/WebCore/platform/network/DataURLDecoder.cpp (modified) (1 diff)
• Source/WebCore/platform/text/TextEncoding.cpp (modified) (2 diffs)
• Source/WebCore/platform/text/TextEncoding.h (modified) (1 diff)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp (modified) (1 diff)
• Source/WebKit/UIProcess/API/glib/WebKitFileChooserRequest.cpp (modified) (1 diff)
• Source/WebKit/UIProcess/WebInspectorProxy.cpp (modified) (1 diff)
• Source/WebKit/WebProcess/Plugins/PluginView.cpp (modified) (1 diff)
• Source/WebKitLegacy/mac/ChangeLog (modified) (1 diff)
• Source/WebKitLegacy/mac/Misc/WebNSURLExtras.mm (modified) (1 diff)
• Source/WebKitLegacy/win/WebDownloadCurl.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-10-01  Alex Christensen  <achristensen@webkit.org>
+
+        URL should not use TextEncoding internally
+        https://bugs.webkit.org/show_bug.cgi?id=190111
+
+        Reviewed by Andy Estes.
+
+        That dependency makes it impossible to move or use elsewhere.
+        Using TextEncoding was overkill because we know the credentials are UTF-8 percent-encoded in a parsed URL.
+        No change in behavior as verified by new API tests.
+
+        * page/SecurityOrigin.cpp:
+        * page/csp/ContentSecurityPolicySourceList.cpp:
+        * platform/URL.cpp:
+        (WebCore::decodeEscapeSequencesFromParsedURL):
+        (WebCore::URL::user const):
+        (WebCore::URL::pass const):
+        (WebCore::URL::fileSystemPath const):
+        (WebCore::decodeURLEscapeSequences): Deleted.
+        * platform/URL.h:
+        * platform/network/DataURLDecoder.cpp:
+        * platform/text/TextEncoding.cpp:
+        (WebCore::decodeURLEscapeSequences):
+        * platform/text/TextEncoding.h:
+
 2018-10-01  Simon Pieters  <zcorpan@gmail.com>
 

trunk/Source/WebCore/page/SecurityOrigin.cpp

@@ -35 +35 @@
 #include "SchemeRegistry.h"
 #include "SecurityPolicy.h"
+#include "TextEncoding.h"
 #include "ThreadableBlobRegistry.h"
 #include <wtf/MainThread.h>

trunk/Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp

@@ -31 +31 @@
 #include "ContentSecurityPolicyDirectiveNames.h"
 #include "ParsingUtilities.h"
+#include "TextEncoding.h"
 #include "URL.h"
 #include <wtf/ASCIICType.h>

trunk/Source/WebCore/platform/URL.cpp

@@ -28 +28 @@
 #include "URL.h"
 
-#include "DecodeEscapeSequences.h"
-#include "TextEncoding.h"
 #include "URLParser.h"
 #include <stdio.h>
@@ -188 +186 @@
 }
 
+static String decodeEscapeSequencesFromParsedURL(StringView input)
+{
+    auto inputLength = input.length();
+    if (!inputLength)
+        return emptyString();
+    Vector<LChar> percentDecoded;
+    percentDecoded.reserveInitialCapacity(inputLength);
+    for (unsigned i = 0; i < inputLength; ++i) {
+        if (input[i] == '%'
+            && inputLength > 2
+            && i < inputLength - 2
+            && isASCIIHexDigit(input[i + 1])
+            && isASCIIHexDigit(input[i + 2])) {
+            percentDecoded.uncheckedAppend(toASCIIHexValue(input[i + 1], input[i + 2]));
+            i += 2;
+        } else
+            percentDecoded.uncheckedAppend(input[i]);
+    }
+    return String::fromUTF8(percentDecoded.data(), percentDecoded.size());
+}
+
 String URL::user() const
 {
-    return decodeURLEscapeSequences(m_string.substring(m_userStart, m_userEnd - m_userStart));
+    return decodeEscapeSequencesFromParsedURL(StringView(m_string).substring(m_userStart, m_userEnd - m_userStart));
 }
 
@@ -198 +217 @@
         return String();
 
-    return decodeURLEscapeSequences(m_string.substring(m_userEnd + 1, m_passwordEnd - m_userEnd - 1));
+    return decodeEscapeSequencesFromParsedURL(StringView(m_string).substring(m_userEnd + 1, m_passwordEnd - m_userEnd - 1));
 }
 
@@ -239 +258 @@
         return String();
 
-    return decodeURLEscapeSequences(path());
+    return decodeEscapeSequencesFromParsedURL(StringView(path()));
 }
 
@@ -657 +676 @@
     URLParser parser(makeString(StringView(m_string).left(m_hostEnd + m_portLength), percentEncodeCharacters(path, questionMarkOrNumberSign), StringView(m_string).substring(m_pathEnd)));
     *this = parser.result();
-}
-
-String decodeURLEscapeSequences(const String& string)
-{
-    if (string.isEmpty())
-        return string;
-    return decodeEscapeSequences<URLEscapeSequence>(string, UTF8Encoding());
-}
-
-String decodeURLEscapeSequences(const String& string, const TextEncoding& encoding)
-{
-    if (string.isEmpty())
-        return string;
-    return decodeEscapeSequences<URLEscapeSequence>(string, encoding);
 }
 

trunk/Source/WebCore/platform/URL.h

@@ -129 +129 @@
     // Unlike user() and pass(), these functions don't decode escape sequences.
     // This is necessary for accurate round-tripping, because encoding doesn't encode '%' characters.
-    String encodedUser() const;
-    String encodedPass() const;
+    WEBCORE_EXPORT String encodedUser() const;
+    WEBCORE_EXPORT String encodedPass() const;
 
     WEBCORE_EXPORT String baseAsString() const;
@@ -303 +303 @@
 String mimeTypeFromDataURL(const String& url);
 
-// Unescapes the given string using URL escaping rules, given an optional
-// encoding (defaulting to UTF-8 otherwise). DANGER: If the URL has "%00"
-// in it, the resulting string will have embedded null characters!
-WEBCORE_EXPORT String decodeURLEscapeSequences(const String&);
-class TextEncoding;
-String decodeURLEscapeSequences(const String&, const TextEncoding&);
-
 // FIXME: This is a wrong concept to expose, different parts of a URL need different escaping per the URL Standard.
 WEBCORE_EXPORT String encodeWithURLEscapeSequences(const String&);

trunk/Source/WebCore/platform/network/DataURLDecoder.cpp

@@ -30 +30 @@
 #include "HTTPParsers.h"
 #include "SharedBuffer.h"
+#include "TextEncoding.h"
 #include "URL.h"
 #include <wtf/MainThread.h>

trunk/Source/WebCore/platform/text/TextEncoding.cpp

@@ -29 +29 @@
 #include "TextEncoding.h"
 
+#include "DecodeEscapeSequences.h"
 #include "TextCodec.h"
 #include "TextEncodingRegistry.h"
@@ -219 +220 @@
 }
 
+String decodeURLEscapeSequences(const String& string, const TextEncoding& encoding)
+{
+    if (string.isEmpty())
+        return string;
+    return decodeEscapeSequences<URLEscapeSequence>(string, encoding);
+}
+
 } // namespace WebCore

trunk/Source/WebCore/platform/text/TextEncoding.h

@@ -73 +73 @@
 WEBCORE_EXPORT const TextEncoding& WindowsLatin1Encoding();
 
+// Unescapes the given string using URL escaping rules.
+// DANGER: If the URL has "%00" in it,
+// the resulting string will have embedded null characters!
+WEBCORE_EXPORT String decodeURLEscapeSequences(const String&, const TextEncoding& = UTF8Encoding());
+
 inline String TextEncoding::decode(const char* characters, size_t length) const
 {

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2018-10-01  Alex Christensen  <achristensen@webkit.org>
+
+        URL should not use TextEncoding internally
+        https://bugs.webkit.org/show_bug.cgi?id=190111
+
+        Reviewed by Andy Estes.
+
+        * UIProcess/WebInspectorProxy.cpp:
+
 2018-10-01  Jeremy Jones  <jeremyj@apple.com>
 

trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp

@@ -41 +41 @@
 #include <WebCore/SharedBuffer.h>
 #include <WebCore/SoupNetworkSession.h>
+#include <WebCore/TextEncoding.h>
 #include <wtf/MainThread.h>
 #include <wtf/glib/RunLoopSourcePriority.h>

trunk/Source/WebKit/UIProcess/API/glib/WebKitFileChooserRequest.cpp

@@ -27 +27 @@
 #include "WebOpenPanelResultListenerProxy.h"
 #include <WebCore/FileSystem.h>
+#include <WebCore/TextEncoding.h>
 #include <WebCore/URL.h>
 #include <glib/gi18n-lib.h>

trunk/Source/WebKit/UIProcess/WebInspectorProxy.cpp

@@ -42 +42 @@
 #include "WebProcessProxy.h"
 #include <WebCore/NotImplemented.h>
+#include <WebCore/TextEncoding.h>
 #include <wtf/SetForScope.h>
 

trunk/Source/WebKit/WebProcess/Plugins/PluginView.cpp

@@ -68 +68 @@
 #include <WebCore/SecurityPolicy.h>
 #include <WebCore/Settings.h>
+#include <WebCore/TextEncoding.h>
 #include <WebCore/UserGestureIndicator.h>
 #include <wtf/CompletionHandler.h>

trunk/Source/WebKitLegacy/mac/ChangeLog

@@ +1 @@
+2018-10-01  Alex Christensen  <achristensen@webkit.org>
+
+        URL should not use TextEncoding internally
+        https://bugs.webkit.org/show_bug.cgi?id=190111
+
+        Reviewed by Andy Estes.
+
+        * Misc/WebNSURLExtras.mm:
+
 2018-09-28  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebKitLegacy/mac/Misc/WebNSURLExtras.mm

@@ -36 +36 @@
 #import <WebCore/URL.h>
 #import <WebCore/LoaderNSURLExtras.h>
+#import <WebCore/TextEncoding.h>
 #import <WebCore/WebCoreNSURLExtras.h>
 #import <wtf/Assertions.h>

trunk/Source/WebKitLegacy/win/WebDownloadCurl.cpp

@@ -51 +51 @@
 #include <WebCore/ResourceRequest.h>
 #include <WebCore/ResourceResponse.h>
+#include <WebCore/TextEncoding.h>
 
 using namespace WebCore;

trunk/Tools/ChangeLog

@@ +1 @@
+2018-10-01  Alex Christensen  <achristensen@webkit.org>
+
+        URL should not use TextEncoding internally
+        https://bugs.webkit.org/show_bug.cgi?id=190111
+
+        Reviewed by Andy Estes.
+
+        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+        (TestWebKitAPI::testUserPass):
+        (TestWebKitAPI::TEST_F):
+
 2018-10-01  Daniel Bates  <dabates@apple.com>
 

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

@@ -501 +501 @@
     // 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/"});
+}
+
+static void testUserPass(const String& value, const String& decoded, const String& encoded)
+{
+    URL userURL(URL(), makeString("http://", value, "@example.com/"));
+    URL passURL(URL(), makeString("http://user:", value, "@example.com/"));
+    EXPECT_EQ(encoded, userURL.encodedUser());
+    EXPECT_EQ(encoded, passURL.encodedPass());
+    EXPECT_EQ(decoded, userURL.user());
+    EXPECT_EQ(decoded, passURL.pass());
+}
+
+static void testUserPass(const String& value, const String& encoded)
+{
+    testUserPass(value, value, encoded);
+}
+
+TEST_F(URLParserTest, Credentials)
+{
+    auto validSurrogate = utf16String<3>({0xD800, 0xDD55, '\0'});
+    auto invalidSurrogate = utf16String<3>({0xD800, 'A', '\0'});
+    auto replacementA = utf16String<3>({0xFFFD, 'A', '\0'});
+
+    testUserPass("a", "a");
+    testUserPass("%", "%");
+    testUserPass("%25", "%", "%25");
+    testUserPass("%2525", "%25", "%2525");
+    testUserPass("%FX", "%FX");
+    testUserPass("%00", String::fromUTF8("\0", 1), "%00");
+    testUserPass("%F%25", "%F%", "%F%25");
+    testUserPass("%X%25", "%X%", "%X%25");
+    testUserPass("%%25", "%%", "%%25");
+    testUserPass("💩", "%C3%B0%C2%9F%C2%92%C2%A9");
+    testUserPass("%💩", "%%C3%B0%C2%9F%C2%92%C2%A9");
+    testUserPass(validSurrogate, "%F0%90%85%95");
+    testUserPass(replacementA, "%EF%BF%BDA");
+    testUserPass(invalidSurrogate, replacementA, "%EF%BF%BDA");
 }