Changeset 250354 in webkit

Timestamp:

Sep 25, 2019 12:43:02 PM (5 years ago)

Author:

achristensen@apple.com

Message:

Don't fall back to default session if session can't be found for cookie operations
​https://bugs.webkit.org/show_bug.cgi?id=202222

Reviewed by Geoff Garen.

Apparently, during teardown of private browsing sessions, there is sometimes a race condition and cookies from a torn-down session are requested.
In this case, just fail like we do all other operations in this file. Otherwise, it's a breach of privacy.

• NetworkProcess/NetworkConnectionToWebProcess.cpp:

(WebKit::NetworkConnectionToWebProcess::storageSession):
(WebKit::NetworkConnectionToWebProcess::cookiesForDOM):
(WebKit::NetworkConnectionToWebProcess::setCookiesFromDOM):
(WebKit::NetworkConnectionToWebProcess::cookiesEnabled):
(WebKit::NetworkConnectionToWebProcess::cookieRequestHeaderFieldValue):
(WebKit::NetworkConnectionToWebProcess::getRawCookies):
(WebKit::NetworkConnectionToWebProcess::deleteCookie):

• NetworkProcess/NetworkConnectionToWebProcess.h:

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/NetworkConnectionToWebProcess.cpp (modified) (6 diffs)
• NetworkProcess/NetworkConnectionToWebProcess.h (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-09-25  Alex Christensen  <achristensen@webkit.org>
+
+        Don't fall back to default session if session can't be found for cookie operations
+        https://bugs.webkit.org/show_bug.cgi?id=202222
+
+        Reviewed by Geoff Garen.
+
+        Apparently, during teardown of private browsing sessions, there is sometimes a race condition and cookies from a torn-down session are requested.
+        In this case, just fail like we do all other operations in this file.  Otherwise, it's a breach of privacy.
+
+        * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+        (WebKit::NetworkConnectionToWebProcess::storageSession):
+        (WebKit::NetworkConnectionToWebProcess::cookiesForDOM):
+        (WebKit::NetworkConnectionToWebProcess::setCookiesFromDOM):
+        (WebKit::NetworkConnectionToWebProcess::cookiesEnabled):
+        (WebKit::NetworkConnectionToWebProcess::cookieRequestHeaderFieldValue):
+        (WebKit::NetworkConnectionToWebProcess::getRawCookies):
+        (WebKit::NetworkConnectionToWebProcess::deleteCookie):
+        * NetworkProcess/NetworkConnectionToWebProcess.h:
+
 2019-09-25  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp

@@ -495 +495 @@
 }
 
