Add a consistency check between URL and CFURL

Source/WebCore:

https://bugs.webkit.org/show_bug.cgi?id=186057
<rdar://problem/40258457>

Reviewed by Geoff Garen.

It is important that WebCore::URL used in WebCore and CFURL that gets serialized in the network pipe remain consistent.
Otherwise, we will end-up with odd bugs.

We add such a check when creating a CFURL from an URL.
To make things more consistent, we also rely now more on WebCore::URL instead of directly creating a CFURL.

* platform/URL.h:
* platform/cf/CFURLExtras.cpp:
(WebCore::isCFURLSameOrigin):
* platform/cf/CFURLExtras.h:
* platform/cf/URLCF.cpp:
(WebCore::URL::createCFURL const):
* platform/mac/URLMac.mm:
(WebCore::URL::createCFURL const):
* platform/mac/WebCoreNSURLExtras.mm:
(WebCore::URLWithUserTypedString):

Source/WebKit:

https://bugs.webkit.org/show_bug.cgi?id=186057
<rdar://problem/40258457>

Reviewed by Geoff Garen.

* Shared/Cocoa/WKNSURLExtras.mm:
(+[NSURL _web_URLWithWTFString:relativeToURL:]):
(urlWithWTFString): Deleted.
(+[NSURL _web_URLWithWTFString:]): Deleted.

Tools:

https://bugs.webkit.org/show_bug.cgi?id=182444
<rdar://problem/37164835>

Reviewed by Geoff Garen.

DRT code expected a non null URL which is no longer the case now.
Updated DRT to remove that assumption.

* DumpRenderTree/TestRunner.cpp:
(TestRunner::redirectionDestinationForURL):
* DumpRenderTree/TestRunner.h:
* DumpRenderTree/mac/ResourceLoadDelegate.mm:
(-[ResourceLoadDelegate webView:resource:willSendRequest:redirectResponse:fromDataSource:]):


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

Source/WebCore/ChangeLog

@@ -1,3 +1,28 @@
+2018-05-29  Youenn Fablet  <[email protected]>
+
+        Add a consistency check between URL and CFURL
+        https://bugs.webkit.org/show_bug.cgi?id=186057
+        <rdar://problem/40258457>
+
+        Reviewed by Geoff Garen.
+
+        It is important that WebCore::URL used in WebCore and CFURL that gets serialized in the network pipe remain consistent.
+        Otherwise, we will end-up with odd bugs.
+
+        We add such a check when creating a CFURL from an URL.
+        To make things more consistent, we also rely now more on WebCore::URL instead of directly creating a CFURL.
+
+        * platform/URL.h:
+        * platform/cf/CFURLExtras.cpp:
+        (WebCore::isCFURLSameOrigin):
+        * platform/cf/CFURLExtras.h:
+        * platform/cf/URLCF.cpp:
+        (WebCore::URL::createCFURL const):
+        * platform/mac/URLMac.mm:
+        (WebCore::URL::createCFURL const):
+        * platform/mac/WebCoreNSURLExtras.mm:
+        (WebCore::URLWithUserTypedString):
+
 2018-05-29  Timothy Hatcher  <[email protected]>
 
         Printing does not apply the right colors in all cases.

Source/WebCore/platform/URL.h

