Bug 71509 – WebKit memory cache doesn't respect Vary header

• Component: New Bugs
• Status: RESOLVED FIXED

[Andrew Berry]

I'm running into a problem where Webkit doesn't appear to be respecting the Vary: Cookie header when Cache-Control is set. For example:

HTTP/1.1 200 OK
Server: Apache/2.2.3 (CentOS)
X-Powered-By: PHP/5.2.17
Cache-Control: public, max-age=300
Last-Modified: Thu, 03 Nov 2011 21:09:15 +0000
Expires: Sun, 11 Mar 1984 12:00:00 GMT
Vary: Cookie
ETag: "1320354555"
Content-Type: text/html; charset=utf-8
Content-Length: 23080
Date: Thu, 03 Nov 2011 21:09:15 GMT
X-Varnish: 1635548510
Age: 0
Via: 1.1 varnish
Connection: keep-alive
X-Varnish-Cache: MISS

This is causing pages previously loaded to only hit the server if a manual reload is done, or if 300 seconds pass.

To reproduce:

1. View a few pages as an anonymous user where the above Cache-Control and Vary headers are set.
2. Log into the site, such that a cookie is set the headers are now something like:

Cache-Control: no-cache, must-revalidate, post-check=0, pre-check=0

3. Browse back to one of the pages you previously loaded. Webkit won't make an outbound HTTP request even though the cookie has changed.

I've verified this in Webkit nightlies in both OS X and Windows, and haven't run into this problem in any other browsers.

[Robert Brown]

I am having the same issue, specifically with Safari 5.1.1 (7534.51.22) for both MacOS and Windows. Chrome does not exhibit this behavior.

I can reproduce as follows.

1) Visit a webpage, /myurl. Returned headers are as follows:
Cache-Control: public, max-age=3600
Expires: Sun, 11 Mar 1984 12:00:00 GMT
Vary: Cookie,Accept-Encoding

Note that there are no cookies presented to the client at this time.

2) Authenticate to the site. A single session cookie is created for the client browser. During the authentication process, the user is redirected to a different URL. 

IE, during this step a POST is made to the same URL, and it receives a 302 Found to redirect to a different URL. I have verified at this point that the session cookie has been properly created.

3) Navigate back to the original page, /myurl. Even though a cookie is present in the browser and the URL had Vary:Cookie set, the browser serves up a cached webpage. I have run a network sniffer and verified that it does not make a connection back to the server for a new page.

I did verify that if I modified the original Cache-Control header to no-cache, this does not happen. So it definitely has to do with Cache-Control and Vary.

I can reproduce this 100% of the time, and I'd be happy to provide a test login to my site if you have an interest in viewing the details. I also have network sniffer captures and ngrep output.

[Robert Brown]

FWIW, I also tested this with the latest Webkit build, 5.1.1 7534.51.22, r100547 and this behavior still happens. So it does not seem like it has been fixed along the way.

[Robert Brown]

(In reply to comment #2)
> FWIW, I also tested this with the latest Webkit build, 5.1.1 7534.51.22, r100547 and this behavior still happens. So it does not seem like it has been fixed along the way.

If anyone else is trying to work around this with Varnish, here's what I did. I'm stripping out the Cache-Control header to ensure that Webkit does not improperly cache it. I only have one page that presents a real problem, hence the && req.url.

sub vcl_fetch {
  if (req.http.user-agent ~ "Safari" && req.url == "/myurl") {
    unset beresp.http.Cache-Control;
    set beresp.http.Cache-Control = "no-cache";
  }
}

[Andrew Berry]

I'm pretty sure that you don't need to unset the cache-control header first. At least, it's working fine for me.

Also remember to separate out your cache bins. Otherwise, if Safari hits a page first, all clients will be presented with the no-cache directive.

Finally, Chrome contains the Safari keyword in it's user agent as well. I'm sure there are other Webkit derivatives that have Safari in the UA, but I'm erring on the side of caution with them.

In sub vcl_fetch:
  # Force Safari to always check the server as it doesn't respect Vary: cookie.
  # See https://bugs.webkit.org/show_bug.cgi?id=71509
  if (req.http.user-agent ~ "Safari" && !req.http.user-agent ~ "Chrome") {
    set beresp.http.cache-control = "max-age: 0";
  }

In sub vcl_hash:
  # Ensure we have a separate bin for Safari as it has a different cache-conrol
  # header.
  if (req.http.user-agent ~ "Safari" && !req.http.user-agent ~ "Chrome") {
    set req.hash += "safari-disable-cache-control";
  }

The root of the problem seems to be that safari doesn't send any cookies in the request.

I'm able to duplicate the problem but when I hit the other page that i've previously hit without being logged in,   Safari sends a request without any cookies.  This causes the proxy (varnish in our case) to send me a cache hit since there are no cookies and it doesn't think I have a session.

from safari:
Request URL:https://www.pdup.allplayers.com/group_search
Request Method:GET
Status Code:200 OK
Request Headers
Accept:text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Referer:https://www.pdup.allplayers.com/group_search
User-Agent:Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_8) AppleWebKit/534.54.16 (KHTML, like Gecko) Version/5.1.4 Safari/534.54.16

