Bug 142989

Summary: [WK2] The WebKit network cache does not cache responses with "Content-Disposition: attachment" header
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: 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
Patch none

Description Chris Dumez 2015-03-23 15:11:20 PDT
The WebKit network cache does not cache responses with "Content-Disposition: attachment" header. I don't see any reason why we shouldn't be able to cache such responses.

This is causing a decent amount of cache miss in the wild. I have noticed for example that when loading disney.com, we fail to cache the following resources because of this policy:
http://pagead2.googlesyndication.com/pagead/osd.js
http://pagead2.googlesyndication.com/pagead/show_companion_ad.js

HTTP headers for these resources:

==============
http://pagead2.googlesyndication.com/pagead/osd.js:

HTTP/1.1 200 OK
P3P: policyref="http://www.googleadservices.com/pagead/p3p.xml", CP="NOI DEV PSA PSD IVA IVD OTP OUR OTR IND OTC"
Content-Type: text/javascript; charset=ISO-8859-1
ETag: 15685665013492268219
Date: Mon, 23 Mar 2015 20:41:53 GMT
Expires: Mon, 23 Mar 2015 21:41:53 GMT
X-Content-Type-Options: nosniff
Content-Disposition: attachment; filename="f.txt"
Server: cafe
Content-Length: 48333
X-XSS-Protection: 1; mode=block
Age: 3250
Cache-Control: public, max-age=3600
Alternate-Protocol: 80:quic,p=0.5
==============
http://pagead2.googlesyndication.com/pagead/show_companion_ad.js:

HTTP/1.1 200 OK
P3P: policyref="http://www.googleadservices.com/pagead/p3p.xml", CP="NOI DEV PSA PSD IVA IVD OTP OUR OTR IND OTC"
Content-Type: text/javascript; charset=UTF-8
Date: Mon, 23 Mar 2015 21:46:36 GMT
Expires: Mon, 23 Mar 2015 22:46:36 GMT
X-Content-Type-Options: nosniff
Content-Disposition: attachment; filename="f.txt"
Server: cafe
X-XSS-Protection: 1; mode=block
Age: 1318
Cache-Control: public, max-age=3600
Alternate-Protocol: 80:quic,p=0.5
Accept-Ranges: none
Vary: Accept-Encoding
Transfer-Encoding: chunked
==============

Given the "Cache-Control: public, max-age=3600" header, these resources are clearly meant to be cacheable.
Comment 1 Radar WebKit Bug Importer 2015-03-23 15:11:47 PDT
<rdar://problem/20265992>
Comment 2 Chris Dumez 2015-03-23 15:37:53 PDT
Created attachment 249288 [details]
Patch
Comment 3 Antti Koivisto 2015-03-24 05:18:00 PDT
Comment on attachment 249288 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=249288&action=review

r=me

I don't remember why I prevented this in the first place.

> LayoutTests/platform/mac-wk1/TestExpectations:100
>  http/tests/cache/disk-cache-validation-no-body.html
>  http/tests/cache/disk-cache-disable.html
>  http/tests/cache/disk-cache-vary-cookie.html
> +http/tests/cache/disk-cache-validation-attachment.html

Maybe we should move the tests to a directory of their own so we can disable them all with one line.
Comment 4 Chris Dumez 2015-03-24 09:36:05 PDT
Created attachment 249330 [details]
Patch
Comment 5 Chris Dumez 2015-03-24 09:43:15 PDT
Created attachment 249331 [details]
Patch
Comment 6 Chris Dumez 2015-03-24 09:44:59 PDT
Comment on attachment 249331 [details]
Patch

Clearing flags on attachment: 249331

Committed r181894: <http://trac.webkit.org/changeset/181894>
Comment 7 Chris Dumez 2015-03-24 09:45:05 PDT
All reviewed patches have been landed.  Closing bug.