cache_page decorator bypasses any Vary headers set in middleware
|Reported by:||carljm||Owned by:||nobody|
|Component:||Core (Cache system)||Version:|
|Cc:||django@…, inactivist@…, denis.cornehl@…||Triage Stage:||Accepted|
|Has patch:||no||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
A number of common response middlewares in Django (gzip, sessions, locale, csrf) add to the Vary header on responses, which is checked both by Django's caching system and upstream HTTP caches to determine what requests can safely be served that cached response. Getting the Vary header correct can be quite important, as failure to include it can mean upstream caches show session-private content to users who should not see it.
Since view decorators run on the outgoing response first, before response middleware, the cache_page decorator caches the response before any of the mentioned response middlewares have a chance to add their Vary headers. This means two things: 1) the cache key used won't include the headers the response ought to vary on, and Django may later serve that response to users who really shouldn't get it, and 2) when that cached response is later served to a user, it still won't include the Vary header that it should have, and thus may also be cached wrongly by an upstream HTTP cache.
I can't see a reasonable way of fixing this that maintains the current semantics of the cache_page decorator. The only option I've come up with is to require all users of full-response caching to always include the UpdateCacheMiddleware in their MIDDLEWARE_CLASSES (in its recommended spot at the top of the list, thus last in response processing), and only do the actual caching there. We'd then have to provide some way to say "don't cache pages unless they are marked" (a setting? ugh), and then the cache_page decorator would just mark the response for later caching (which would override the global don't-cache flag).
Change History (19)
comment:1 Changed 5 years ago by carljm
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
- Triage Stage changed from Unreviewed to Design decision needed
Changed 5 years ago by idangazit
comment:16 Changed 3 years ago by carljm
- Triage Stage changed from Design decision needed to Accepted