This is after login so there are a number of Cookies that have been set on this domain.

Here with same scenerio what I got on chrome (which worked fine):
Request URL:https://www.pdup.allplayers.com/group_search
Request Method:GET
Status Code:200 OK
Request Headers
Accept:text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Charset:ISO-8859-1,utf-8;q=0.7,*;q=0.3
Accept-Encoding:gzip,deflate,sdch
Accept-Language:en-US,en;q=0.8
Connection:keep-alive
Cookie:SESSe74701e02960fa02cd447c1478f7abe1=ee4235eac16a7104698da11cb0ee1231; CHOCOLATECHIPSSLb4db41620f5fb578b70bec49d4025279=0KDzOgVQrwzDuGc%2FaqzD%2FgSMFuUAlD7lOn8oL2E07HwhG%2BlyfKjNejZSXg9Z2QCJPGtL08bz3s1wwPts2RvzxxcZe6yCOZ5QhpSXTfvcrauNnfZVGp1nyKAh5q%2Fa651iB6QnByNXSw26THXReIVHBWyOcAuAAg85oq4ac9LPijuF3hdIBOI3fvBdjTfjrrKmRovgDtnjjtxeEPuSy7LjRduAP5v%2BeAGtgGVMFxCG07mhSdt0gl84Zgl1zJdQEjN%2FViHR3HzBzU5Jf23oWMAjNoAoOf76OW7qmRXJIIEyJiIdImdJd%2BbjeF54wA7jbf2Ye0oueYehoBFVqcUzggCJE8RZoSQxGdPDnH6OMRsmlC0U0F%2BfuE5pwwRV%2B9%2Fv3%2BqS; __utma=181778935.695123550.1332169001.1332180843.1332184029.3; __utmc=181778935; __utmz=181778935.1332169001.1.1.utmcsr=(direct)|utmccn=(direct)|utmcmd=(none); SESSc911711ef8706eaf3847304b167bf69d=7c0aba40cd2cfac4336206df312018e3; CHOCOLATECHIPSSL1ad866034ea2b30f49d3f9fc345b7170=j7J%2BBjfnwH%2BHSmRSI3HAoTLI4C4x3OA4oDCz6Jk2TSh1ekZzojRnz09dD0h9qtSpMZjDdNJHB4YnGc6yBQIqE9WTAH0g5ycNH%2BUWnavg8uUdb%2BvU96nok1NruqPElke6NuBFntubUXwW4g5EPiChLSMAbPfnwZxfSMJHKmhApiEeSZG3Y%2FS%2B8FDLXKFuuflIH8uQ63yYRUQ8bGHUjH6n5X7i5jH46%2F%2FO%2FN9GBShL38tltcIT4TQF3OVzVRPvO7ClKxUaJlcZNiFjzrZH%2FQZZEkbgby0QF%2B9Z0FVBkE1N1ZyTyrAtzWnXno8WpQLky9KW1aHbrZDpZZECYh3g%2BsXaNJEQqgyyXcrWMrr2z16gBz7%2BstRDD9Q8FlFJD0ESd9pl2pEHizLEvUvgG9%2BPu6fX0w%3D%3D; __utma=24126587.930127860.1332169084.1332184202.1332350831.4; __utmb=24126587.11.10.1332350831; __utmc=24126587; __utmz=24126587.1332169084.1.1.utmcsr=(direct)|utmccn=(direct)|utmcmd=(none)
Host:www.pdup.allplayers.com
If-Modified-Since:Wed, 21 Mar 2012 17:27:35 +0000
If-None-Match:"1332350855"
Referer:https://www.pdup.allplayers.com/g/_gear_wildcats/registration_complete
User-Agent:Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_8) AppleWebKit/535.11 (KHTML, like Gecko) Chrome/17.0.963.79 Safari/535.11

