[Django] #20346: cache middleware should vary on full URL
Django
-------------------------------------+--------------------
Reporter: jamey | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: 1.3
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+--------------------
Standard HTTP cache behavior treats different URLs as different resources,
but Django's cache middleware only generates cache keys based on
`request.get_full_path()`.
I believe that `django.utils.cache._generate_cache_[header_]key` should
use `request.build_absolute_uri()` instead of `request.get_full_path()` so
different hosts and schemes will get different cache entries.
I'm using Django 1.3 but as far as I can tell this hasn't changed as of
today's git master.
--
Ticket URL: <https://code.djangoproject.com/ticket/20346>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: jamey | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: 1.3
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by ludw):
* needs_docs: => 0
* needs_better_patch: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
This seems like a reasonable idea. Can't find any reasons why we shouldn't
do this.
--
Ticket URL: <https://code.djangoproject.com/ticket/20346#comment:1>
Django
Reporter: jamey | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: 1.3
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by susan):
PR is here: https://github.com/django/django/pull/1341 Feel free to code
review. Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/20346#comment:2>
Django
Reporter: jamey | Owner: susan
Type: Bug | Status: assigned
Component: Core (Cache system) | Version: 1.3
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by susan):
* status: new => assigned
* owner: nobody => susan
--
Ticket URL: <https://code.djangoproject.com/ticket/20346#comment:3>
Django
Reporter: jamey | Owner: susan
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by anonymous):
* has_patch: 0 => 1
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/20346#comment:4>
Django
Reporter: jamey | Owner:
Type: Bug | Status: new
Component: Core (Cache system) | Version: 1.3
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by susan):
* owner: susan =>
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/20346#comment:5>
Django
Reporter: jamey | Owner:
Type: Bug | Status: new
Component: Core (Cache system) | Version: 1.3
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by susan):
* cc: susan.tan.fleckerl@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/20346#comment:6>
Django
Reporter: jamey | Owner:
Type: Bug | Status: new
Component: Core (Cache system) | Version: 1.3
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timo):
This needs a test to demonstrate the change in behavior.
--
Ticket URL: <https://code.djangoproject.com/ticket/20346#comment:7>
Django
Reporter: jamey | Owner:
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_better_patch: 0 => 1
* version: 1.3 => master
* needs_tests: 1 => 0
* needs_docs: 0 => 1
Comment:
Here's an [https://code.djangoproject.com/ticket/20346 updated PR] with
tests. I left some minor comments for improvement.
One concern is that upgrading Django will change cache keys for all the
existing pages now that the domain name is part of the key. At a minimum,
this needs to be documented in the release notes. Ideas for how we might
be able to do this in a backwards compatible fashion would also be
welcome.
--
Ticket URL: <https://code.djangoproject.com/ticket/20346#comment:8>
Django
Reporter: jamey | Owner:
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Comment (by ijl):
I've documented the changes in the 1.7 release notes. I'm at a loss for
backwards-compatible changes using the keys, given that existing cache
keys use a hash.
--
Ticket URL: <https://code.djangoproject.com/ticket/20346#comment:9>
Django
Reporter: jamey | Owner:
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Changes (by timo):
* needs_docs: 1 => 0
Comment:
Docs still need some minor tweaks. Please uncheck "Patch needs
improvement" when you have a chance to update it.
--
Ticket URL: <https://code.djangoproject.com/ticket/20346#comment:10>
Django
Reporter: jamey | Owner: Tim
Type: Bug | Graham <timograham@…>
Component: Core (Cache system) | Status: closed
Severity: Normal | Version: master
Keywords: | Resolution: fixed
Has patch: 1 | Triage Stage: Accepted
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 1
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham <timograham@…>):
* owner: => Tim Graham <timograham@…>
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"71a03e01aa19cbde08e915d156abf39b67d904ef"]:
{{{
#!CommitTicketReference repository=""
revision="71a03e01aa19cbde08e915d156abf39b67d904ef"
Fixed #20346 -- Made cache middleware vary on the full URL.
Previously, only the URL path was included in the cache key.
Thanks jamey for the suggestion.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20346#comment:11>