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 · Dec 28, 2013 · 71a03e0 · 71a03e0
1 parent 280c1a6
commit 71a03e0
Show file tree

Hide file tree

Showing 4 changed files with 92 additions and 19 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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 @@ def test_get_cache_key(self):
         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,15 +1263,35 @@ def test_get_cache_key_with_query(self):
         # 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)
         response = HttpResponse()
         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 @@ def test_get_cache_key(self):
         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 @@ def test_get_cache_key_with_query(self):
         # 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):