@@ -120,6 +120,7 @@ class URL {
     bool hasPassword() const;
     bool hasQuery() const;
     bool hasFragment() const;
+    bool hasPath() const;
 
     // Unlike user() and pass(), these functions don't decode escape sequences.
     // This is necessary for accurate round-tripping, because encoding doesn't encode '%' characters.
@@ -217,8 +218,6 @@ class URL {
     void init(const URL&, const String&, const TextEncoding&);
     void copyToBuffer(Vector<char, 512>& buffer) const;
 
-    bool hasPath() const;
-
     String m_string;
     bool m_isValid : 1;
     bool m_protocolIsInHTTPFamily : 1;

Source/WebCore/platform/cf/CFURLExtras.cpp

@@ -26,6 +26,7 @@
 #include "config.h"
 #include "CFURLExtras.h"
 
+#include "URL.h"
 #include <wtf/text/CString.h>
 
 namespace WebCore {
@@ -59,4 +60,22 @@ void getURLBytes(CFURLRef url, CString& result)
     ASSERT_UNUSED(finalLength, finalLength == bytesLength);
 }
 
+bool isCFURLSameOrigin(CFURLRef cfURL, const URL& url)
+{
+    ASSERT(url.protocolIsInHTTPFamily());
+
+    if (url.hasUsername() || url.hasPassword())
+        return protocolHostAndPortAreEqual(url, URL { cfURL });
+
+    URLCharBuffer bytes;
+    getURLBytes(cfURL, bytes);
+    StringView cfURLString { reinterpret_cast<const LChar*>(bytes.data()), static_cast<unsigned>(bytes.size()) };
+
+    if (!url.hasPath())
+        return StringView { url.string() } == cfURLString;
+
+    auto urlWithoutPath = StringView { url.string() }.substring(0, url.pathStart() + 1);
+    return cfURLString.startsWith(urlWithoutPath);
+}
+
 }

Source/WebCore/platform/cf/CFURLExtras.h

@@ -32,12 +32,15 @@
 
 namespace WebCore {
 
+class URL;
 typedef Vector<char, 512> URLCharBuffer;
 
 WEBCORE_EXPORT RetainPtr<CFURLRef> createCFURLFromBuffer(const char*, size_t, CFURLRef baseURL = 0);
 WEBCORE_EXPORT void getURLBytes(CFURLRef, URLCharBuffer&);
 WEBCORE_EXPORT void getURLBytes(CFURLRef, CString&);
 
+bool isCFURLSameOrigin(CFURLRef, const URL&);
+
 }
 
 #endif // CFURLExtras_h

Source/WebCore/platform/cf/URLCF.cpp

@@ -59,7 +59,12 @@ RetainPtr<CFURLRef> URL::createCFURL() const
     // which is clearly wrong.
     URLCharBuffer buffer;
     copyToBuffer(buffer);
-    return createCFURLFromBuffer(buffer.data(), buffer.size());
+    auto cfURL = createCFURLFromBuffer(buffer.data(), buffer.size());
+
+    if (protocolIsInHTTPFamily() && !isCFURLSameOrigin(cfURL.get(), *this))
+        return nullptr;
+
+    return cfURL;
 }
 #endif
 

Source/WebCore/platform/mac/URLMac.mm

@@ -69,14 +69,22 @@ - (BOOL)_web_looksLikeIPAddress;
         return reinterpret_cast<CFURLRef>(adoptNS([[NSURL alloc] initWithString:@""]).get());
     }
 
+    RetainPtr<CFURLRef> cfURL;
+
     // Fast path if the input data is 8-bit to avoid copying into a temporary buffer.
     if (LIKELY(m_string.is8Bit()))
-        return createCFURLFromBuffer(reinterpret_cast<const char*>(m_string.characters8()), m_string.length());
+        cfURL = createCFURLFromBuffer(reinterpret_cast<const char*>(m_string.characters8()), m_string.length());
+    else {
+        // Slower path.
+        URLCharBuffer buffer;
+        copyToBuffer(buffer);
+        cfURL = createCFURLFromBuffer(buffer.data(), buffer.size());
+    }
+
+    if (protocolIsInHTTPFamily() && !isCFURLSameOrigin(cfURL.get(), *this))
+        return nullptr;
 
-    // Slower path.
-    URLCharBuffer buffer;
-    copyToBuffer(buffer);
-    return createCFURLFromBuffer(buffer.data(), buffer.size());
+    return cfURL;
 }
 
 bool URL::hostIsIPAddress(StringView host)

Source/WebCore/platform/mac/WebCoreNSURLExtras.mm

@@ -886,7 +886,7 @@ static void applyHostNameFunctionToURLString(NSString *string, StringRangeApplie
     return [NSData dataWithBytesNoCopy:outBytes length:outLength]; // adopts outBytes
 }
 
