Fix cookies with private browsing and NetworkSession
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Mar 2016 02:40:12 +0000 (02:40 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Mar 2016 02:40:12 +0000 (02:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=155147
rdar://problem/25018279

Reviewed by Brady Eidson.

Source/WebKit2:

In the NetworkSession constructor, we look for a NetworkStorageSession in the SessionTracker
and use that NetworkStorageSession's CFHTTPCookieStorageRef in the NSURLSessionConfiguration.
NetworkStorageSessions were being set in SessionTracker's storageSessionMap after NetworkSessions
were created, causing the NSHTTPCookieStorage of the NSURLSession to be different from the
CFHTTPCookieStorageRef used by document.cookie in newly created private browsing sessions.
This fixes that problem by passing the NetworkStorageSession as a constructor parameter to the
NetworkSession so it can use the correct CFHTTPCookieStorageRef in its NSURLSessionConfiguration
before it is in the storageSessionMap.

* NetworkProcess/NetworkSession.h:
(WebKit::NetworkSession::sessionID):
* NetworkProcess/cocoa/NetworkSessionCocoa.mm:
(WebKit::NetworkSession::defaultSession):
(WebKit::NetworkSession::NetworkSession):
* NetworkProcess/mac/RemoteNetworkingContext.mm:
(WebKit::RemoteNetworkingContext::ensurePrivateBrowsingSession):
* WebProcess/WebCoreSupport/mac/WebFrameNetworkingContext.mm:
(WebKit::WebFrameNetworkingContext::ensurePrivateBrowsingSession):

LayoutTests:

* http/tests/cookies/private-cookie-storage-expected.txt: Added.
* http/tests/cookies/private-cookie-storage.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/cookies/private-cookie-storage-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/cookies/private-cookie-storage.html [new file with mode: 0644]
Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/NetworkSession.h
Source/WebKit2/NetworkProcess/cocoa/NetworkSessionCocoa.mm
Source/WebKit2/NetworkProcess/mac/RemoteNetworkingContext.mm
Source/WebKit2/WebProcess/WebCoreSupport/mac/WebFrameNetworkingContext.mm

index fa7ebe3..6300bd6 100644 (file)
@@ -1,3 +1,14 @@
+2016-03-07  Alex Christensen  <achristensen@webkit.org>
+
+        Fix cookies with private browsing and NetworkSession
+        https://bugs.webkit.org/show_bug.cgi?id=155147
+        rdar://problem/25018279
+
+        Reviewed by Brady Eidson.
+
+        * http/tests/cookies/private-cookie-storage-expected.txt: Added.
+        * http/tests/cookies/private-cookie-storage.html: Added.
+
 2016-03-07  Ryan Haddad  <ryanhaddad@apple.com>
 
         Marking media/track/track-cues-pause-on-exit.html as flaky on ElCapitan Debug WK2
diff --git a/LayoutTests/http/tests/cookies/private-cookie-storage-expected.txt b/LayoutTests/http/tests/cookies/private-cookie-storage-expected.txt
new file mode 100644 (file)
index 0000000..b2f6b7c
--- /dev/null
@@ -0,0 +1,18 @@
+This test checks that cookies set from a Set-Cookie header in private browsing mode are stored in the same cookie storage as cookies set with document.cookie.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Cookies before setting (should be empty):
+Cookies before enabling private browsing (should be key1=value1):key1=value1
+PASS cookie is 'key1=value1'.
+Cookies after enabling private browsing (should be empty):
+PASS cookie is ''.
+Cookies after setting in private browsing (should be key2=value2):key2=value2
+PASS cookie is 'key2=value2'.
+Cookies after disabling private browsing (should be key1=value1):key1=value1
+PASS cookie is 'key1=value1'.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/cookies/private-cookie-storage.html b/LayoutTests/http/tests/cookies/private-cookie-storage.html
new file mode 100644 (file)
index 0000000..72b90df
--- /dev/null
@@ -0,0 +1,26 @@
+<script src="resources/cookies-test-pre.js"></script>
+<script>
+description("This test checks that cookies set from a Set-Cookie header in private browsing mode are stored in the same cookie storage as cookies set with document.cookie.");
+
+debug("Cookies before setting (should be empty):" + document.cookie);
+
+document.cookie = "key1=value1";
+debug("Cookies before enabling private browsing (should be key1=value1):" + document.cookie);
+testCookies("key1=value1");
+
+if (window.testRunner) { testRunner.setPrivateBrowsingEnabled(true); }
+
+debug("Cookies after enabling private browsing (should be empty):" + document.cookie);
+testCookies(""); // We're using a different cookie storage now.  key1=value1 is still in the default cookie storage.
+document.cookie = "key2=value2";
+debug("Cookies after setting in private browsing (should be key2=value2):" + document.cookie);
+testCookies("key2=value2");
+clearCookies();
+
+if (window.testRunner) { testRunner.setPrivateBrowsingEnabled(false); }
+
+debug("Cookies after disabling private browsing (should be key1=value1):" + document.cookie);
+testCookies("key1=value1");
+
+</script>
+<script src="resources/cookies-test-post.js"></script>
index 962b61b..e687b60 100644 (file)
@@ -1,3 +1,30 @@
+2016-03-07  Alex Christensen  <achristensen@webkit.org>
+
+        Fix cookies with private browsing and NetworkSession
+        https://bugs.webkit.org/show_bug.cgi?id=155147
+        rdar://problem/25018279
+
+        Reviewed by Brady Eidson.
+
+        In the NetworkSession constructor, we look for a NetworkStorageSession in the SessionTracker
+        and use that NetworkStorageSession's CFHTTPCookieStorageRef in the NSURLSessionConfiguration.
+        NetworkStorageSessions were being set in SessionTracker's storageSessionMap after NetworkSessions
+        were created, causing the NSHTTPCookieStorage of the NSURLSession to be different from the
+        CFHTTPCookieStorageRef used by document.cookie in newly created private browsing sessions.
+        This fixes that problem by passing the NetworkStorageSession as a constructor parameter to the
+        NetworkSession so it can use the correct CFHTTPCookieStorageRef in its NSURLSessionConfiguration
+        before it is in the storageSessionMap.
+
+        * NetworkProcess/NetworkSession.h:
+        (WebKit::NetworkSession::sessionID):
+        * NetworkProcess/cocoa/NetworkSessionCocoa.mm:
+        (WebKit::NetworkSession::defaultSession):
+        (WebKit::NetworkSession::NetworkSession):
+        * NetworkProcess/mac/RemoteNetworkingContext.mm:
+        (WebKit::RemoteNetworkingContext::ensurePrivateBrowsingSession):
+        * WebProcess/WebCoreSupport/mac/WebFrameNetworkingContext.mm:
+        (WebKit::WebFrameNetworkingContext::ensurePrivateBrowsingSession):
+
 2016-03-07  Brent Fulgham  <bfulgham@apple.com>
 
         Correct bug in resource load statistics debug flag for legacy clients
index 5753881..387a4f4 100644 (file)
@@ -39,6 +39,10 @@ OBJC_CLASS WKNetworkSessionDelegate;
 #include <wtf/Ref.h>
 #include <wtf/RefCounted.h>
 
+namespace WebCore {
+class NetworkStorageSession;
+}
+
 namespace WebKit {
 
 class CustomProtocolManager;
@@ -50,7 +54,7 @@ public:
         Normal,
         Ephemeral
     };
-    NetworkSession(Type, WebCore::SessionID, CustomProtocolManager*);
+    NetworkSession(Type, WebCore::SessionID, CustomProtocolManager*, WebCore::NetworkStorageSession*);
     ~NetworkSession();
 
     WebCore::SessionID sessionID() { return m_sessionID; }
index ed5326f..e761f8a 100644 (file)
@@ -259,11 +259,11 @@ void NetworkSession::setCustomProtocolManager(CustomProtocolManager* customProto
 NetworkSession& NetworkSession::defaultSession()
 {
     ASSERT(isMainThread());
-    static NeverDestroyed<NetworkSession> session(NetworkSession::Type::Normal, WebCore::SessionID::defaultSessionID(), globalCustomProtocolManager().get());
+    static NeverDestroyed<NetworkSession> session(NetworkSession::Type::Normal, WebCore::SessionID::defaultSessionID(), globalCustomProtocolManager().get(), &WebCore::NetworkStorageSession::defaultStorageSession());
     return session;
 }
 
-NetworkSession::NetworkSession(Type type, WebCore::SessionID sessionID, CustomProtocolManager* customProtocolManager)
+NetworkSession::NetworkSession(Type type, WebCore::SessionID sessionID, CustomProtocolManager* customProtocolManager, WebCore::NetworkStorageSession* networkStorageSession)
     : m_sessionID(sessionID)
 {
     m_sessionDelegate = adoptNS([[WKNetworkSessionDelegate alloc] initWithNetworkSession:*this]);
@@ -279,8 +279,8 @@ NetworkSession::NetworkSession(Type type, WebCore::SessionID sessionID, CustomPr
     setCollectsTimingData();
 #endif
 
-    if (auto* storageSession = SessionTracker::storageSession(sessionID)) {
-        if (CFHTTPCookieStorageRef storage = storageSession->cookieStorage().get())
+    if (networkStorageSession) {
+        if (CFHTTPCookieStorageRef storage = networkStorageSession->cookieStorage().get())
             configuration.HTTPCookieStorage = [[[NSHTTPCookieStorage alloc] _initWithCFHTTPCookieStorage:storage] autorelease];
     }
     m_sessionWithCredentialStorage = [NSURLSession sessionWithConfiguration:configuration delegate:static_cast<id>(m_sessionDelegate.get()) delegateQueue:[NSOperationQueue mainQueue]];
index 44a1d73..8d9a0ad 100644 (file)
@@ -100,9 +100,13 @@ void RemoteNetworkingContext::ensurePrivateBrowsingSession(SessionID sessionID)
     else
         base = SessionTracker::getIdentifierBase();
 
-    SessionTracker::setSession(sessionID, NetworkStorageSession::createPrivateBrowsingSession(base + '.' + String::number(sessionID.sessionID()))
+    auto networkStorageSession = NetworkStorageSession::createPrivateBrowsingSession(base + '.' + String::number(sessionID.sessionID()));
 #if USE(NETWORK_SESSION)
-        , std::make_unique<NetworkSession>(NetworkSession::Type::Ephemeral, sessionID, NetworkProcess::singleton().supplement<CustomProtocolManager>())
+    auto networkSession = std::make_unique<NetworkSession>(NetworkSession::Type::Ephemeral, sessionID, NetworkProcess::singleton().supplement<CustomProtocolManager>(), networkStorageSession.get());
+#endif
+    SessionTracker::setSession(sessionID, WTFMove(networkStorageSession)
+#if USE(NETWORK_SESSION)
+        , WTFMove(networkSession)
 #endif
     );
 }
index 202c80d..807d1dc 100644 (file)
@@ -58,9 +58,13 @@ void WebFrameNetworkingContext::ensurePrivateBrowsingSession(SessionID sessionID
     else
         base = SessionTracker::getIdentifierBase();
 
-    SessionTracker::setSession(sessionID, NetworkStorageSession::createPrivateBrowsingSession(base + '.' + String::number(sessionID.sessionID()))
+    auto networkStorageSession = NetworkStorageSession::createPrivateBrowsingSession(base + '.' + String::number(sessionID.sessionID()));
 #if USE(NETWORK_SESSION)
-        , std::make_unique<NetworkSession>(NetworkSession::Type::Ephemeral, sessionID, nullptr)
+    auto networkSession = std::make_unique<NetworkSession>(NetworkSession::Type::Ephemeral, sessionID, nullptr, networkStorageSession.get());
+#endif
+    SessionTracker::setSession(sessionID, WTFMove(networkStorageSession)
+#if USE(NETWORK_SESSION)
+        , WTFMove(networkSession)
 #endif
     );
 }