Attachment 341537 Details for Bug 186057 – Patch

[patch] Patch

bug-186057-20180529164953.patch (text/plain), 12.69 KB, created by youenn fablet on 2018-05-29 16:49:53 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2018-05-29 16:49:53 PDT

Size: 12.69 KB

patch

obsolete

>Subversion Revision: 232263
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index bc0ea74ecb7dc89c21a3c320b511504c1e2f43df..fa5683d81d14c1d03d9887674906f3090b968c2f 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2018-05-27  Youenn Fablet  <youenn@apple.com>
>+
>+        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  Nan Wang  <n_wang@apple.com>
> 
>         AX: setValue on contenteditable should preserve whitespace
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 23ffac7f794c5707068e0e3de019ebeb46a2f052..5365dc48fe03f803001b2c15f302190f5f8d2803 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-05-27  Youenn Fablet  <youenn@apple.com>
>+
>+        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  Geoffrey Garen  <ggaren@apple.com>
> 
>         Removed some unused WebSQL trackers
>diff --git a/Source/WebCore/platform/URL.h b/Source/WebCore/platform/URL.h
>index d9a02a669236f76c267c0c52a60f2d6ef3aa2c95..d1e609a7c67bb77e89636d9b27a57be9d0a7ff51 100644
>--- a/Source/WebCore/platform/URL.h
>+++ b/Source/WebCore/platform/URL.h
>@@ -120,6 +120,7 @@ public:
>     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 @@ private:
>     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;
>diff --git a/Source/WebCore/platform/cf/CFURLExtras.cpp b/Source/WebCore/platform/cf/CFURLExtras.cpp
>index 9178cb192f053e063a86afee38a760f7289b54ee..73b43bc7a07b80ae4baa35fdb7335ac3ba764417 100644
>--- a/Source/WebCore/platform/cf/CFURLExtras.cpp
>+++ b/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);
>+}
>+
> }
>diff --git a/Source/WebCore/platform/cf/CFURLExtras.h b/Source/WebCore/platform/cf/CFURLExtras.h
>index d5e72bf50d3969c400e1f5f6cc26927a99f8e051..ffd570660da6aa98d122864078fed7dd6e779ef5 100644
>--- a/Source/WebCore/platform/cf/CFURLExtras.h
>+++ b/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
>diff --git a/Source/WebCore/platform/cf/URLCF.cpp b/Source/WebCore/platform/cf/URLCF.cpp
>index 8fedef226f1b94d1dffcdee1be4d885bd713f77c..ab641ed2d9bec13d6a1778f63662f9b667a971c9 100644
>--- a/Source/WebCore/platform/cf/URLCF.cpp
>+++ b/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
> 
>diff --git a/Source/WebCore/platform/mac/URLMac.mm b/Source/WebCore/platform/mac/URLMac.mm
>index fc5c46d6327304a72dbe41852a0beb7bd7a3ba97..2164116be0318ab97c7daa288b890efeeff2a47a 100644
>--- a/Source/WebCore/platform/mac/URLMac.mm
>+++ b/Source/WebCore/platform/mac/URLMac.mm
>@@ -69,14 +69,22 @@ RetainPtr<CFURLRef> URL::createCFURL() const
>         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(const String& host)
>diff --git a/Source/WebCore/platform/mac/WebCoreNSURLExtras.mm b/Source/WebCore/platform/mac/WebCoreNSURLExtras.mm
>index e4fe33a5d4580d59f4bee53e19c73b2e0c28ec6f..88d9c0baf445c97c040f5b8e636cedc8d4a6bcd1 100644
>--- a/Source/WebCore/platform/mac/WebCoreNSURLExtras.mm
>+++ b/Source/WebCore/platform/mac/WebCoreNSURLExtras.mm
>@@ -886,7 +886,7 @@ static NSData *dataWithUserTypedString(NSString *string)
>     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 @@ NSURL *URLWithUserTypedString(NSString *string, NSURL *URL)
>     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)
>diff --git a/Source/WebKit/Shared/Cocoa/WKNSURLExtras.mm b/Source/WebKit/Shared/Cocoa/WKNSURLExtras.mm
>index 56d4910dce16a3385b4f817e0604812c66430383..895afe99263683a5d32e7f02b49536d3b6a5a363 100644
>--- a/Source/WebKit/Shared/Cocoa/WKNSURLExtras.mm
>+++ b/Source/WebKit/Shared/Cocoa/WKNSURLExtras.mm
>@@ -27,6 +27,7 @@
> #import "WKNSURLExtras.h"
> 
> #import <WebCore/CFURLExtras.h>
>+#import <WebCore/URL.h>
> #import <wtf/text/CString.h>
> #import <wtf/text/WTFString.h>
> 
>@@ -34,23 +35,16 @@ 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
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 5a25e5739a5bfd6af7e7a5b84bbb0e4c0ce3e826..6b5c474e43c0e4f13443c2b42651bad670b92142 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,20 @@
>+2018-05-29  Youenn Fablet  <youenn@apple.com>
>+
>+        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  Sihui Liu  <sihui_liu@apple.com>
> 
>         Unable to remove IndexedDB Databases with Cocoa API removeDataOfTypes
>diff --git a/Tools/DumpRenderTree/TestRunner.cpp b/Tools/DumpRenderTree/TestRunner.cpp
>index 707a6b9253d1ffae85acb201e3954aec541ce7ba..f8ad95151740b66c74692ad52d374a305da44671 100644
>--- a/Tools/DumpRenderTree/TestRunner.cpp
>+++ b/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)
>diff --git a/Tools/DumpRenderTree/TestRunner.h b/Tools/DumpRenderTree/TestRunner.h
>index 1e8c1e94f6cb4735c8f48a0702494f788099141b..3e328b822323fec4b4b54f777aced2434646b6c1 100644
>--- a/Tools/DumpRenderTree/TestRunner.h
>+++ b/Tools/DumpRenderTree/TestRunner.h
>@@ -59,7 +59,7 @@ public:
>     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);
>diff --git a/Tools/DumpRenderTree/mac/ResourceLoadDelegate.mm b/Tools/DumpRenderTree/mac/ResourceLoadDelegate.mm
>index 1b383bb305f0b77efe6697d11a47244f0a1937b8..6cb5f24262397b21d110aa50adcd47a88b24c3f9 100644
>--- a/Tools/DumpRenderTree/mac/ResourceLoadDelegate.mm
>+++ b/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];
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 186057: 341500 | 341524 | 341537