302 Redirect Responses are Cached to disk despite "Cache-control: no-cache", no-store", "Pragma: no-cache" and "Expires: -1" HTTP header being set

RESOLVED FIXED in Firefox 20

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: lsmercer, Assigned: mayhemer)

Tracking

({privacy, regression})

16 Branch
mozilla21
x86_64
Windows 7
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox20+ fixed, firefox21 fixed)

Details

Attachments

(1 attachment)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.11 (KHTML, like Gecko) Chrome/23.0.1271.64 Safari/537.11

Steps to reproduce:

Using firefox 16.0.2 I navigated to a page over HTTPS that responds with a 302 redirect response.

The 302 response had the following HTTP Headers set:
Cache-control: no-cache, no-store
Pragma: no-cache
Expires: -1

This can be recreated with the following php page:
<?php
header('Cache-control: no-cache, no-store, private');
header('Pragma: no-cache');
header('Expires: -1');
header('Location: http://localhost/page.php', true, 302);
exit;
?>

The entire response was:
HTTP/1.1 302 Found
Date: Thu, 15 Nov 2012 10:34:29 GMT
Server: Apache
Cache-control: no-cache, no-store
Pragma: no-cache
Expires: -1
Location: https://localhost/page.php
Content-Length: 0
Keep-Alive: timeout=5, max=100
Connection: Keep-Alive
Content-Type: text/html


Actual results:

The browser redirected to the target location successfully. However, in the about:cache page the 302 is listed under the about:cache?device=disk

Furthermore, after restarting the browser session, the entry persisted.

The output of about:cache?device=disk can be found below. I would not expect to see redirect_test_1.php in the cache:

Information about the Cache Service

Disk cache device

Number of entries: 3
Maximum storage size: 1048576 KiB
Storage in use: 14 KiB
Cache Directory: C:\...\Cache
________________________________
Key Data size Fetch count Last modified Expires
https://localhost/redirect_test_1.php 0 bytes 1 2012-11-15 10:44:53 1970-01-01 00:00:00
http://localhost/page.php 217 bytes 1 2012-11-15 10:44:54 1970-01-01 00:00:00
http://localhost/favicon.ico 7782 bytes 3 2012-11-15 10:44:54 2012-12-06 17:28:21
----------------------------------

After restarting Firefox, these entries persist (despite the 'Expires' value being in the past).


Expected results:

If the following HTTP headers are set, the page or request should not be cached to disk:

Cache-control: no-cache, no-store
Pragma: no-cache
Expires: -1

On firefox 10.0.2, the 302 response is cached to 'memory'. It can be seen in the about:cache?device=memory page and is not present on the about:cache?device=disk page.

When the user exits the browser, the memory cache is cleared and the cache entry does not persist.

This behaviour would be preferable.
Honza, Patrick, shouldn't we be calling UpdateInhibitPersistentCachingFlag in the redirect case?  We don't seem to be....
Status: UNCONFIRMED → NEW
Component: Untriaged → Networking: HTTP
Ever confirmed: true
Product: Firefox → Core
We would like to get fixed sometime at this cycle.
Tracking only so we can make sure it got into trunk by the time 20 moves to Aurora.  Tracking on trunk version doesn't really get much release-driver attention on the bug so please prioritize this and get an assignee on it so that the likelihood of a fix in the next 5 weeks is higher.
I'll fix this.
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Posted patch v1Splinter Review
coding: 10 minutes, writing the test: 3 hours...

https://tbpl.mozilla.org/?tree=Try&rev=0cf261b0f837
Attachment #707932 - Flags: review?(michal.novotny)
Comment on attachment 707932 [details] [diff] [review]
v1

Review of attachment 707932 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/test/unit/test_bug812167.js
@@ +12,5 @@
> +- check cache entry for the 302 response is not cached at all
> +*/
> +
> +var httpserver = null;
> +// Need to randomize, because apparently no one clears our cache

Wouldn't it be better to simply clear the cache by calling evict_cache_entries()?
Attachment #707932 - Flags: review?(michal.novotny) → review+
(In reply to Michal Novotny (:michal) from comment #7)
> Wouldn't it be better to simply clear the cache by calling
> evict_cache_entries()?

It may just slow things down, IMO.  This randomization is copied from other tests.

Thanks for the review and help with this patch!
Comment on attachment 707932 [details] [diff] [review]
v1

https://hg.mozilla.org/integration/mozilla-inbound/rev/c4887bfe1e9a

This is tracked for Fx20 (aurora at this time).  Do I have to ask for approval for this patch or just land?
You need to ask for approval.
https://hg.mozilla.org/mozilla-central/rev/c4887bfe1e9a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Comment on attachment 707932 [details] [diff] [review]
v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): since ever
User impact if declined: storing potentially private information despite server forbade storing it
Testing completed (on m-c, etc.): landed on m-c recently
Risk to taking this patch (and alternatives if risky): low, IMO, but I don't have an argument why ; however, the same technique of adding the flag to inhibit caching is used on other places of this code, not causing troubles
String or UUID changes made by this patch: none/none
Attachment #707932 - Flags: approval-mozilla-aurora?
Comment on attachment 707932 [details] [diff] [review]
v1

Approving, assuming we have enough time to bake this on Aurora and Beta before shipping to shake out potential regressions.
Attachment #707932 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.