<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[182157] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/182157">182157</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2015-03-30 15:47:05 -0700 (Mon, 30 Mar 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Cached "Expires" header is not updated upon successful resource revalidation
https://bugs.webkit.org/show_bug.cgi?id=143228
<rdar://problem/20348059>
Reviewed by Antti Koivisto.
Source/WebCore:
Cached "Expires" header was not updated upon successful resource
revalidation. This affected both our disk cache and our memory cache.
This was caused by shouldUpdateHeaderAfterRevalidation() in
CacheValidation.cpp returning false for the "Expires" header.
There is a comment there stating that the list of ignored headers
matches Chromium's net library but that's not the case, at least not
anymore:
http://osxr.org/android/source/external/chromium/net/http/http_response_headers.cc
HTTP servers such as Apache return an "Expires" header in their 304
responses and the "Expires" header is potentially a new one. However,
our caches were ignoring the updated expiration date and kept using the
old one, which meant that the cached resource expired sooner than it
should have.
See the following Apache bugs that explain the issue:
https://bz.apache.org/bugzilla/show_bug.cgi?id=24884
https://bz.apache.org/bugzilla/show_bug.cgi?id=25123
Test: http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html
* platform/network/CacheValidation.cpp:
LayoutTests:
Add layout test to check that a cached response's "Expires" header is
updated from the 304 response's headers upon successful revalidation.
* http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header-expected.txt: Added.
* http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html: Added.
* http/tests/cache/disk-cache/resources/cache-test.js:
(generateTestURL):
(loadResource):
* http/tests/cache/disk-cache/resources/generate-response.cgi:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestshttptestscachediskcacheresourcescachetestjs">trunk/LayoutTests/http/tests/cache/disk-cache/resources/cache-test.js</a></li>
<li><a href="#trunkLayoutTestshttptestscachediskcacheresourcesgenerateresponsecgi">trunk/LayoutTests/http/tests/cache/disk-cache/resources/generate-response.cgi</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformnetworkCacheValidationcpp">trunk/Source/WebCore/platform/network/CacheValidation.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestshttptestscachediskcachediskcacherevalidationnewexpireheaderexpectedtxt">trunk/LayoutTests/http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header-expected.txt</a></li>
<li><a href="#trunkLayoutTestshttptestscachediskcachediskcacherevalidationnewexpireheaderhtml">trunk/LayoutTests/http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (182156 => 182157)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-03-30 22:43:26 UTC (rev 182156)
+++ trunk/LayoutTests/ChangeLog 2015-03-30 22:47:05 UTC (rev 182157)
</span><span class="lines">@@ -1,3 +1,21 @@
</span><ins>+2015-03-30 Chris Dumez <cdumez@apple.com>
+
+ Cached "Expires" header is not updated upon successful resource revalidation
+ https://bugs.webkit.org/show_bug.cgi?id=143228
+ <rdar://problem/20348059>
+
+ Reviewed by Antti Koivisto.
+
+ Add layout test to check that a cached response's "Expires" header is
+ updated from the 304 response's headers upon successful revalidation.
+
+ * http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header-expected.txt: Added.
+ * http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html: Added.
+ * http/tests/cache/disk-cache/resources/cache-test.js:
+ (generateTestURL):
+ (loadResource):
+ * http/tests/cache/disk-cache/resources/generate-response.cgi:
+
</ins><span class="cx"> 2015-03-30 Joseph Pecoraro <pecoraro@apple.com>
</span><span class="cx">
</span><span class="cx"> Web Inspector: Regression: Preview for [[null]] shouldn't be []
</span></span></pre></div>
<a id="trunkLayoutTestshttptestscachediskcachediskcacherevalidationnewexpireheaderexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header-expected.txt (0 => 182157)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header-expected.txt 2015-03-30 22:47:05 UTC (rev 182157)
</span><span class="lines">@@ -0,0 +1,19 @@
</span><ins>+Test that the 'Expires' header is updated upon successful validation.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+
+response headers: {"Expires":"now(0)","ETag":"match"}
+response's 'Expires' header is overriden by future date in 304 response
+response source: Disk cache after validation
+
+304 response included an 'Expires' header in the future, so we should not need to revalidate this time.
+response headers: {"Expires":"now(0)","ETag":"match"}
+response's 'Expires' header is overriden by future date in 304 response
+response source: Disk cache
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestshttptestscachediskcachediskcacherevalidationnewexpireheaderhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html (0 => 182157)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html (rev 0)
+++ trunk/LayoutTests/http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html 2015-03-30 22:47:05 UTC (rev 182157)
</span><span class="lines">@@ -0,0 +1,21 @@
</span><ins>+<script src="/js-test-resources/js-test-pre.js"></script>
+<script src="resources/cache-test.js"></script>
+<body>
+<script>
+
+var tests =
+[
+ { responseHeaders: {'Expires': 'now(0)', 'ETag': 'match' }, expiresInFutureIn304: true },
+];
+
+description("Test that the 'Expires' header is updated upon successful validation.");
+
+debug("");
+
+runTests(tests, function() {
+ debug("304 response included an 'Expires' header in the future, so we should not need to revalidate this time.");
+ runTests(tests);
+});
+
+</script>
+<script src="/js-test-resources/js-test-post.js"></script>
</ins></span></pre></div>
<a id="trunkLayoutTestshttptestscachediskcacheresourcescachetestjs"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/http/tests/cache/disk-cache/resources/cache-test.js (182156 => 182157)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/cache/disk-cache/resources/cache-test.js 2015-03-30 22:43:26 UTC (rev 182156)
+++ trunk/LayoutTests/http/tests/cache/disk-cache/resources/cache-test.js 2015-03-30 22:47:05 UTC (rev 182157)
</span><span class="lines">@@ -38,12 +38,15 @@
</span><span class="cx"> return value;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-function generateTestURL(test, includeBody)
</del><ins>+function generateTestURL(test, includeBody, expiresInFutureIn304)
</ins><span class="cx"> {
</span><span class="cx"> includeBody = typeof includeBody !== 'undefined' ? includeBody : true;
</span><ins>+ expiresInFutureIn304 = typeof expiresInFutureIn304 !== 'undefined' ? expiresInFutureIn304 : false;
</ins><span class="cx"> var uniqueTestId = Math.floor((Math.random() * 1000000000000));
</span><del>- var cgi_script = "resources/generate-response.cgi?include-body=" + (includeBody ? "1" : "0");
- var testURL = cgi_script + "&uniqueId=" + uniqueTestId++ + "&Content-type=text/plain";
</del><ins>+ var testURL = "resources/generate-response.cgi?include-body=" + (includeBody ? "1" : "0");
+ if (expiresInFutureIn304)
+ testURL += "&expires-in-future-in-304=1";
+ testURL += "&uniqueId=" + uniqueTestId++ + "&Content-type=text/plain";
</ins><span class="cx"> for (var header in test.responseHeaders)
</span><span class="cx"> testURL += '&' + header + '=' + makeHeaderValue(test.responseHeaders[header]);
</span><span class="cx"> return testURL;
</span><span class="lines">@@ -52,7 +55,7 @@
</span><span class="cx"> function loadResource(test, onload)
</span><span class="cx"> {
</span><span class="cx"> if (!test.url)
</span><del>- test.url = generateTestURL(test, test.includeBody);
</del><ins>+ test.url = generateTestURL(test, test.includeBody, test.expiresInFutureIn304);
</ins><span class="cx">
</span><span class="cx"> test.xhr = new XMLHttpRequest();
</span><span class="cx"> test.xhr.onload = onload;
</span><span class="lines">@@ -81,6 +84,8 @@
</span><span class="cx"> for (var i = 0; i < tests.length; ++i) {
</span><span class="cx"> var test = tests[i];
</span><span class="cx"> debug("response headers: " + JSON.stringify(test.responseHeaders));
</span><ins>+ if (test.expiresInFutureIn304)
+ debug("response's 'Expires' header is overriden by future date in 304 response");
</ins><span class="cx"> if (test.requestHeaders)
</span><span class="cx"> debug("request headers: " + JSON.stringify(test.requestHeaders));
</span><span class="cx"> responseSource = internals.xhrResponseSource(test.xhr);
</span></span></pre></div>
<a id="trunkLayoutTestshttptestscachediskcacheresourcesgenerateresponsecgi"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/http/tests/cache/disk-cache/resources/generate-response.cgi (182156 => 182157)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/cache/disk-cache/resources/generate-response.cgi 2015-03-30 22:43:26 UTC (rev 182156)
+++ trunk/LayoutTests/http/tests/cache/disk-cache/resources/generate-response.cgi 2015-03-30 22:47:05 UTC (rev 182157)
</span><span class="lines">@@ -6,17 +6,24 @@
</span><span class="cx"> my $query = new CGI;
</span><span class="cx"> @names = $query->param;
</span><span class="cx"> my $includeBody = $query->param('include-body') || 0;
</span><ins>+my $expiresInFutureIn304 = $query->param('expires-in-future-in-304') || 0;
</ins><span class="cx">
</span><span class="cx"> my $hasStatusCode = 0;
</span><ins>+my $hasExpiresHeader = 0;
</ins><span class="cx"> if ($query->http && $query->http("If-None-Match") eq "match") {
</span><span class="cx"> print "Status: 304\n";
</span><span class="cx"> $hasStatusCode = 1;
</span><ins>+ if ($expiresInFutureIn304) {
+ print "Expires: " . HTTP::Date::time2str(time() + 100) . "\n";
+ $hasExpiresHeader = 1;
+ }
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> foreach (@names) {
</span><span class="cx"> next if ($_ eq "uniqueId");
</span><span class="cx"> next if ($_ eq "include-body");
</span><span class="cx"> next if ($_ eq "Status" and $hasStatusCode);
</span><ins>+ next if ($_ eq "Expires" and $hasExpiresHeader);
</ins><span class="cx"> print $_ . ": " . $query->param($_) . "\n";
</span><span class="cx"> }
</span><span class="cx"> print "\n";
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (182156 => 182157)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-03-30 22:43:26 UTC (rev 182156)
+++ trunk/Source/WebCore/ChangeLog 2015-03-30 22:47:05 UTC (rev 182157)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+2015-03-30 Chris Dumez <cdumez@apple.com>
+
+ Cached "Expires" header is not updated upon successful resource revalidation
+ https://bugs.webkit.org/show_bug.cgi?id=143228
+ <rdar://problem/20348059>
+
+ Reviewed by Antti Koivisto.
+
+ Cached "Expires" header was not updated upon successful resource
+ revalidation. This affected both our disk cache and our memory cache.
+ This was caused by shouldUpdateHeaderAfterRevalidation() in
+ CacheValidation.cpp returning false for the "Expires" header.
+
+ There is a comment there stating that the list of ignored headers
+ matches Chromium's net library but that's not the case, at least not
+ anymore:
+ http://osxr.org/android/source/external/chromium/net/http/http_response_headers.cc
+
+ HTTP servers such as Apache return an "Expires" header in their 304
+ responses and the "Expires" header is potentially a new one. However,
+ our caches were ignoring the updated expiration date and kept using the
+ old one, which meant that the cached resource expired sooner than it
+ should have.
+
+ See the following Apache bugs that explain the issue:
+ https://bz.apache.org/bugzilla/show_bug.cgi?id=24884
+ https://bz.apache.org/bugzilla/show_bug.cgi?id=25123
+
+ Test: http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html
+
+ * platform/network/CacheValidation.cpp:
+
</ins><span class="cx"> 2015-03-30 Antti Koivisto <antti@apple.com>
</span><span class="cx">
</span><span class="cx"> Don't cache resources that are very unlikely to be reused
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformnetworkCacheValidationcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/network/CacheValidation.cpp (182156 => 182157)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/network/CacheValidation.cpp 2015-03-30 22:43:26 UTC (rev 182156)
+++ trunk/Source/WebCore/platform/network/CacheValidation.cpp 2015-03-30 22:47:05 UTC (rev 182157)
</span><span class="lines">@@ -39,7 +39,6 @@
</span><span class="cx"> "allow",
</span><span class="cx"> "connection",
</span><span class="cx"> "etag",
</span><del>- "expires",
</del><span class="cx"> "keep-alive",
</span><span class="cx"> "last-modified"
</span><span class="cx"> "proxy-authenticate",
</span></span></pre>
</div>
</div>
</body>
</html>