Title: [278800] trunk
Revision
278800
Author
commit-qu...@webkit.org
Date
2021-06-11 17:06:21 -0700 (Fri, 11 Jun 2021)

Log Message

Partition CrossOriginPreflightResultCache by SessionID
https://bugs.webkit.org/show_bug.cgi?id=226910

Patch by Alex Christensen <achristen...@webkit.org> on 2021-06-11
Reviewed by Youenn Fablet.

Source/WebCore:

* loader/CrossOriginAccessControl.cpp:
(WebCore::validatePreflightResponse):
* loader/CrossOriginAccessControl.h:
* loader/CrossOriginPreflightChecker.cpp:
(WebCore::CrossOriginPreflightChecker::validatePreflightResponse):
* loader/CrossOriginPreflightResultCache.cpp:
(WebCore::CrossOriginPreflightResultCache::appendEntry):
(WebCore::CrossOriginPreflightResultCache::canSkipPreflight):
* loader/CrossOriginPreflightResultCache.h:
* loader/DocumentThreadableLoader.cpp:
(WebCore::DocumentThreadableLoader::makeCrossOriginAccessRequest):

Source/WebKit:

* NetworkProcess/NetworkCORSPreflightChecker.cpp:
(WebKit::NetworkCORSPreflightChecker::didCompleteWithError):
* NetworkProcess/NetworkLoadChecker.cpp:
(WebKit::NetworkLoadChecker::checkCORSRequestWithPreflight):

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm:
(TEST):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (278799 => 278800)


--- trunk/Source/WebCore/ChangeLog	2021-06-11 23:45:12 UTC (rev 278799)
+++ trunk/Source/WebCore/ChangeLog	2021-06-12 00:06:21 UTC (rev 278800)
@@ -1,3 +1,22 @@
+2021-06-11  Alex Christensen  <achristen...@webkit.org>
+
+        Partition CrossOriginPreflightResultCache by SessionID
+        https://bugs.webkit.org/show_bug.cgi?id=226910
+
+        Reviewed by Youenn Fablet.
+
+        * loader/CrossOriginAccessControl.cpp:
+        (WebCore::validatePreflightResponse):
+        * loader/CrossOriginAccessControl.h:
+        * loader/CrossOriginPreflightChecker.cpp:
+        (WebCore::CrossOriginPreflightChecker::validatePreflightResponse):
+        * loader/CrossOriginPreflightResultCache.cpp:
+        (WebCore::CrossOriginPreflightResultCache::appendEntry):
+        (WebCore::CrossOriginPreflightResultCache::canSkipPreflight):
+        * loader/CrossOriginPreflightResultCache.h:
+        * loader/DocumentThreadableLoader.cpp:
+        (WebCore::DocumentThreadableLoader::makeCrossOriginAccessRequest):
+
 2021-06-11  Chris Dumez  <cdu...@apple.com>
 
         Use SharedBuffer!=() in CachedResource::tryReplaceEncodedData()

Modified: trunk/Source/WebCore/loader/CrossOriginAccessControl.cpp (278799 => 278800)


--- trunk/Source/WebCore/loader/CrossOriginAccessControl.cpp	2021-06-11 23:45:12 UTC (rev 278799)
+++ trunk/Source/WebCore/loader/CrossOriginAccessControl.cpp	2021-06-12 00:06:21 UTC (rev 278800)
@@ -257,7 +257,7 @@
     return { };
 }
 
