Changeset 181651 in webkit

Timestamp:

Mar 17, 2015 9:09:06 AM (9 years ago)

Author:

Antti Koivisto

Message:

Disk cache should support Vary: Cookie
​https://bugs.webkit.org/show_bug.cgi?id=142770
Source/WebKit2:

rdar://problem/19764945

Reviewed by Anders Carlsson.

Cookies are not part of the original request but are added by the networking layer when submitting the request.
Fetch them explicitly when resolving Vary: Cookie.

The implementation is not perfect as it fetches the cookie for the cache entry when saving a Vary:Cookie response,
not when making the request. In principle the cookie may have changed in-between. This should be enough to handle
reasonable cases though. Fetching cookies for every request might be too expensive for this rarely used feature.

• NetworkProcess/cache/NetworkCache.cpp:

(WebKit::NetworkCache::headerValueForVary):
(WebKit::NetworkCache::encodeStorageEntry):
(WebKit::NetworkCache::verifyVaryingRequestHeaders):

LayoutTests:

Reviewed by Anders Carlsson.

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

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/cache/disk-cache-vary-cookie-expected.txt (added)
• LayoutTests/http/tests/cache/disk-cache-vary-cookie.html (added)
• LayoutTests/platform/mac-wk1/TestExpectations (modified) (1 diff)
• LayoutTests/platform/win/TestExpectations (modified) (1 diff)
• Source/WebKit2/ChangeLog (modified) (1 diff)
• Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2015-03-17  Antti Koivisto  <antti@apple.com>
+
+        Disk cache should support Vary: Cookie
+        https://bugs.webkit.org/show_bug.cgi?id=142770
+
+        Reviewed by Anders Carlsson.
+
+        * http/tests/cache/disk-cache-vary-cookie-expected.txt: Added.
+        * http/tests/cache/disk-cache-vary-cookie.html: Added.
+
 2015-03-16  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/LayoutTests/platform/mac-wk1/TestExpectations

@@ -95 +95 @@
 http/tests/cache/disk-cache-validation.html
 http/tests/cache/disk-cache-disable.html
+http/tests/cache/disk-cache-vary-cookie.html
 
 ### END OF (2) Failures without bug reports

trunk/LayoutTests/platform/win/TestExpectations

@@ -2223 +2223 @@
 http/tests/cache/disk-cache-validation.html
 http/tests/cache/disk-cache-disable.html
+http/tests/cache/disk-cache-vary-cookie.html
 
 # The following are unreviewed:

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2015-03-17  Antti Koivisto  <antti@apple.com>
+
+        Disk cache should support Vary: Cookie
+        https://bugs.webkit.org/show_bug.cgi?id=142770
+        rdar://problem/19764945
+
+        Reviewed by Anders Carlsson.
+
+        Cookies are not part of the original request but are added by the networking layer when submitting the request.
+        Fetch them explicitly when resolving Vary: Cookie.
+
+        The implementation is not perfect as it fetches the cookie for the cache entry when saving a Vary:Cookie response,
+        not when making the request. In principle the cookie may have changed in-between. This should be enough to handle
+        reasonable cases though. Fetching cookies for every request might be too expensive for this rarely used feature.
+
+        * NetworkProcess/cache/NetworkCache.cpp:
+        (WebKit::NetworkCache::headerValueForVary):
+        (WebKit::NetworkCache::encodeStorageEntry):
+        (WebKit::NetworkCache::verifyVaryingRequestHeaders):
+
 2015-03-17  Zan Dobersek  <zdobersek@igalia.com>
 

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp

@@ -39 +39 @@
 #include <WebCore/FileSystem.h>
 #include <WebCore/HTTPHeaderNames.h>
+#include <WebCore/NetworkStorageSession.h>
+#include <WebCore/PlatformCookieJar.h>
 #include <WebCore/ResourceResponse.h>
 #include <WebCore/SharedBuffer.h>
@@ -98 +100 @@
 }
 
+static String headerValueForVary(const WebCore::ResourceRequest& request, const String& headerName)
+{
+    // Explicit handling for cookies is needed because they are added magically by the networking layer.
+    // FIXME: The value might have changed between making the request and retrieving the cookie here.
+    // 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(WebCore::HTTPHeaderName::Cookie))
+        return WebCore::cookieRequestHeaderFieldValue(WebCore::NetworkStorageSession::defaultStorageSession(), request.firstPartyForCookies(), request.url());
+    return request.httpHeaderField(headerName);
+}
+
 static Storage::Entry encodeStorageEntry(const WebCore::ResourceRequest& request, const WebCore::ResourceResponse& response, PassRefPtr<WebCore::SharedBuffer> responseData)
 {
@@ -115 +128 @@
         for (auto& varyHeaderName : varyingHeaderNames) {
             String headerName = varyHeaderName.stripWhiteSpace();
-            varyingRequestHeaders.append(std::make_pair(headerName, request.httpHeaderField(headerName)));
+            String headerValue = headerValueForVary(request, headerName);
+            varyingRequestHeaders.append(std::make_pair(headerName, headerValue));
         }
         encoder << varyingRequestHeaders;
@@ -136 +150 @@
         if (varyingRequestHeader.first == "*")
             return false;
-        String requestValue = request.httpHeaderField(varyingRequestHeader.first);
-        if (requestValue != varyingRequestHeader.second)
+        String headerValue = headerValueForVary(request, varyingRequestHeader.first);
+        if (headerValue != varyingRequestHeader.second)
             return false;
     }