the default.vcl shown in comment #4 

does resolve the issue on our server setup.  It essentially makes it so that safari always says give me a new page don't allow it to be cached.  I guess the inspect element looking at headers on safari don't show the full header, as it must be sending cookies even though they are not shown on the inspect element web kit, but are shown on chrome.

thanks   Andrew Berry for the tips.

[Silvia Pfeiffer]

Note there is a similar issue with byte range requests: https://bugs.webkit.org/show_bug.cgi?id=82672

[Alexey Proskuryakov]

Yes, these are both deficiencies of WebCore memory cache. Unfortunately, there is no way to disable WebCore memory cache, but keep the more correct CFNetwork cache.

[Antti Koivisto]

Could someone provide an actual repro URL for this bug? The only repro I have seen starts with "1. Install Drupal core."

[Andrew Berry]

This might be fixed now - I can't reproduce this on Safari 8.0.7. Can anyone else?

[ruv]

(In reply to comment #10)
> This might be fixed now - I can't reproduce this on Safari 8.0.7.

Some of our users have faced this issue using Safari 8.0.7.
Could you please check the bug via the following testcase:
https://iafastro.directory/iac/testcase/cache-vary/

This script measures time to check whether the page was loaded from a cache after cookie is changed.

[Antti Koivisto]

Safari 9.0 on El Capitan passes the test so I think this has been fixed by the new cache code.

[ruv]

(In reply to comment #12)
Antti, could you please refresh the page and run the test again by press Auto-test button?
I have added comparison the current and the sent cookie to make the test more accurate.

[Antti Koivisto]

Ok, the updated test case still fails.

[Antti Koivisto]

Thanks for the test!

[Damien McKenna]

FYI Safari 9.0.3 on OSX 10.11.3 passes the test.

[Antti Koivisto]

Created attachment 280597 [details]
patch

[Chris Dumez]

Comment on attachment 280597 [details]
patch

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

> Source/WebCore/loader/cache/CachedResource.cpp:762
> +        // If Vary header is present don't use the memory cache entry. Instead fall back to disk cache that knows how to handle it.

This comment seems a bit odd. In case of memory cache revalidation, we bypass the disk cache.

[Chris Dumez]

Comment on attachment 280597 [details]
patch

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

R=me although I wish we simply implemented basic Vary support in the memory cache.

>> Source/WebCore/loader/cache/CachedResource.cpp:762
>> +        // If Vary header is present don't use the memory cache entry. Instead fall back to disk cache that knows how to handle it.
> 
> This comment seems a bit odd. In case of memory cache revalidation, we bypass the disk cache.

Oh, I see that you worked around this by tweaking canUseCacheValidator() above. Ok.

[Sam Weinig]

How much additional complex it would it be to implement full vary support?

[Antti Koivisto]

> R=me although I wish we simply implemented basic Vary support in the memory
> cache.

There is not really "basic" Vary support, it needs to be fully implemented. I don't think it is worth the complexity to move the logic to WebCore. Similarly I don't think memory cache should handle redirects (as it currently does).

[Antti Koivisto]

(In reply to comment #20)
> How much additional complex it would it be to implement full vary support?

It would require moving the logic from network cache code in WK2 to WebCore. Memory cache entries would need to contain the varying header values. 

The biggest problem is "Vary:Cookie" (which also the most common use) as cookie header is not visible in ResourceRequest and gets added magically behind the scenes by CFNetwork. Getting the value from web process involves synchronous IPC to network process and synchronous cookie storage read. Making this not a performance risk is tricky.

[Radar WebKit Bug Importer]

<rdar://problem/26651033>

[Antti Koivisto]

Created attachment 280613 [details]
alternative patch

Here is a full implementation. It is not too bad. We only need to fetch cookies when we actually see Vary:Cookie so maybe that is not a real issue.

[Build Bot]

Comment on attachment 280613 [details]
alternative patch

Attachment 280613 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1454339

New failing tests:
http/tests/navigation/redirect-to-random-url-versus-memory-cache.html

[Build Bot]

Created attachment 280627 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Antti Koivisto]

Created attachment 280630 [details]
vary header implementation for memory cache

[Ryosuke Niwa]

Comment on attachment 280630 [details]
vary header implementation for memory cache

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

> LayoutTests/ChangeLog:12
> [phone]  Antti Koivisto  <[e-mail]>

Double change logs.

[Antti Koivisto]

https://trac.webkit.org/r201805