Opened 16 years ago

Last modified 23 hours ago

#13894 assigned Cleanup/optimization

Provide caching guidance for messages framework

Reported by: Torsten Bronger Owned by: Sarah Boyce
Component: Documentation Version: 1.2
Severity: Normal Keywords:
Cc: bronger@…, Sarah Boyce, Andy Miller Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Upstream caching should be disabled when messages have been consumed in the current request. I added

    if request._messages.used:
        add_never_cache_headers(response)

to "process_response" of MessageMiddleware and it works nicely for me. Doesn't it make sense to do that always, or at least, by default?

Rationale:

When the message framework is used and the messages are iterared over in the current view, this means that it is posible that they are displayed and consumed. Thus, when the same page is retrieved next time, they won't appear, or with different content.

It is very difficult to handle this in cache-related code of that view because the messages have been generated in a previous request, which may even belong to another view.

Change History (8)

comment:1 by Chris Beaven, 16 years ago

Component: UncategorizedContrib apps
Triage Stage: UnreviewedDesign decision needed

It does sound like a valid issue.

But isn't the opposite true as well? If a page (without any messages displayed) is cached, then assuming messages were in the queue and this page was hit again, the messages will not get rendered (or consumed). It seems like fixing this only fixes half of the caching problem.

comment:2 by Torsten Bronger, 16 years ago

First of all: As we had to find out, "add_never_cache_headers" doesn't work. It only sets "Expires" and "Cache-control/max-age" but it also sets "ETag" and "Last-Modified" so that the next request may be taken from the browser's cache. (It's strange that such a function is called "add_never_cache_headers". It should be called "add_expire_immediately_headers". Anyway.)

Instead, we use

if request._messages.used:
    del response["ETag"]
    del response["Last-Modified"]
    response["Expires"] = "Fri, 01 Jan 1990 00:00:00 GMT"
    # FixMe: One should check whether the following settings are
    # sensible.
    response["Pragma"] = "no-cache"
    response["Cache-Control"] = "no-cache, no-store, max-age=0, must-revalidate, private"

Now to your question. You're absolutely right. We don't run into this problem because messages are shown on pages which have recently changed anyway (the message reads "You have successfully changed ..."). If we had a message saying "Editing aborted. Nothing was changed.", it would not get displayed.

comment:3 by Gabriel Hurley, 15 years ago

Component: Contrib appscontrib.messages

comment:4 by Graham King, 15 years ago

Severity: Normal
Type: Bug

comment:5 by Aymeric Augustin, 14 years ago

Easy pickings: unset
Resolution: wontfix
Status: newclosed
UI/UX: unset

I don't think Django can handle this problem automatically and correctly.

It is a very common pattern to loop over messages in the base template of a website. If we disable caching simply because messages might be displayed on a given page, we're just killing the cache for most websites. If we disable it only when messages are displayed, we're only resolving half of the problem, as explained in comments 1 and 2.

To sum up:

  • Caching dynamic pages is hard :)
  • Use ETags to save bandwidth — Django provides a lot of flexibility at this level.
  • Use fragment caching in templates to save CPU — but don't cache dynamic parts, like messages!

comment:6 by Jacob Walls, 23 hours ago

Cc: Sarah Boyce added
Component: contrib.messagesDocumentation
Resolution: wontfix
Status: closednew
Summary: Disable upstream caching with messages frameworkProvide caching guidance for messages framework
Triage Stage: Design decision neededAccepted
Type: BugCleanup/optimization

The Security Team just closed a report extrapolating from CVE-2026-35192, suggesting patch_vary_headers(response, ("Cookie",)) was missing from the messages middleware. We closed it on the basis that caching pages with messages is problematic, and Django should refuse to guess what the correct behavior is. Still, we could provide some guidance in the docs to prevent mistakes.

I believe Sarah also plans to open a Forum thread to consider if we need any code changes here as well, but Accepting on the assumption we at least want a docs update.

comment:7 by Jacob Walls, 23 hours ago

Owner: changed from nobody to Sarah Boyce
Status: newassigned

comment:8 by Jacob Walls, 23 hours ago

Cc: Andy Miller added
Note: See TracTickets for help on using tickets.
Back to Top