-Expected<void, String> validatePreflightResponse(const ResourceRequest& request, const ResourceResponse& response, StoredCredentialsPolicy storedCredentialsPolicy, const SecurityOrigin& securityOrigin, const CrossOriginAccessControlCheckDisabler* checkDisabler)
+Expected<void, String> validatePreflightResponse(const PAL::SessionID& sessionID, const ResourceRequest& request, const ResourceResponse& response, StoredCredentialsPolicy storedCredentialsPolicy, const SecurityOrigin& securityOrigin, const CrossOriginAccessControlCheckDisabler* checkDisabler)
 {
     if (!response.isSuccessful())
         return makeUnexpected("Preflight response is not successful"_s);
@@ -272,7 +272,7 @@
 
     auto entry = WTFMove(result.value());
     auto errorDescription = entry->validateMethodAndHeaders(request.httpMethod(), request.httpHeaderFields());
-    CrossOriginPreflightResultCache::singleton().appendEntry(securityOrigin.toString(), request.url(), entry.moveToUniquePtr());
+    CrossOriginPreflightResultCache::singleton().appendEntry(sessionID, securityOrigin.toString(), request.url(), entry.moveToUniquePtr());
 
     if (errorDescription)
         return makeUnexpected(WTFMove(*errorDescription));

Modified: trunk/Source/WebCore/loader/CrossOriginAccessControl.h (278799 => 278800)


--- trunk/Source/WebCore/loader/CrossOriginAccessControl.h	2021-06-11 23:45:12 UTC (rev 278799)
+++ trunk/Source/WebCore/loader/CrossOriginAccessControl.h	2021-06-12 00:06:21 UTC (rev 278800)
@@ -33,6 +33,10 @@
 #include <wtf/Forward.h>
 #include <wtf/OptionSet.h>
 
+namespace PAL {
+class SessionID;
+}
+
 namespace WebCore {
 
 class CachedResourceRequest;
@@ -78,7 +82,7 @@
 };
 
 WEBCORE_EXPORT Expected<void, String> passesAccessControlCheck(const ResourceResponse&, StoredCredentialsPolicy, const SecurityOrigin&, const CrossOriginAccessControlCheckDisabler*);
-WEBCORE_EXPORT Expected<void, String> validatePreflightResponse(const ResourceRequest&, const ResourceResponse&, StoredCredentialsPolicy, const SecurityOrigin&, const CrossOriginAccessControlCheckDisabler*);
+WEBCORE_EXPORT Expected<void, String> validatePreflightResponse(const PAL::SessionID&, const ResourceRequest&, const ResourceResponse&, StoredCredentialsPolicy, const SecurityOrigin&, const CrossOriginAccessControlCheckDisabler*);
 
 WEBCORE_EXPORT std::optional<ResourceError> validateCrossOriginResourcePolicy(const SecurityOrigin&, const URL&, const ResourceResponse&);
 std::optional<ResourceError> validateRangeRequestedFlag(const ResourceRequest&, const ResourceResponse&);

Modified: trunk/Source/WebCore/loader/CrossOriginPreflightChecker.cpp (278799 => 278800)


--- trunk/Source/WebCore/loader/CrossOriginPreflightChecker.cpp	2021-06-11 23:45:12 UTC (rev 278799)
+++ trunk/Source/WebCore/loader/CrossOriginPreflightChecker.cpp	2021-06-12 00:06:21 UTC (rev 278800)
@@ -61,13 +61,20 @@
 void CrossOriginPreflightChecker::validatePreflightResponse(DocumentThreadableLoader& loader, ResourceRequest&& request, unsigned long identifier, const ResourceResponse& response)
 {
     auto* frame = loader.document().frame();
-    ASSERT(frame);
+    if (!frame) {
+        ASSERT_NOT_REACHED();
+        return;
+    }
 
-    auto result = WebCore::validatePreflightResponse(request, response, loader.options().storedCredentialsPolicy, loader.securityOrigin(), &CrossOriginAccessControlCheckDisabler::singleton());
+    auto* page = loader.document().page();
+    if (!page) {
+        ASSERT_NOT_REACHED();
+        return;
+    }
+
+    auto result = WebCore::validatePreflightResponse(page->sessionID(), request, response, loader.options().storedCredentialsPolicy, loader.securityOrigin(), &CrossOriginAccessControlCheckDisabler::singleton());
     if (!result) {
-        if (auto* document = frame->document())
-            document->addConsoleMessage(MessageSource::Security, MessageLevel::Error, result.error());
-
+        loader.document().addConsoleMessage(MessageSource::Security, MessageLevel::Error, result.error());
         loader.preflightFailure(identifier, ResourceError(errorDomainWebKitInternal, 0, request.url(), result.error(), ResourceError::Type::AccessControl));
         return;
     }

Modified: trunk/Source/WebCore/loader/CrossOriginPreflightResultCache.cpp (278799 => 278800)


--- trunk/Source/WebCore/loader/CrossOriginPreflightResultCache.cpp	2021-06-11 23:45:12 UTC (rev 278799)
+++ trunk/Source/WebCore/loader/CrossOriginPreflightResultCache.cpp	2021-06-12 00:06:21 UTC (rev 278800)
@@ -121,16 +121,16 @@
     return cache;
 }
 
-void CrossOriginPreflightResultCache::appendEntry(const String& origin, const URL& url, std::unique_ptr<CrossOriginPreflightResultCacheItem> preflightResult)
+void CrossOriginPreflightResultCache::appendEntry(const PAL::SessionID& sessionID, const String& origin, const URL& url, std::unique_ptr<CrossOriginPreflightResultCacheItem> preflightResult)
 {
     ASSERT(isMainThread());
-    m_preflightHashMap.set(std::make_pair(origin, url), WTFMove(preflightResult));
+    m_preflightHashMap.set(std::make_tuple(sessionID, origin, url), WTFMove(preflightResult));
 }
 
-bool CrossOriginPreflightResultCache::canSkipPreflight(const String& origin, const URL& url, StoredCredentialsPolicy storedCredentialsPolicy, const String& method, const HTTPHeaderMap& requestHeaders)
+bool CrossOriginPreflightResultCache::canSkipPreflight(const PAL::SessionID& sessionID, const String& origin, const URL& url, StoredCredentialsPolicy storedCredentialsPolicy, const String& method, const HTTPHeaderMap& requestHeaders)
 {
     ASSERT(isMainThread());
-    auto it = m_preflightHashMap.find(std::make_pair(origin, url));
+    auto it = m_preflightHashMap.find(std::make_tuple(sessionID, origin, url));
     if (it == m_preflightHashMap.end())
         return false;
 

Modified: trunk/Source/WebCore/loader/CrossOriginPreflightResultCache.h (278799 => 278800)


--- trunk/Source/WebCore/loader/CrossOriginPreflightResultCache.h	2021-06-11 23:45:12 UTC (rev 278799)
+++ trunk/Source/WebCore/loader/CrossOriginPreflightResultCache.h	2021-06-12 00:06:21 UTC (rev 278800)
@@ -27,6 +27,7 @@
 #pragma once
 
 #include "StoredCredentialsPolicy.h"
+#include <pal/SessionID.h>
 #include <wtf/Expected.h>
 #include <wtf/HashMap.h>
 #include <wtf/HashSet.h>
@@ -66,8 +67,8 @@
     WTF_MAKE_NONCOPYABLE(CrossOriginPreflightResultCache); WTF_MAKE_FAST_ALLOCATED;
 public:
     WEBCORE_EXPORT static CrossOriginPreflightResultCache& singleton();
-    WEBCORE_EXPORT void appendEntry(const String& origin, const URL&, std::unique_ptr<CrossOriginPreflightResultCacheItem>);
-    WEBCORE_EXPORT bool canSkipPreflight(const String& origin, const URL&, StoredCredentialsPolicy, const String& method, const HTTPHeaderMap& requestHeaders);
+    WEBCORE_EXPORT void appendEntry(const PAL::SessionID&, const String& origin, const URL&, std::unique_ptr<CrossOriginPreflightResultCacheItem>);
+    WEBCORE_EXPORT bool canSkipPreflight(const PAL::SessionID&, const String& origin, const URL&, StoredCredentialsPolicy, const String& method, const HTTPHeaderMap& requestHeaders);
     WEBCORE_EXPORT void clear();
 
 private:
@@ -74,7 +75,7 @@
     friend NeverDestroyed<CrossOriginPreflightResultCache>;
     CrossOriginPreflightResultCache();
 
-    HashMap<std::pair<String, URL>, std::unique_ptr<CrossOriginPreflightResultCacheItem>> m_preflightHashMap;
+    HashMap<std::tuple<PAL::SessionID, String, URL>, std::unique_ptr<CrossOriginPreflightResultCacheItem>> m_preflightHashMap;
 };
 
 inline CrossOriginPreflightResultCacheItem::CrossOriginPreflightResultCacheItem(MonotonicTime absoluteExpiryTime, StoredCredentialsPolicy  storedCredentialsPolicy, HashSet<String>&& methods, HashSet<String, ASCIICaseInsensitiveHash>&& headers)

Modified: trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp (278799 => 278800)


--- trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp	2021-06-11 23:45:12 UTC (rev 278799)
+++ trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp	2021-06-12 00:06:21 UTC (rev 278800)
@@ -223,7 +223,7 @@
             return;
 
         m_simpleRequest = false;
-        if (CrossOriginPreflightResultCache::singleton().canSkipPreflight(securityOrigin().toString(), request.url(), m_options.storedCredentialsPolicy, request.httpMethod(), request.httpHeaderFields()))
+        if (auto* page = document().page(); page && CrossOriginPreflightResultCache::singleton().canSkipPreflight(page->sessionID(), securityOrigin().toString(), request.url(), m_options.storedCredentialsPolicy, request.httpMethod(), request.httpHeaderFields()))
             preflightSuccess(WTFMove(request));
         else
             makeCrossOriginAccessRequestWithPreflight(WTFMove(request));

Modified: trunk/Source/WebKit/ChangeLog (278799 => 278800)


--- trunk/Source/WebKit/ChangeLog	2021-06-11 23:45:12 UTC (rev 278799)
+++ trunk/Source/WebKit/ChangeLog	2021-06-12 00:06:21 UTC (rev 278800)
@@ -1,3 +1,15 @@
+2021-06-11  Alex Christensen  <achristen...@webkit.org>
+
+        Partition CrossOriginPreflightResultCache by SessionID
+        https://bugs.webkit.org/show_bug.cgi?id=226910
+
+        Reviewed by Youenn Fablet.
+
+        * NetworkProcess/NetworkCORSPreflightChecker.cpp:
+        (WebKit::NetworkCORSPreflightChecker::didCompleteWithError):
+        * NetworkProcess/NetworkLoadChecker.cpp:
+        (WebKit::NetworkLoadChecker::checkCORSRequestWithPreflight):
+
 2021-06-11  Chris Dumez  <cdu...@apple.com>
 
         Enable more release logging in UIProcess/WebProcess for ephemeral sessions

Modified: trunk/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp (278799 => 278800)


--- trunk/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp	2021-06-11 23:45:12 UTC (rev 278799)
+++ trunk/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp	2021-06-12 00:06:21 UTC (rev 278800)
@@ -139,7 +139,7 @@
 
     RELEASE_LOG_IF_ALLOWED("didComplete http_status_code=%d", m_response.httpStatusCode());
 
-    auto result = validatePreflightResponse(m_parameters.originalRequest, m_response, m_parameters.storedCredentialsPolicy, m_parameters.sourceOrigin, m_networkResourceLoader.get());
+    auto result = validatePreflightResponse(m_parameters.sessionID, m_parameters.originalRequest, m_response, m_parameters.storedCredentialsPolicy, m_parameters.sourceOrigin, m_networkResourceLoader.get());
     if (!result) {
         RELEASE_LOG_IF_ALLOWED("didComplete, AccessControl error: %s", result.error().utf8().data());
         m_completionCallback(ResourceError { errorDomainWebKitInternal, 0, m_parameters.originalRequest.url(), result.error(), ResourceError::Type::AccessControl });

Modified: trunk/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp (278799 => 278800)


--- trunk/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp	2021-06-11 23:45:12 UTC (rev 278799)
+++ trunk/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp	2021-06-12 00:06:21 UTC (rev 278800)
@@ -370,8 +370,7 @@
     ASSERT(m_options.mode == FetchOptions::Mode::Cors);
 
     m_isSimpleRequest = false;
-    // FIXME: We should probably partition preflight result cache by session ID.
-    if (CrossOriginPreflightResultCache::singleton().canSkipPreflight(m_origin->toString(), request.url(), m_storedCredentialsPolicy, request.httpMethod(), m_originalRequestHeaders)) {
+    if (CrossOriginPreflightResultCache::singleton().canSkipPreflight(m_sessionID, m_origin->toString(), request.url(), m_storedCredentialsPolicy, request.httpMethod(), m_originalRequestHeaders)) {
         RELEASE_LOG_IF_ALLOWED("checkCORSRequestWithPreflight - preflight can be skipped thanks to cached result");
         updateRequestForAccessControl(request, *m_origin, m_storedCredentialsPolicy);
         handler(WTFMove(request));

Modified: trunk/Tools/ChangeLog (278799 => 278800)


--- trunk/Tools/ChangeLog	2021-06-11 23:45:12 UTC (rev 278799)
+++ trunk/Tools/ChangeLog	2021-06-12 00:06:21 UTC (rev 278800)
@@ -1,3 +1,13 @@
+2021-06-11  Alex Christensen  <achristen...@webkit.org>
+
+        Partition CrossOriginPreflightResultCache by SessionID
+        https://bugs.webkit.org/show_bug.cgi?id=226910
+
+        Reviewed by Youenn Fablet.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm:
+        (TEST):
+
 2021-06-11  Peng Liu  <peng.l...@apple.com>
 
         Fix the references to audio-buffer-size.html in project.pbxproj

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm (278799 => 278800)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm	2021-06-11 23:45:12 UTC (rev 278799)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NetworkProcess.mm	2021-06-12 00:06:21 UTC (rev 278800)
@@ -145,3 +145,50 @@
     while ([WKWebsiteDataStore _defaultNetworkProcessExists])
         TestWebKitAPI::Util::spinRunLoop();
 }
+
+TEST(NetworkProcess, CORSPreflightCachePartitioned)
+{
+    using namespace TestWebKitAPI;
+    size_t preflightRequestsReceived { 0 };
+    HTTPServer server([&] (Connection connection) {
+        connection.receiveHTTPRequest([&, connection] (Vector<char>&& request) {
+            const char* expectedRequestBegin = "OPTIONS / HTTP/1.1\r\n";
+            EXPECT_TRUE(!memcmp(request.data(), expectedRequestBegin, strlen(expectedRequestBegin)));
+            EXPECT_TRUE(strnstr(request.data(), "Origin: http://example.com\r\n", request.size()));
+            EXPECT_TRUE(strnstr(request.data(), "Access-Control-Request-Method: DELETE\r\n", request.size()));
+            
+            const char* response =
+            "HTTP/1.1 204 No Content\r\n"
+            "Access-Control-Allow-Origin: http://example.com\r\n"
+            "Access-Control-Allow-Methods: OPTIONS, GET, POST, DELETE\r\n"
+            "Cache-Control: max-age=604800\r\n\r\n";
+            connection.send(response, [&, connection] {
+                connection.receiveHTTPRequest([&, connection] (Vector<char>&& request) {
+                    const char* expectedRequestBegin = "DELETE / HTTP/1.1\r\n";
+                    EXPECT_TRUE(!memcmp(request.data(), expectedRequestBegin, strlen(expectedRequestBegin)));
+                    EXPECT_TRUE(strnstr(request.data(), "Origin: http://example.com\r\n", request.size()));
+                    const char* response =
+                    "HTTP/1.1 200 OK\r\n"
+                    "Content-Length: 2\r\n\r\n"
+                    "hi";
+                    connection.send(response, [&, connection] {
+                        preflightRequestsReceived++;
+                    });
+                });
+            });
+        });
+    });
+    NSString *html = [NSString stringWithFormat:@"<script>var xhr = new XMLHttpRequest();xhr.open('DELETE', 'http://localhost:%d/');xhr.send()</script>", server.port()];
+    NSURL *baseURL = [NSURL URLWithString:@"http://example.com/"];
+    auto firstWebView = adoptNS([WKWebView new]);
+    [firstWebView loadHTMLString:html baseURL:baseURL];
+    while (preflightRequestsReceived != 1)
+        TestWebKitAPI::Util::spinRunLoop();
+
+    auto configuration = adoptNS([WKWebViewConfiguration new]);
+    configuration.get().websiteDataStore = [WKWebsiteDataStore nonPersistentDataStore];
+    auto secondWebView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+    [secondWebView loadHTMLString:html baseURL:baseURL];
+    while (preflightRequestsReceived != 2)
+        TestWebKitAPI::Util::spinRunLoop();
+}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to