Vary:Cookie validation doesn't work in private browsing
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Jun 2016 14:12:16 +0000 (14:12 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Jun 2016 14:12:16 +0000 (14:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=158616
Source/WebCore:

<rdar://problem/26755067>

Reviewed by Andreas Kling.

There wasn't a way to get cookie based on SessionID from WebCore.

* platform/CookiesStrategy.h:

    Add a cookie retrival function that takes SessionID instead of NetworkStorageSession.

* platform/network/CacheValidation.cpp:
(WebCore::headerValueForVary):

    Use it.

(WebCore::verifyVaryingRequestHeaders):

Source/WebKit/mac:

<rdar://problem/26755067>

Reviewed by Andreas Kling.

* WebCoreSupport/WebFrameNetworkingContext.h:
(WebFrameNetworkingContext::create):
* WebCoreSupport/WebFrameNetworkingContext.mm:
(privateSession):
(WebFrameNetworkingContext::ensurePrivateBrowsingSession):

    Expose the private browsing session.

(WebFrameNetworkingContext::destroyPrivateBrowsingSession):
* WebCoreSupport/WebPlatformStrategies.h:
* WebCoreSupport/WebPlatformStrategies.mm:
(WebPlatformStrategies::cookieRequestHeaderFieldValue):

    Implement SessionID version of the function.

(WebPlatformStrategies::getRawCookies):

Source/WebKit2:

<rdar://problem/26755067>

Reviewed by Andreas Kling.

* WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:
(WebKit::WebPlatformStrategies::cookieRequestHeaderFieldValue):

    Implement SessionID version of the function.

* WebProcess/WebCoreSupport/WebPlatformStrategies.h:

LayoutTests:

Reviewed by Darin Adler.

* http/tests/cache/disk-cache/disk-cache-vary-cookie-expected.txt:
* http/tests/cache/disk-cache/disk-cache-vary-cookie.html:

Exapand the existing test to cover memory cache and private browsing.

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

18 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/cache/disk-cache/disk-cache-vary-cookie-expected.txt
LayoutTests/http/tests/cache/disk-cache/disk-cache-vary-cookie.html
Source/WebCore/ChangeLog
Source/WebCore/platform/CookiesStrategy.h
Source/WebCore/platform/network/CacheValidation.cpp
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebCoreSupport/WebFrameNetworkingContext.h
Source/WebKit/mac/WebCoreSupport/WebFrameNetworkingContext.mm
Source/WebKit/mac/WebCoreSupport/WebPlatformStrategies.h
Source/WebKit/mac/WebCoreSupport/WebPlatformStrategies.mm
Source/WebKit/win/WebCoreSupport/WebFrameNetworkingContext.cpp
Source/WebKit/win/WebCoreSupport/WebFrameNetworkingContext.h
Source/WebKit/win/WebCoreSupport/WebPlatformStrategies.cpp
Source/WebKit/win/WebCoreSupport/WebPlatformStrategies.h
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp
Source/WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.h

index d01c13e..8082160 100644 (file)
@@ -1,3 +1,15 @@
+2016-06-15  Antti Koivisto  <antti@apple.com>
+
+        Vary:Cookie validation doesn't work in private browsing
+        https://bugs.webkit.org/show_bug.cgi?id=158616
+
+        Reviewed by Darin Adler.
+
+        * http/tests/cache/disk-cache/disk-cache-vary-cookie-expected.txt:
+        * http/tests/cache/disk-cache/disk-cache-vary-cookie.html:
+
+        Exapand the existing test to cover memory cache and private browsing.
+
 2016-06-14  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Honor bidi unicode codepoints
index 8ee540c..d53649c 100644 (file)
@@ -1,8 +1,9 @@
-Test that Vary: Cookie in response is handled by the disk cache.
+Test that Vary: Cookie in response is handled by caches.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
+Testing disk cache
 Setting cookie and loading
 response headers: {"Cache-control":"max-age=100"}
 response source: Network
@@ -31,6 +32,64 @@ response source: Disk cache
 response headers: {"Vary":"Cookie","Cache-control":"max-age=100"}
 response source: Disk cache
 
+Testing memory cache
+Setting cookie and loading
+response headers: {"Cache-control":"max-age=100"}
+response source: Network
+
+response headers: {"Vary":"Cookie","Cache-control":"max-age=100"}
+response source: Network
+
+Loading again
+response headers: {"Cache-control":"max-age=100"}
+response source: Memory cache
+
+response headers: {"Vary":"Cookie","Cache-control":"max-age=100"}
+response source: Memory cache
+
+Changing cookie and loading
+response headers: {"Cache-control":"max-age=100"}
+response source: Memory cache
+
+response headers: {"Vary":"Cookie","Cache-control":"max-age=100"}
+response source: Network
+
+Loading again
+response headers: {"Cache-control":"max-age=100"}
+response source: Memory cache
+
+response headers: {"Vary":"Cookie","Cache-control":"max-age=100"}
+response source: Memory cache
+
+Testing memory cache in private browsing
+Setting cookie and loading
+response headers: {"Cache-control":"max-age=100"}
+response source: Network
+
+response headers: {"Vary":"Cookie","Cache-control":"max-age=100"}
+response source: Network
+
+Loading again
+response headers: {"Cache-control":"max-age=100"}
+response source: Memory cache
+
+response headers: {"Vary":"Cookie","Cache-control":"max-age=100"}
+response source: Memory cache
+
+Changing cookie and loading
+response headers: {"Cache-control":"max-age=100"}
+response source: Memory cache
+
+response headers: {"Vary":"Cookie","Cache-control":"max-age=100"}
+response source: Network
+
+Loading again
+response headers: {"Cache-control":"max-age=100"}
+response source: Memory cache
+
+response headers: {"Vary":"Cookie","Cache-control":"max-age=100"}
+response source: Memory cache
+
 PASS successfullyParsed is true
 
 TEST COMPLETE
index b9c72ea..4da9fe4 100644 (file)
@@ -3,32 +3,48 @@
 <body>
 <script>
 
-var tests =
-[
-  { responseHeaders: {'Cache-control': 'max-age=100'} },
-  { responseHeaders: {'Vary': 'Cookie', 'Cache-control': 'max-age=100'} },
-];
 
-description("Test that Vary: Cookie in response is handled by the disk cache.");
+description("Test that Vary: Cookie in response is handled by caches.");
 
-debug("Setting cookie and loading");
-document.cookie = "cookie=value";
-loadResources(tests, function () {
-    printResults(tests);
-    debug("Loading again");
-    loadResources(tests, function () {
+function testCookies(testDiskCache, completionHandler)
+{
+    var tests = [
+      { responseHeaders: {'Cache-control': 'max-age=100'} },
+      { responseHeaders: {'Vary': 'Cookie', 'Cache-control': 'max-age=100'} },
+    ];
+
+    var options = { "ClearMemoryCache" : testDiskCache };
+    debug("Setting cookie and loading");
+    document.cookie = "cookie=" + Math.floor((Math.random() * 1000000000000));
+    loadResourcesWithOptions(tests, options, function () {
         printResults(tests);
-        debug("Changing cookie and loading");
-        document.cookie = "cookie=othervalue";
-        loadResources(tests, function () {
+        debug("Loading again");
+        loadResourcesWithOptions(tests, options, function () {
             printResults(tests);
-            debug("Loading again");
-            loadResources(tests, function () {
-               printResults(tests);
-               finishJSTest();
+            debug("Changing cookie and loading");
+            document.cookie = "cookie" + Math.floor((Math.random() * 1000000000000));
+            loadResourcesWithOptions(tests, options, function () {
+                printResults(tests);
+                debug("Loading again");
+                loadResourcesWithOptions(tests, options, function () {
+                   printResults(tests);
+                   completionHandler();
+                });
             });
         });
     });
+}
+
+debug("Testing disk cache");
+testCookies(true, function () {
+    debug("Testing memory cache");
+    testCookies(false, function () {
+        debug("Testing memory cache in private browsing");
+        testRunner.setPrivateBrowsingEnabled(true);
+        testCookies(false, function () {
+            finishJSTest();
+        });
+    });
 });
 
 </script>
index 74303f6..e697f3c 100644 (file)
@@ -1,3 +1,24 @@
+2016-06-15  Antti Koivisto  <antti@apple.com>
+
+        Vary:Cookie validation doesn't work in private browsing
+        https://bugs.webkit.org/show_bug.cgi?id=158616
+        <rdar://problem/26755067>
+
+        Reviewed by Andreas Kling.
+
+        There wasn't a way to get cookie based on SessionID from WebCore.
+
+        * platform/CookiesStrategy.h:
+
+            Add a cookie retrival function that takes SessionID instead of NetworkStorageSession.
+
+        * platform/network/CacheValidation.cpp:
+        (WebCore::headerValueForVary):
+
+            Use it.
+
+        (WebCore::verifyVaryingRequestHeaders):
+
 2016-06-15  Per Arne Vollan  <pvollan@apple.com>
 
         [Win] The test accessibility/selected-text-range-aria-elements.html is failing.
index 9c7e6fb..b51bad0 100644 (file)
@@ -26,6 +26,7 @@
 #ifndef CookiesStrategy_h
 #define CookiesStrategy_h
 
+#include "SessionID.h"
 #include <wtf/HashSet.h>
 #include <wtf/RetainPtr.h>
 #include <wtf/Vector.h>
@@ -43,6 +44,7 @@ public:
     virtual void setCookiesFromDOM(const NetworkStorageSession&, const URL& firstParty, const URL&, const String& cookieString) = 0;
     virtual bool cookiesEnabled(const NetworkStorageSession&, const URL& firstParty, const URL&) = 0;
     virtual String cookieRequestHeaderFieldValue(const NetworkStorageSession&, const URL& firstParty, const URL&) = 0;
+    virtual String cookieRequestHeaderFieldValue(SessionID, const URL& firstParty, const URL&) = 0;
     virtual bool getRawCookies(const NetworkStorageSession&, const URL& firstParty, const URL&, Vector<Cookie>&) = 0;
     virtual void deleteCookie(const NetworkStorageSession&, const URL&, const String& cookieName) = 0;
     virtual void addCookie(const NetworkStorageSession&, const URL&, const Cookie&) = 0;
index 07ee84a..0710e42 100644 (file)
@@ -338,15 +338,12 @@ static String headerValueForVary(const ResourceRequest& request, const String& h
     // We could fetch the cookie when making the request but that seems overkill as the case is very rare and it
     // is a blocking operation. This should be sufficient to cover reasonable cases.
     if (headerName == httpHeaderNameString(HTTPHeaderName::Cookie)) {
-        if (sessionID != SessionID::defaultSessionID()) {
-            // FIXME: Don't know how to get the cookie. There should be a global way to get NetworkStorageSession from sessionID.
-            return "";
-        }
-        auto& session = NetworkStorageSession::defaultStorageSession();
         auto* cookieStrategy = platformStrategies() ? platformStrategies()->cookiesStrategy() : nullptr;
-        if (!cookieStrategy)
-            return cookieRequestHeaderFieldValue(session, request.firstPartyForCookies(), request.url());
-        return cookieStrategy->cookieRequestHeaderFieldValue(session, request.firstPartyForCookies(), request.url());
+        if (!cookieStrategy) {
+            ASSERT(sessionID == SessionID::defaultSessionID());
+            return cookieRequestHeaderFieldValue(NetworkStorageSession::defaultStorageSession(), request.firstPartyForCookies(), request.url());
+        }
+        return cookieStrategy->cookieRequestHeaderFieldValue(sessionID, request.firstPartyForCookies(), request.url());
     }
     return request.httpHeaderField(headerName);
 }
@@ -374,10 +371,6 @@ bool verifyVaryingRequestHeaders(const Vector<std::pair<String, String>>& varyin
         // FIXME: Vary: * in response would ideally trigger a cache delete instead of a store.
         if (varyingRequestHeader.first == "*")
             return false;
-        if (sessionID != SessionID::defaultSessionID() && varyingRequestHeader.first == httpHeaderNameString(HTTPHeaderName::Cookie)) {
-            // FIXME: See the comment in headerValueForVary.
-            return false;
-        }
         String headerValue = headerValueForVary(request, varyingRequestHeader.first, sessionID);
         if (headerValue != varyingRequestHeader.second)
             return false;
index 3c52354..a69079a 100644 (file)
@@ -1,3 +1,28 @@
+2016-06-15  Antti Koivisto  <antti@apple.com>
+
+        Vary:Cookie validation doesn't work in private browsing
+        https://bugs.webkit.org/show_bug.cgi?id=158616
+        <rdar://problem/26755067>
+
+        Reviewed by Andreas Kling.
+
+        * WebCoreSupport/WebFrameNetworkingContext.h:
+        (WebFrameNetworkingContext::create):
+        * WebCoreSupport/WebFrameNetworkingContext.mm:
+        (privateSession):
+        (WebFrameNetworkingContext::ensurePrivateBrowsingSession):
+
+            Expose the private browsing session.
+
+        (WebFrameNetworkingContext::destroyPrivateBrowsingSession):
+        * WebCoreSupport/WebPlatformStrategies.h:
+        * WebCoreSupport/WebPlatformStrategies.mm:
+        (WebPlatformStrategies::cookieRequestHeaderFieldValue):
+
+            Implement SessionID version of the function.
+
+        (WebPlatformStrategies::getRawCookies):
+
 2016-06-13  Alex Christensen  <achristensen@webkit.org>
 
         Add WebSocketProvider stub
index 9b3c309..b08d56e 100644 (file)
@@ -35,7 +35,7 @@ public:
         return adoptRef(new WebFrameNetworkingContext(frame));
     }
 
-    static void ensurePrivateBrowsingSession();
+    static WebCore::NetworkStorageSession& ensurePrivateBrowsingSession();
     static void destroyPrivateBrowsingSession();
 
 private:
index c8b971c..70a6c59 100644 (file)
@@ -50,14 +50,16 @@ static std::unique_ptr<NetworkStorageSession>& privateSession()
     return session;
 }
 
-void WebFrameNetworkingContext::ensurePrivateBrowsingSession()
+NetworkStorageSession& WebFrameNetworkingContext::ensurePrivateBrowsingSession()
 {
     ASSERT(isMainThread());
 
     if (privateSession())
-        return;
+        return *privateSession();
 
     privateSession() = NetworkStorageSession::createPrivateBrowsingSession(SessionID::legacyPrivateSessionID(), [[NSBundle mainBundle] bundleIdentifier]);
+
+    return *privateSession();
 }
 
 void WebFrameNetworkingContext::destroyPrivateBrowsingSession()
index 307a079..b345dda 100644 (file)
@@ -54,6 +54,7 @@ private:
     void setCookiesFromDOM(const WebCore::NetworkStorageSession&, const WebCore::URL& firstParty, const WebCore::URL&, const String&) override;
     bool cookiesEnabled(const WebCore::NetworkStorageSession&, const WebCore::URL& firstParty, const WebCore::URL&) override;
     String cookieRequestHeaderFieldValue(const WebCore::NetworkStorageSession&, const WebCore::URL& firstParty, const WebCore::URL&) override;
+    String cookieRequestHeaderFieldValue(WebCore::SessionID, const WebCore::URL& firstParty, const WebCore::URL&) override;
     bool getRawCookies(const WebCore::NetworkStorageSession&, const WebCore::URL& firstParty, const WebCore::URL&, Vector<WebCore::Cookie>&) override;
     void deleteCookie(const WebCore::NetworkStorageSession&, const WebCore::URL&, const String&) override;
     void addCookie(const WebCore::NetworkStorageSession&, const WebCore::URL&, const WebCore::Cookie&) override;
index ab0695f..ceebf37 100644 (file)
@@ -102,6 +102,12 @@ String WebPlatformStrategies::cookieRequestHeaderFieldValue(const NetworkStorage
     return WebCore::cookieRequestHeaderFieldValue(session, firstParty, url);
 }
 
+String WebPlatformStrategies::cookieRequestHeaderFieldValue(SessionID sessionID, const URL& firstParty, const URL& url)
+{
+    auto& session = sessionID.isEphemeral() ? WebFrameNetworkingContext::ensurePrivateBrowsingSession() : NetworkStorageSession::defaultStorageSession();
+    return WebCore::cookieRequestHeaderFieldValue(session, firstParty, url);
+}
+
 bool WebPlatformStrategies::getRawCookies(const NetworkStorageSession& session, const URL& firstParty, const URL& url, Vector<Cookie>& rawCookies)
 {
     return WebCore::getRawCookies(session, firstParty, url, rawCookies);
index 29a6adc..737d431 100644 (file)
@@ -71,13 +71,13 @@ void WebFrameNetworkingContext::setPrivateBrowsingStorageSessionIdentifierBase(c
     identifierBase() = base;
 }
 
-void WebFrameNetworkingContext::ensurePrivateBrowsingSession()
+NetworkStorageSession& WebFrameNetworkingContext::ensurePrivateBrowsingSession()
 {
 #if USE(CFNETWORK)
     ASSERT(isMainThread());
 
     if (privateSession())
-        return;
+        return *privateSession();
 
     String base;
     if (identifierBase().isNull()) {
@@ -88,7 +88,9 @@ void WebFrameNetworkingContext::ensurePrivateBrowsingSession()
         base = identifierBase();
 
     privateSession() = NetworkStorageSession::createPrivateBrowsingSession(SessionID::legacyPrivateSessionID(), base);
+
 #endif
+    return *privateSession();
 }
 
 void WebFrameNetworkingContext::destroyPrivateBrowsingSession()
index d935be8..59fe4e9 100644 (file)
@@ -38,7 +38,7 @@ public:
     static void setCookieAcceptPolicyForAllContexts(WebKitCookieStorageAcceptPolicy);
 #endif
     static void setPrivateBrowsingStorageSessionIdentifierBase(const String&);
-    static void ensurePrivateBrowsingSession();
+    static WebCore::NetworkStorageSession& ensurePrivateBrowsingSession();
     static void destroyPrivateBrowsingSession();
 
 private:
index 4a33da1..6b79f94 100644 (file)
@@ -94,6 +94,12 @@ String WebPlatformStrategies::cookieRequestHeaderFieldValue(const NetworkStorage
     return WebCore::cookieRequestHeaderFieldValue(session, firstParty, url);
 }
 
+String WebPlatformStrategies::cookieRequestHeaderFieldValue(WebCore::SessionID sessionID, const URL& firstParty, const URL& url)
+{
+    auto& session = sessionID.isEphemeral() ? WebFrameNetworkingContext::ensurePrivateBrowsingSession() : NetworkStorageSession::defaultStorageSession();
+    return WebCore::cookieRequestHeaderFieldValue(session, firstParty, url);
+}
+
 bool WebPlatformStrategies::getRawCookies(const NetworkStorageSession& session, const URL& firstParty, const URL& url, Vector<Cookie>& rawCookies)
 {
     return WebCore::getRawCookies(session, firstParty, url, rawCookies);
index 3637155..7727050 100644 (file)
@@ -50,6 +50,7 @@ private:
     virtual void setCookiesFromDOM(const WebCore::NetworkStorageSession&, const WebCore::URL& firstParty, const WebCore::URL&, const String&);
     virtual bool cookiesEnabled(const WebCore::NetworkStorageSession&, const WebCore::URL& firstParty, const WebCore::URL&);
     virtual String cookieRequestHeaderFieldValue(const WebCore::NetworkStorageSession&, const WebCore::URL& firstParty, const WebCore::URL&);
+    virtual String cookieRequestHeaderFieldValue(WebCore::SessionID, const WebCore::URL& firstParty, const WebCore::URL&);
     virtual bool getRawCookies(const WebCore::NetworkStorageSession&, const WebCore::URL& firstParty, const WebCore::URL&, Vector<WebCore::Cookie>&);
     virtual void deleteCookie(const WebCore::NetworkStorageSession&, const WebCore::URL&, const String&);
     virtual void addCookie(const WebCore::NetworkStorageSession&, const WebCore::URL&, const WebCore::Cookie&);
index cc3f5f3..64943ca 100644 (file)
@@ -1,3 +1,18 @@
+2016-06-15  Antti Koivisto  <antti@apple.com>
+
+        Vary:Cookie validation doesn't work in private browsing
+        https://bugs.webkit.org/show_bug.cgi?id=158616
+        <rdar://problem/26755067>
+
+        Reviewed by Andreas Kling.
+
+        * WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:
+        (WebKit::WebPlatformStrategies::cookieRequestHeaderFieldValue):
+
+            Implement SessionID version of the function.
+
+        * WebProcess/WebCoreSupport/WebPlatformStrategies.h:
+
 2016-06-14  Chris Dumez  <cdumez@apple.com>
 
         Avoid constructing a AuthenticationChallenge unnecessarily in the NetworkLoad constructor
index 023c3d1..d99ffc7 100644 (file)
@@ -137,8 +137,13 @@ bool WebPlatformStrategies::cookiesEnabled(const NetworkStorageSession& session,
 
 String WebPlatformStrategies::cookieRequestHeaderFieldValue(const NetworkStorageSession& session, const URL& firstParty, const URL& url)
 {
+    return cookieRequestHeaderFieldValue(SessionTracker::sessionID(session), firstParty, url);
+}
+
+String WebPlatformStrategies::cookieRequestHeaderFieldValue(SessionID sessionID, const URL& firstParty, const URL& url)
+{
     String result;
-    if (!WebProcess::singleton().networkConnection()->connection()->sendSync(Messages::NetworkConnectionToWebProcess::CookieRequestHeaderFieldValue(SessionTracker::sessionID(session), firstParty, url), Messages::NetworkConnectionToWebProcess::CookieRequestHeaderFieldValue::Reply(result), 0))
+    if (!WebProcess::singleton().networkConnection()->connection()->sendSync(Messages::NetworkConnectionToWebProcess::CookieRequestHeaderFieldValue(sessionID, firstParty, url), Messages::NetworkConnectionToWebProcess::CookieRequestHeaderFieldValue::Reply(result), 0))
         return String();
     return result;
 }
index 7b81c63..398ea03 100644 (file)
@@ -56,6 +56,7 @@ private:
     void setCookiesFromDOM(const WebCore::NetworkStorageSession&, const WebCore::URL& firstParty, const WebCore::URL&, const String&) override;
     bool cookiesEnabled(const WebCore::NetworkStorageSession&, const WebCore::URL& firstParty, const WebCore::URL&) override;
     String cookieRequestHeaderFieldValue(const WebCore::NetworkStorageSession&, const WebCore::URL& firstParty, const WebCore::URL&) override;
+    String cookieRequestHeaderFieldValue(WebCore::SessionID, const WebCore::URL& firstParty, const WebCore::URL&) override;
     bool getRawCookies(const WebCore::NetworkStorageSession&, const WebCore::URL& firstParty, const WebCore::URL&, Vector<WebCore::Cookie>&) override;
     void deleteCookie(const WebCore::NetworkStorageSession&, const WebCore::URL&, const String&) override;
     void addCookie(const WebCore::NetworkStorageSession&, const WebCore::URL&, const WebCore::Cookie&) override;