-NSURL *URLWithUserTypedString(NSString *string, NSURL *URL)
+NSURL *URLWithUserTypedString(NSString *string, NSURL *nsURL)
 {
     if (!string)
         return nil;
@@ -895,11 +895,8 @@ static void applyHostNameFunctionToURLString(NSString *string, StringRangeApplie
     if (!string)
         return nil;
 
-    NSData *data = dataWithUserTypedString(string);
-    if (!data)
-        return [NSURL URLWithString:@""];
-
-    return URLWithData(data, URL);
+    URL url { URL { nsURL }, string };
+    return (__bridge NSURL*) url.createCFURL().autorelease();
 }
 
 NSURL *URLWithUserTypedStringDeprecated(NSString *string, NSURL *URL)

Source/WebKit/ChangeLog

@@ -1,3 +1,16 @@
+2018-05-29  Youenn Fablet  <[email protected]>
+
+        Add a consistency check between URL and CFURL
+        https://bugs.webkit.org/show_bug.cgi?id=186057
+        <rdar://problem/40258457>
+
+        Reviewed by Geoff Garen.
+
+        * Shared/Cocoa/WKNSURLExtras.mm:
+        (+[NSURL _web_URLWithWTFString:relativeToURL:]):
+        (urlWithWTFString): Deleted.
+        (+[NSURL _web_URLWithWTFString:]): Deleted.
+
 2018-05-29  Alex Christensen  <[email protected]>
 
         Remove unused WebPage::dummy

Source/WebKit/Shared/Cocoa/WKNSURLExtras.mm

@@ -27,30 +27,24 @@
 #import "WKNSURLExtras.h"
 
 #import <WebCore/CFURLExtras.h>
+#import <WebCore/URL.h>
 #import <wtf/text/CString.h>
 #import <wtf/text/WTFString.h>
 
 using namespace WebCore;
 
 @implementation NSURL (WKExtras)
 
-static inline NSURL *urlWithWTFString(const String& string, NSURL *baseURL = nil)
-{
-    if (!string)
-        return nil;
-
-    CString buffer = string.utf8();
-    return (NSURL *)createCFURLFromBuffer(buffer.data(), buffer.length(), (CFURLRef)baseURL).autorelease();
-}
-
 + (instancetype)_web_URLWithWTFString:(const String&)string
 {
-    return urlWithWTFString(string);
+    URL url { URL { }, string };
+    return (__bridge NSURL*) url.createCFURL().autorelease();
 }
 
 + (instancetype)_web_URLWithWTFString:(const String&)string relativeToURL:(NSURL *)baseURL
 {
-    return urlWithWTFString(string, baseURL);
+    URL url { URL { baseURL }, string };
+    return (__bridge NSURL*) url.createCFURL().autorelease();
 }
 
 - (String)_web_originalDataAsWTFString

Tools/ChangeLog

@@ -1,3 +1,20 @@
+2018-05-29  Youenn Fablet  <[email protected]>
+
+        Add a consistency check between URL and CFURL
+        https://bugs.webkit.org/show_bug.cgi?id=182444
+        <rdar://problem/37164835>
+
+        Reviewed by Geoff Garen.
+
+        DRT code expected a non null URL which is no longer the case now.
+        Updated DRT to remove that assumption.
+
+        * DumpRenderTree/TestRunner.cpp:
+        (TestRunner::redirectionDestinationForURL):
+        * DumpRenderTree/TestRunner.h:
+        * DumpRenderTree/mac/ResourceLoadDelegate.mm:
+        (-[ResourceLoadDelegate webView:resource:willSendRequest:redirectResponse:fromDataSource:]):
+
 2018-05-29  Brendan McLoughlin  <[email protected]>
 
         Export changes to web-platform-test as part of the webkit-patch upload workflow

Tools/DumpRenderTree/TestRunner.cpp

@@ -2335,9 +2335,16 @@ void TestRunner::addURLToRedirect(std::string origin, std::string destination)
     m_URLsToRedirect[origin] = destination;
 }
 
-const std::string& TestRunner::redirectionDestinationForURL(std::string origin)
+const char* TestRunner::redirectionDestinationForURL(const char* origin)
 {
-    return m_URLsToRedirect[origin];
+    if (!origin)
+        return nullptr;
+
+    auto iterator = m_URLsToRedirect.find(origin);
+    if (iterator == m_URLsToRedirect.end())
+        return nullptr;
+
+    return iterator->second.data();
 }
 
 void TestRunner::setShouldPaintBrokenImage(bool shouldPaintBrokenImage)

Tools/DumpRenderTree/TestRunner.h

@@ -59,7 +59,7 @@ class TestRunner : public WTR::UIScriptContextDelegate, public RefCounted<TestRu
     const std::set<std::string>& allowedHosts() const { return m_allowedHosts; }
     void setAllowedHosts(std::set<std::string> hosts) { m_allowedHosts = WTFMove(hosts); }
     void addURLToRedirect(std::string origin, std::string destination);
-    const std::string& redirectionDestinationForURL(std::string);
+    const char* redirectionDestinationForURL(const char*);
     void clearAllApplicationCaches();
     void clearAllDatabases();
     void clearApplicationCacheForOrigin(JSStringRef name);

Tools/DumpRenderTree/mac/ResourceLoadDelegate.mm

@@ -182,9 +182,8 @@ -(NSURLRequest *)webView: (WebView *)wv resource:identifier willSendRequest: (NS
         [newRequest setValue:nil forHTTPHeaderField:nsHeader];
         [nsHeader release];
     }
-    const std::string& destination = gTestRunner->redirectionDestinationForURL([[url absoluteString] UTF8String]);
-    if (destination.length())
-        [newRequest setURL:[NSURL URLWithString:[NSString stringWithUTF8String:destination.data()]]];
+    if (auto* destination = gTestRunner->redirectionDestinationForURL([[url absoluteString] UTF8String]))
+        [newRequest setURL:[NSURL URLWithString:[NSString stringWithUTF8String:destination]]];
 
     return [newRequest autorelease];
 }