Bug 143228

Summary: Cached "Expires" header is not updated upon successful resource revalidation
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Page LoadingAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, commit-queue, koivisto, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Chris Dumez
Reported 2015-03-30 10:31:47 PDT
Cached "Expires" header is not updated upon successful resource revalidation. This affects both our disk cache and our memory cache. This is because we shouldUpdateHeaderAfterRevalidation() in CacheValidation.cpp returns false for the "Expires" header. HTTP servers such as Apache returns an "Expires" header in their 304 responses and the "Expires" header is potentially a new one. However, our caches ignore it and keep the old one, which means that the cached resource expires sooner than it should. 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 There is a comment in the code that says the list of 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
Attachments
Patch (8.35 KB, patch)
2015-03-30 14:25 PDT, Chris Dumez
no flags
Patch (8.99 KB, patch)
2015-03-30 15:29 PDT, Chris Dumez
no flags
Radar WebKit Bug Importer
Comment 1 2015-03-30 10:32:38 PDT
Antti Koivisto
Comment 2 2015-03-30 11:15:16 PDT
Good catch
Chris Dumez
Comment 3 2015-03-30 14:25:42 PDT
Antti Koivisto
Comment 4 2015-03-30 15:15:17 PDT
Comment on attachment 249761 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249761&action=review r=me, nice. We should also add some explicit Expires header tests at some point. > LayoutTests/http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header-expected.txt:7 > +response headers: {"Expires":"now(0)","ETag":"match"} Would be nice if this printed the expiresInFutureIn304 flag too. The suggestion below would cover that automatically. > LayoutTests/http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html:8 > + { responseHeaders: {'Expires': 'now(0)', 'ETag': 'match' }, expiresInFutureIn304: true }, Another option would be to add another magical Expires value (nowButUpdateToFuture or something). Don't know if that is really any better.
Chris Dumez
Comment 5 2015-03-30 15:29:25 PDT
WebKit Commit Bot
Comment 6 2015-03-30 15:47:52 PDT
Comment on attachment 249773 [details] Patch Clearing flags on attachment: 249773 Committed r182157: <http://trac.webkit.org/changeset/182157>
WebKit Commit Bot
Comment 7 2015-03-30 15:47:58 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.