Code

Opened 4 years ago

Last modified 9 months ago

#13008 assigned Bug

@never_cache decorator should add 'no-cache' & 'no-store'

Reported by: harm Owned by: susan
Component: Core (Cache system) Version: 1.2-beta
Severity: Normal Keywords:
Cc: obeattie, net147 Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I noticed that the never_cache django/views/decorators/cache.py does not add all the proper http headers to prevent that a page is cached.
It just adds a max-age=0. This does _not_ prevent browsers to actually cache that page.
For example when hitting back, firefox will happily show such a page if the connection to the server disappeared.

proposal, let never_cache() act as advertised: never cache. Let it add 'no-cache, must-revalidate' to the http response.

something like:

--- virtualenv/src/django/django/utils/cache.py 2010-03-02 15:11:54.000000000 +0100
+++ /tmp/cache.py       2010-03-02 15:11:48.000000000 +0100
@@ -104,6 +104,8 @@
         cache_timeout = settings.CACHE_MIDDLEWARE_SECONDS
     if cache_timeout < 0:
         cache_timeout = 0 # Can't have max-age negative
+        patch_cache_control(response, no_cache=True)
+        patch_cache_control(response, must_revalidate=True)
     if not response.has_header('ETag'):
         response['ETag'] = '"%s"' % md5_constructor(response.content).hexdigest()
     if not response.has_header('Last-Modified'):

Attachments (1)

13008.diff (507 bytes) - added by SmileyChris 2 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 4 years ago by russellm

  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 follow-up: Changed 4 years ago by lukeplant

Personally, I really hate it when I press the back button and the browser tells me I can't view that page any longer. Adding this behaviour to all the admin pages could be a pain in the neck, especially in the common work flow of "list of objects -> click one to edit/view -> go back to the list because you don't need to change it". Oon the other hand I am probably a 'power user' who knows when I am likely to be dealing with old data, and I can always use new tabs. But I do think we should think about the usability impact on the admin, which has 'never_cache' applied everywhere.

comment:3 in reply to: ↑ 2 Changed 4 years ago by kmtracey

Replying to lukeplant:

Personally, I really hate it when I press the back button and the browser tells me I can't view that page any longer.

Me too. But I don't see that behavior (from Firefox 3.5.7 or 3.6, or Chrome 4.0) if I run with the proposed change. In fact I don't even see either of those browsers re-requesting the page on using the back and forward buttons if I run with the proposed change, so I'm not entirely sure the proposed change accomplishes what it was intended to.

I do see IE8 re-request the page with the proposed change. However, for the others I have to also add: patch_cache_control(response, no_store=True), before they will start re-fetching the page when accessed via the back/forward buttons. But when they refetch, they all do it automatically and not with any error message about not being able to access the page anymore (unless I stop the server and then I get the standard can't establish connection message).

comment:4 Changed 4 years ago by obeattie

  • Cc obeattie added

comment:5 Changed 3 years ago by lukeplant

  • Severity set to Normal
  • Type set to Bug

@kmtracey - I just realised, we don't see those annoying pages about not being able to view the page because they happen when the page has been set to never cache/store AND was the result of a POST. Since we always re-direct after a POST in Django admin, we don't see those pages, and the web browser is happy to silently re-request the page, since the pages in history are always the result of GET requests. Well done us!

comment:6 Changed 3 years ago by harm

Besides the no_cache and must_revalidate this also needs no_store. (For Firefox)

+          patch_cache_control(response, no_store=True)

comment:7 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:8 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:9 Changed 2 years ago by eykd

Shouldn't this also set the Pragma: no-cache header? (See http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9)

Last edited 2 years ago by eykd (previous) (diff)

comment:10 Changed 2 years ago by harm

I think it should not be needed. Pragma: no-cache is intended for http requests, not http responses. (See http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9)

comment:11 Changed 2 years ago by SmileyChris

More explicitly, http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.32 says:

Note: because the meaning of "Pragma: no-cache as a response
header field is not actually specified, it does not provide a
reliable replacement for "Cache-Control: no-cache" in a response

I can't see must-revalidate being necessary, either. But maybe I'm overlooking something.

Changed 2 years ago by SmileyChris

comment:12 Changed 20 months ago by net147

  • Cc net147 added

comment:13 Changed 16 months ago by harm

confirmed, must-revalidate is not necessary.

tested adding cache control headers with

@cache_control(max_age=0, no_cache=True, no_store=True)

on FF 19.0 + FF 19.02 + chrome 25 (on mac)
A 'back' button in the browser, does NOT reload a view.

So I think your patch 13008.diff is correct.

Last edited 16 months ago by harm (previous) (diff)

comment:14 Changed 16 months ago by harm

  • Has patch set

comment:15 Changed 9 months ago by susan

  • Owner changed from nobody to susan
  • Status changed from new to assigned
  • Summary changed from @never_cache decorator should add 'no-cache' & 'must-revalidate' to @never_cache decorator should add 'no-cache' & 'no-store'

comment:16 Changed 9 months ago by susan

Where should I write a test? Should it be in cache/tests.py? The function add_never_cache_headers is never tested in thetests/ directory.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as assigned
The owner will be changed from susan to anonymous. Next status will be 'assigned'
The ticket will be disowned. Next status will be 'new'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.