URL should not use TextEncoding internally
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Oct 2018 18:16:33 +0000 (18:16 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Oct 2018 18:16:33 +0000 (18:16 +0000)
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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236674 268f45cc-cd09-0410-ab3c-d52691b4dbfc

18 files changed:
Source/WebCore/ChangeLog
Source/WebCore/page/SecurityOrigin.cpp
Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp
Source/WebCore/platform/URL.cpp
Source/WebCore/platform/URL.h
Source/WebCore/platform/network/DataURLDecoder.cpp
Source/WebCore/platform/text/TextEncoding.cpp
Source/WebCore/platform/text/TextEncoding.h
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp
Source/WebKit/UIProcess/API/glib/WebKitFileChooserRequest.cpp
Source/WebKit/UIProcess/WebInspectorProxy.cpp
Source/WebKit/WebProcess/Plugins/PluginView.cpp
Source/WebKitLegacy/mac/ChangeLog
Source/WebKitLegacy/mac/Misc/WebNSURLExtras.mm
Source/WebKitLegacy/win/WebDownloadCurl.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp

index e019da1..dc86f10 100644 (file)
@@ -1,3 +1,28 @@
+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>
 
         <form> in quirks mode should have margin-block-end: 1em
index 962a2e1..229eff8 100644 (file)
@@ -34,6 +34,7 @@
 #include "URL.h"
 #include "SchemeRegistry.h"
 #include "SecurityPolicy.h"
+#include "TextEncoding.h"
 #include "ThreadableBlobRegistry.h"
 #include <wtf/MainThread.h>
 #include <wtf/NeverDestroyed.h>
index 07e946d..1b43312 100644 (file)
@@ -30,6 +30,7 @@
 #include "ContentSecurityPolicy.h"
 #include "ContentSecurityPolicyDirectiveNames.h"
 #include "ParsingUtilities.h"
+#include "TextEncoding.h"
 #include "URL.h"
 #include <wtf/ASCIICType.h>
 #include <wtf/NeverDestroyed.h>
index a15c590..05ea6ed 100644 (file)
@@ -27,8 +27,6 @@
 #include "config.h"
 #include "URL.h"
 
-#include "DecodeEscapeSequences.h"
-#include "TextEncoding.h"
 #include "URLParser.h"
 #include <stdio.h>
 #include <unicode/uidna.h>
@@ -187,9 +185,30 @@ String URL::protocolHostAndPort() const
     return result;
 }
 
+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));
 }
 
 String URL::pass() const
@@ -197,7 +216,7 @@ String URL::pass() const
     if (m_passwordEnd == m_userEnd)
         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));
 }
 
 String URL::encodedUser() const
@@ -238,7 +257,7 @@ String URL::fileSystemPath() const
     if (!isValid() || !isLocalFile())
         return String();
 
-    return decodeURLEscapeSequences(path());
+    return decodeEscapeSequencesFromParsedURL(StringView(path()));
 }
 
 #endif
@@ -658,20 +677,6 @@ void URL::setPath(const String& s)
     *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);
-}
-
 #if PLATFORM(IOS)
 
 static bool shouldCanonicalizeScheme = true;
index 1a6e6e4..b907169 100644 (file)
@@ -128,8 +128,8 @@ public:
 
     // 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;
 
@@ -302,13 +302,6 @@ bool isValidProtocol(const String&);
 
 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&);
 
index 60749ac..3f4fb1a 100644 (file)
@@ -29,6 +29,7 @@
 #include "DecodeEscapeSequences.h"
 #include "HTTPParsers.h"
 #include "SharedBuffer.h"
+#include "TextEncoding.h"
 #include "URL.h"
 #include <wtf/MainThread.h>
 #include <wtf/RunLoop.h>
index a009616..e305c8c 100644 (file)
@@ -28,6 +28,7 @@
 #include "config.h"
 #include "TextEncoding.h"
 
+#include "DecodeEscapeSequences.h"
 #include "TextCodec.h"
 #include "TextEncodingRegistry.h"
 #include <unicode/unorm.h>
