From 71a03e01aa19cbde08e915d156abf39b67d904ef Mon Sep 17 00:00:00 2001 From: ijl Date: Thu, 26 Dec 2013 23:14:08 -0500 Subject: [PATCH] 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. --- django/utils/cache.py | 20 ++++++------- docs/releases/1.7.txt | 13 ++++++++ docs/topics/cache.txt | 8 ++++- tests/cache/tests.py | 70 ++++++++++++++++++++++++++++++++++++++----- 4 files changed, 92 insertions(+), 19 deletions(-) diff --git a/django/utils/cache.py b/django/utils/cache.py index d89408a7a2..365733fedf 100644 --- a/django/utils/cache.py +++ b/django/utils/cache.py @@ -191,25 +191,25 @@ def _generate_cache_key(request, method, headerlist, key_prefix): value = request.META.get(header, None) if value is not None: ctx.update(force_bytes(value)) - path = hashlib.md5(force_bytes(iri_to_uri(request.get_full_path()))) + url = hashlib.md5(force_bytes(iri_to_uri(request.build_absolute_uri()))) cache_key = 'views.decorators.cache.cache_page.%s.%s.%s.%s' % ( - key_prefix, method, path.hexdigest(), ctx.hexdigest()) + key_prefix, method, url.hexdigest(), ctx.hexdigest()) return _i18n_cache_key_suffix(request, cache_key) def _generate_cache_header_key(key_prefix, request): """Returns a cache key for the header cache.""" - path = hashlib.md5(force_bytes(iri_to_uri(request.get_full_path()))) + url = hashlib.md5(force_bytes(iri_to_uri(request.build_absolute_uri()))) cache_key = 'views.decorators.cache.cache_header.%s.%s' % ( - key_prefix, path.hexdigest()) + key_prefix, url.hexdigest()) return _i18n_cache_key_suffix(request, cache_key) def get_cache_key(request, key_prefix=None, method='GET', cache=None): """ - Returns a cache key based on the request path and query. It can be used + Returns a cache key based on the request URL and query. It can be used in the request phase because it pulls the list of headers to take into - account from the global path registry and uses those to build a cache key + account from the global URL registry and uses those to build a cache key to check against. If there is no headerlist stored, the page needs to be rebuilt, so this @@ -229,9 +229,9 @@ def get_cache_key(request, key_prefix=None, method='GET', cache=None): def learn_cache_key(request, response, cache_timeout=None, key_prefix=None, cache=None): """ - Learns what headers to take into account for some request path from the - response object. It stores those headers in a global path registry so that - later access to that path will know what headers to take into account + Learns what headers to take into account for some request URL from the + response object. It stores those headers in a global URL registry so that + later access to that URL will know what headers to take into account without building the response object itself. The headers are named in the Vary header of the response, but we want to prevent response generation. @@ -264,7 +264,7 @@ def learn_cache_key(request, response, cache_timeout=None, key_prefix=None, cach return _generate_cache_key(request, request.method, headerlist, key_prefix) else: # if there is no Vary header, we still need a cache key - # for the request.get_full_path() + # for the request.build_absolute_uri() cache.set(cache_key, [], cache_timeout) return _generate_cache_key(request, request.method, [], key_prefix) diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index 2c3e3f217d..dbbdcf9c81 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -622,6 +622,19 @@ cache-specific errors. This has been fixed in Django 1.7, see .. _Ticket #21200: https://code.djangoproject.com/ticket/21200 +Cache keys are now generated from the request's absolute URL +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Previous versions of Django generated cache keys using a request's path and +query string but not the scheme or host. If a Django application was serving +multiple subdomains or domains, cache keys could collide. In Django 1.7, cache +keys vary by the absolute URL of the request including scheme, host, path, and +query string. For example, the URL portion of a cache key is now generated from +``http://www.example.com/path/to/?key=val`` rather than ``/path/to/?key=val``. +The cache keys generated by Django 1.7 will be different from the keys +generated by older versions of Django. After upgrading to Django 1.7, the first +request to any previously cached URL will be a cache miss . + Passing ``None`` to ``Manager.db_manager()`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/topics/cache.txt b/docs/topics/cache.txt index 7ef7285fb7..5fc352f4ec 100644 --- a/docs/topics/cache.txt +++ b/docs/topics/cache.txt @@ -1046,13 +1046,19 @@ the contents of a Web page depend on a user's language preference, the page is said to "vary on language." By default, Django's cache system creates its cache keys using the requested -path and query -- e.g., ``"/stories/2005/?order_by=author"``. This means every +fully-qualified URL -- e.g., +``"http://www.example.com/stories/2005/?order_by=author"``. This means every request to that URL will use the same cached version, regardless of user-agent differences such as cookies or language preferences. However, if this page produces different content based on some difference in request headers -- such as a cookie, or a language, or a user-agent -- you'll need to use the ``Vary`` header to tell caching mechanisms that the page output depends on those things. + .. versionchanged:: 1.7 + + Cache keys use the request's fully-qualified URL rather than path + and query string. + To do this in Django, use the convenient :func:`django.views.decorators.vary.vary_on_headers` view decorator, like so:: diff --git a/tests/cache/tests.py b/tests/cache/tests.py index e17bad1523..94790ed740 100644 --- a/tests/cache/tests.py +++ b/tests/cache/tests.py @@ -1198,8 +1198,20 @@ class CacheUtils(TestCase): """TestCase for django.utils.cache functions.""" def setUp(self): + self.host = 'www.example.com' self.path = '/cache/test/' - self.factory = RequestFactory() + self.factory = RequestFactory(HTTP_HOST=self.host) + + def _get_request_cache(self, method='GET', query_string=None, update_cache=None): + request = self._get_request(self.host, self.path, + method, query_string=query_string) + request._cache_update_cache = True if not update_cache else update_cache + return request + + def _set_cache(self, request, msg): + response = HttpResponse() + response.content = msg + return UpdateCacheMiddleware().process_response(request, response) def test_patch_vary_headers(self): headers = ( @@ -1229,10 +1241,19 @@ class CacheUtils(TestCase): self.assertEqual(get_cache_key(request), None) # Set headers to an empty list. learn_cache_key(request, response) - self.assertEqual(get_cache_key(request), 'views.decorators.cache.cache_page.settingsprefix.GET.9fa0fd092afb73bdce204bb4f94d5804.d41d8cd98f00b204e9800998ecf8427e') + + self.assertEqual( + get_cache_key(request), + 'views.decorators.cache.cache_page.settingsprefix.GET.' + '18a03f9c9649f7d684af5db3524f5c99.d41d8cd98f00b204e9800998ecf8427e' + ) # Verify that a specified key_prefix is taken into account. learn_cache_key(request, response, key_prefix=key_prefix) - self.assertEqual(get_cache_key(request, key_prefix=key_prefix), 'views.decorators.cache.cache_page.localprefix.GET.9fa0fd092afb73bdce204bb4f94d5804.d41d8cd98f00b204e9800998ecf8427e') + self.assertEqual( + get_cache_key(request, key_prefix=key_prefix), + 'views.decorators.cache.cache_page.localprefix.GET.' + '18a03f9c9649f7d684af5db3524f5c99.d41d8cd98f00b204e9800998ecf8427e' + ) def test_get_cache_key_with_query(self): request = self.factory.get(self.path, {'test': 1}) @@ -1242,7 +1263,22 @@ class CacheUtils(TestCase): # Set headers to an empty list. learn_cache_key(request, response) # Verify that the querystring is taken into account. - self.assertEqual(get_cache_key(request), 'views.decorators.cache.cache_page.settingsprefix.GET.d11198ba31883732b0de5786a80cc12b.d41d8cd98f00b204e9800998ecf8427e') + + self.assertEqual( + get_cache_key(request), + 'views.decorators.cache.cache_page.settingsprefix.GET.' + 'beaf87a9a99ee81c673ea2d67ccbec2a.d41d8cd98f00b204e9800998ecf8427e' + ) + + def test_cache_key_varies_by_url(self): + """ + get_cache_key keys differ by fully-qualfied URL instead of path + """ + request1 = self.factory.get(self.path, HTTP_HOST='sub-1.example.com') + learn_cache_key(request1, HttpResponse()) + request2 = self.factory.get(self.path, HTTP_HOST='sub-2.example.com') + learn_cache_key(request2, HttpResponse()) + self.assertTrue(get_cache_key(request1) != get_cache_key(request2)) def test_learn_cache_key(self): request = self.factory.head(self.path) @@ -1250,7 +1286,12 @@ class CacheUtils(TestCase): response['Vary'] = 'Pony' # Make sure that the Vary header is added to the key hash learn_cache_key(request, response) - self.assertEqual(get_cache_key(request), 'views.decorators.cache.cache_page.settingsprefix.GET.9fa0fd092afb73bdce204bb4f94d5804.d41d8cd98f00b204e9800998ecf8427e') + + self.assertEqual( + get_cache_key(request), + 'views.decorators.cache.cache_page.settingsprefix.GET.' + '18a03f9c9649f7d684af5db3524f5c99.d41d8cd98f00b204e9800998ecf8427e' + ) def test_patch_cache_control(self): tests = ( @@ -1874,10 +1915,19 @@ class TestWithTemplateResponse(TestCase): self.assertEqual(get_cache_key(request), None) # Set headers to an empty list. learn_cache_key(request, response) - self.assertEqual(get_cache_key(request), 'views.decorators.cache.cache_page.settingsprefix.GET.9fa0fd092afb73bdce204bb4f94d5804.d41d8cd98f00b204e9800998ecf8427e') + + self.assertEqual( + get_cache_key(request), + 'views.decorators.cache.cache_page.settingsprefix.GET.' + '58a0a05c8a5620f813686ff969c26853.d41d8cd98f00b204e9800998ecf8427e' + ) # Verify that a specified key_prefix is taken into account. learn_cache_key(request, response, key_prefix=key_prefix) - self.assertEqual(get_cache_key(request, key_prefix=key_prefix), 'views.decorators.cache.cache_page.localprefix.GET.9fa0fd092afb73bdce204bb4f94d5804.d41d8cd98f00b204e9800998ecf8427e') + self.assertEqual( + get_cache_key(request, key_prefix=key_prefix), + 'views.decorators.cache.cache_page.localprefix.GET.' + '58a0a05c8a5620f813686ff969c26853.d41d8cd98f00b204e9800998ecf8427e' + ) def test_get_cache_key_with_query(self): request = self.factory.get(self.path, {'test': 1}) @@ -1887,7 +1937,11 @@ class TestWithTemplateResponse(TestCase): # Set headers to an empty list. learn_cache_key(request, response) # Verify that the querystring is taken into account. - self.assertEqual(get_cache_key(request), 'views.decorators.cache.cache_page.settingsprefix.GET.d11198ba31883732b0de5786a80cc12b.d41d8cd98f00b204e9800998ecf8427e') + self.assertEqual( + get_cache_key(request), + 'views.decorators.cache.cache_page.settingsprefix.GET.' + '0f1c2d56633c943073c4569d9a9502fe.d41d8cd98f00b204e9800998ecf8427e' + ) @override_settings(USE_ETAGS=False) def test_without_etag(self):