- Revision
- 181651
- Author
- [email protected]
- Date
- 2015-03-17 09:09:06 -0700 (Tue, 17 Mar 2015)
Log 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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (181650 => 181651)
--- trunk/LayoutTests/ChangeLog 2015-03-17 15:50:44 UTC (rev 181650)
+++ trunk/LayoutTests/ChangeLog 2015-03-17 16:09:06 UTC (rev 181651)
@@ -1,3 +1,13 @@
+2015-03-17 Antti Koivisto <[email protected]>
+
+ 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 <[email protected]>
Enable ES6 classes by default
Added: trunk/LayoutTests/http/tests/cache/disk-cache-vary-cookie-expected.txt (0 => 181651)
--- trunk/LayoutTests/http/tests/cache/disk-cache-vary-cookie-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/cache/disk-cache-vary-cookie-expected.txt 2015-03-17 16:09:06 UTC (rev 181651)
@@ -0,0 +1,37 @@
+Test that Vary: Cookie in response is handled by the disk cache.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+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: Disk cache
+
+response headers: {"Vary":"Cookie","Cache-control":"max-age=100"}
+response source: Disk cache
+
+Changing cookie and loading
+response headers: {"Cache-control":"max-age=100"}
+response source: Disk cache
+
+response headers: {"Vary":"Cookie","Cache-control":"max-age=100"}
+response source: Network
+
+Loading again
+response headers: {"Cache-control":"max-age=100"}
+response source: Disk cache
+
+response headers: {"Vary":"Cookie","Cache-control":"max-age=100"}
+response source: Disk cache
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/http/tests/cache/disk-cache-vary-cookie.html (0 => 181651)
--- trunk/LayoutTests/http/tests/cache/disk-cache-vary-cookie.html (rev 0)
+++ trunk/LayoutTests/http/tests/cache/disk-cache-vary-cookie.html 2015-03-17 16:09:06 UTC (rev 181651)
@@ -0,0 +1,38 @@
+<script src=""
+<script src=""
+<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.");
+
+debug("Setting cookie and loading");
+document.cookie = "cookie=value";
+loadResources(tests, function () {
+ printResults(tests);
+ internals.clearMemoryCache();
+ debug("Loading again");
+ loadResources(tests, function () {
+ printResults(tests);
+ internals.clearMemoryCache();
+ debug("Changing cookie and loading");
+ document.cookie = "cookie=othervalue";
+ loadResources(tests, function () {
+ printResults(tests);
+ internals.clearMemoryCache()
+ debug("Loading again");
+ loadResources(tests, function () {
+ printResults(tests);
+ finishJSTest();
+ });
+ });
+ });
+});
+
+</script>
+<script src=""
Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (181650 => 181651)
--- trunk/LayoutTests/platform/mac-wk1/TestExpectations 2015-03-17 15:50:44 UTC (rev 181650)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations 2015-03-17 16:09:06 UTC (rev 181651)
@@ -94,6 +94,7 @@
# Disk cache is WK2 only
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
########################################
Modified: trunk/LayoutTests/platform/win/TestExpectations (181650 => 181651)
--- trunk/LayoutTests/platform/win/TestExpectations 2015-03-17 15:50:44 UTC (rev 181650)
+++ trunk/LayoutTests/platform/win/TestExpectations 2015-03-17 16:09:06 UTC (rev 181651)
@@ -2222,6 +2222,7 @@
# Disk cache is WK2 only
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:
http/tests/cache/content-type-ignored-during-revalidation.html [ Failure ]
Modified: trunk/Source/WebKit2/ChangeLog (181650 => 181651)
--- trunk/Source/WebKit2/ChangeLog 2015-03-17 15:50:44 UTC (rev 181650)
+++ trunk/Source/WebKit2/ChangeLog 2015-03-17 16:09:06 UTC (rev 181651)
@@ -1,3 +1,23 @@
+2015-03-17 Antti Koivisto <[email protected]>
+
+ 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 <[email protected]>
[WK2] Use C++ lambdas in IPC::Connection
Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp (181650 => 181651)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp 2015-03-17 15:50:44 UTC (rev 181650)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp 2015-03-17 16:09:06 UTC (rev 181651)
@@ -38,6 +38,8 @@
#include <WebCore/CacheValidation.h>
#include <WebCore/FileSystem.h>
#include <WebCore/HTTPHeaderNames.h>
+#include <WebCore/NetworkStorageSession.h>
+#include <WebCore/PlatformCookieJar.h>
#include <WebCore/ResourceResponse.h>
#include <WebCore/SharedBuffer.h>
#include <wtf/NeverDestroyed.h>
@@ -97,6 +99,17 @@
return { request.httpMethod(), partition, request.url().string() };
}
+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)
{
Encoder encoder;
@@ -114,7 +127,8 @@
Vector<std::pair<String, String>> varyingRequestHeaders;
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;
}
@@ -135,8 +149,8 @@
// FIXME: Vary: * in response would ideally trigger a cache delete instead of a store.
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;
}
return true;