@@ -218,4 +219,11 @@ const TextEncoding& WindowsLatin1Encoding()
     return globalWindowsLatin1Encoding;
 }
 
+String decodeURLEscapeSequences(const String& string, const TextEncoding& encoding)
+{
+    if (string.isEmpty())
+        return string;
+    return decodeEscapeSequences<URLEscapeSequence>(string, encoding);
+}
+
 } // namespace WebCore
index ffd35fc..77c5288 100644 (file)
@@ -72,6 +72,11 @@ const TextEncoding& UTF16LittleEndianEncoding();
 WEBCORE_EXPORT const TextEncoding& UTF8Encoding();
 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
 {
     bool ignored;
index 124deef..e5d3622 100644 (file)
@@ -1,3 +1,12 @@
+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>
 
         Unify implementation in VideoFullscreenInterfaceAVKit
index 849b4d5..d6d9ae2 100644 (file)
@@ -40,6 +40,7 @@
 #include <WebCore/NetworkStorageSession.h>
 #include <WebCore/SharedBuffer.h>
 #include <WebCore/SoupNetworkSession.h>
+#include <WebCore/TextEncoding.h>
 #include <wtf/MainThread.h>
 #include <wtf/glib/RunLoopSourcePriority.h>
 
index 7880494..cd026e4 100644 (file)
@@ -26,6 +26,7 @@
 #include "WebKitFileChooserRequestPrivate.h"
 #include "WebOpenPanelResultListenerProxy.h"
 #include <WebCore/FileSystem.h>
+#include <WebCore/TextEncoding.h>
 #include <WebCore/URL.h>
 #include <glib/gi18n-lib.h>
 #include <wtf/glib/GRefPtr.h>
index 689b4cf..200e52b 100644 (file)
@@ -41,6 +41,7 @@
 #include "WebProcessPool.h"
 #include "WebProcessProxy.h"
 #include <WebCore/NotImplemented.h>
+#include <WebCore/TextEncoding.h>
 #include <wtf/SetForScope.h>
 
 #if PLATFORM(GTK)
index e7b8064..f0752bd 100644 (file)
@@ -67,6 +67,7 @@
 #include <WebCore/SecurityOrigin.h>
 #include <WebCore/SecurityPolicy.h>
 #include <WebCore/Settings.h>
+#include <WebCore/TextEncoding.h>
 #include <WebCore/UserGestureIndicator.h>
 #include <wtf/CompletionHandler.h>
 #include <wtf/text/StringBuilder.h>
index 60da2f7..0c7dac0 100644 (file)
@@ -1,3 +1,12 @@
+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>
 
         Refactoring: eliminate raw pointer usage in Fullscreen code
index 972584c..f3baaea 100644 (file)
@@ -35,6 +35,7 @@
 #import <Foundation/NSURLRequest.h>
 #import <WebCore/URL.h>
 #import <WebCore/LoaderNSURLExtras.h>
+#import <WebCore/TextEncoding.h>
 #import <WebCore/WebCoreNSURLExtras.h>
 #import <wtf/Assertions.h>
 #import <wtf/ObjcRuntimeExtras.h>
index 644aaba..ed5c738 100644 (file)
@@ -50,6 +50,7 @@
 #include <WebCore/ResourceHandle.h>
 #include <WebCore/ResourceRequest.h>
 #include <WebCore/ResourceResponse.h>
+#include <WebCore/TextEncoding.h>
 
 using namespace WebCore;
 
index 8e128b4..b554c39 100644 (file)
@@ -1,3 +1,14 @@
+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>
 
         LLDB tests may use wrong configuration of lldbWebKitTester
index 56cb7a7..3e10592 100644 (file)
@@ -502,6 +502,43 @@ TEST_F(URLParserTest, Basic)
     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");
+}
+
 TEST_F(URLParserTest, ParseRelative)
 {
     checkRelativeURL("/index.html", "http://webkit.org/path1/path2/", {"http", "", "", "webkit.org", 0, "/index.html", "", "", "http://webkit.org/index.html"});