-NetworkStorageSession& NetworkConnectionToWebProcess::storageSession()
-{
-    if (m_sessionID != PAL::SessionID::defaultSessionID()) {
-        if (auto* storageSession = networkProcess().storageSession(m_sessionID))
-            return *storageSession;
-
-        // Some requests with private browsing mode requested may still be coming shortly after NetworkProcess was told to destroy its session.
-        // FIXME: Find a way to track private browsing sessions more rigorously.
+NetworkStorageSession* NetworkConnectionToWebProcess::storageSession()
+{
+    auto* session = networkProcess().storageSession(m_sessionID);
+    if (!session)
         LOG_ERROR("Non-default storage session was requested, but there was no session for it. Please file a bug unless you just disabled private browsing, in which case it's an expected race.");
-    }
-    return networkProcess().defaultStorageSession();
+    return session;
 }
 
@@ -534 +529 @@
 void NetworkConnectionToWebProcess::cookiesForDOM(const URL& firstParty, const SameSiteInfo& sameSiteInfo, const URL& url, Optional<FrameIdentifier> frameID, Optional<PageIdentifier> pageID, IncludeSecureCookies includeSecureCookies, CompletionHandler<void(String cookieString, bool secureCookiesAccessed)>&& completionHandler)
 {
-    auto& networkStorageSession = storageSession();
-    auto result = networkStorageSession.cookiesForDOM(firstParty, sameSiteInfo, url, frameID, pageID, includeSecureCookies);
+    auto* networkStorageSession = storageSession();
+    if (!networkStorageSession)
+        return completionHandler({ }, false);
+    auto result = networkStorageSession->cookiesForDOM(firstParty, sameSiteInfo, url, frameID, pageID, includeSecureCookies);
 #if ENABLE(RESOURCE_LOAD_STATISTICS) && !RELEASE_LOG_DISABLED
     if (auto* session = networkSession()) {
         if (session->shouldLogCookieInformation())
-            NetworkResourceLoader::logCookieInformation(*this, "NetworkConnectionToWebProcess::cookiesForDOM", reinterpret_cast<const void*>(this), networkStorageSession, firstParty, sameSiteInfo, url, emptyString(), frameID, pageID, WTF::nullopt);
+            NetworkResourceLoader::logCookieInformation(*this, "NetworkConnectionToWebProcess::cookiesForDOM", reinterpret_cast<const void*>(this), *networkStorageSession, firstParty, sameSiteInfo, url, emptyString(), frameID, pageID, WTF::nullopt);
     }
 #endif
@@ -547 +544 @@
 void NetworkConnectionToWebProcess::setCookiesFromDOM(const URL& firstParty, const SameSiteInfo& sameSiteInfo, const URL& url, Optional<WebCore::FrameIdentifier> frameID, Optional<PageIdentifier> pageID, const String& cookieString)
 {
-    auto& networkStorageSession = storageSession();
-    networkStorageSession.setCookiesFromDOM(firstParty, sameSiteInfo, url, frameID, pageID, cookieString);
+    auto* networkStorageSession = storageSession();
+    if (!networkStorageSession)
+        return;
+    networkStorageSession->setCookiesFromDOM(firstParty, sameSiteInfo, url, frameID, pageID, cookieString);
 #if ENABLE(RESOURCE_LOAD_STATISTICS) && !RELEASE_LOG_DISABLED
     if (auto* session = networkSession()) {
         if (session->shouldLogCookieInformation())
-            NetworkResourceLoader::logCookieInformation(*this, "NetworkConnectionToWebProcess::setCookiesFromDOM", reinterpret_cast<const void*>(this), networkStorageSession, firstParty, sameSiteInfo, url, emptyString(), frameID, pageID, WTF::nullopt);
+            NetworkResourceLoader::logCookieInformation(*this, "NetworkConnectionToWebProcess::setCookiesFromDOM", reinterpret_cast<const void*>(this), *networkStorageSession, firstParty, sameSiteInfo, url, emptyString(), frameID, pageID, WTF::nullopt);
     }
 #endif
@@ -559 +558 @@
 void NetworkConnectionToWebProcess::cookiesEnabled(CompletionHandler<void(bool)>&& completionHandler)
 {
-    completionHandler(storageSession().cookiesEnabled());
+    auto* networkStorageSession = storageSession();
+    if (!networkStorageSession)
+        return completionHandler(false);
+    completionHandler(networkStorageSession->cookiesEnabled());
 }
 
 void NetworkConnectionToWebProcess::cookieRequestHeaderFieldValue(const URL& firstParty, const SameSiteInfo& sameSiteInfo, const URL& url, Optional<FrameIdentifier> frameID, Optional<PageIdentifier> pageID, IncludeSecureCookies includeSecureCookies, CompletionHandler<void(String, bool)>&& completionHandler)
 {
-    auto result = storageSession().cookieRequestHeaderFieldValue(firstParty, sameSiteInfo, url, frameID, pageID, includeSecureCookies);
+    auto* networkStorageSession = storageSession();
+    if (!networkStorageSession)
+        return completionHandler({ }, false);
+    auto result = networkStorageSession->cookieRequestHeaderFieldValue(firstParty, sameSiteInfo, url, frameID, pageID, includeSecureCookies);
     completionHandler(WTFMove(result.first), result.second);
 }
@@ -570 +575 @@
 void NetworkConnectionToWebProcess::getRawCookies(const URL& firstParty, const SameSiteInfo& sameSiteInfo, const URL& url, Optional<FrameIdentifier> frameID, Optional<PageIdentifier> pageID, CompletionHandler<void(Vector<WebCore::Cookie>&&)>&& completionHandler)
 {
+    auto* networkStorageSession = storageSession();
+    if (!networkStorageSession)
+        return completionHandler({ });
     Vector<WebCore::Cookie> result;
-    storageSession().getRawCookies(firstParty, sameSiteInfo, url, frameID, pageID, result);
+    networkStorageSession->getRawCookies(firstParty, sameSiteInfo, url, frameID, pageID, result);
     completionHandler(WTFMove(result));
 }
@@ -577 +585 @@
 void NetworkConnectionToWebProcess::deleteCookie(const URL& url, const String& cookieName)
 {
-    storageSession().deleteCookie(url, cookieName);
+    auto* networkStorageSession = storageSession();
+    if (!networkStorageSession)
+        return;
+    networkStorageSession->deleteCookie(url, cookieName);
 }
 

trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h

@@ -160 +160 @@
 
     void didFinishPreconnection(uint64_t preconnectionIdentifier, const WebCore::ResourceError&);
-    WebCore::NetworkStorageSession& storageSession();
+    WebCore::NetworkStorageSession* storageSession();
 
     // IPC::Connection::Client