Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #13283 -- Corrected CACHE_MIDDLEWARE_ANONYMOUS_ONLY's bad habit…

… of setting Vary: Cookie on all responses and destroying cache efficiency. Thanks to natrius for the fix.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@15381 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 00fda7f45db2425c1dcb5d927093ede45734d841 1 parent 0bf5fbf
Carl Meyer authored February 01, 2011
21  django/middleware/cache.py
@@ -50,7 +50,7 @@
50 50
 
51 51
 from django.conf import settings
52 52
 from django.core.cache import get_cache, DEFAULT_CACHE_ALIAS
53  
-from django.utils.cache import get_cache_key, learn_cache_key, patch_response_headers, get_max_age
  53
+from django.utils.cache import get_cache_key, learn_cache_key, patch_response_headers, get_max_age, has_vary_header
54 54
 
55 55
 
56 56
 class UpdateCacheMiddleware(object):
@@ -69,9 +69,19 @@ def __init__(self):
69 69
         self.cache_alias = settings.CACHE_MIDDLEWARE_ALIAS
70 70
         self.cache = get_cache(self.cache_alias)
71 71
 
  72
+    def _should_update_cache(self, request, response):
  73
+        if not hasattr(request, '_cache_update_cache') or not request._cache_update_cache:
  74
+            return False
  75
+        if self.cache_anonymous_only and has_vary_header(response, 'Cookie'):
  76
+            assert hasattr(request, 'user'), "The Django cache middleware with CACHE_MIDDLEWARE_ANONYMOUS_ONLY=True requires authentication middleware to be installed. Edit your MIDDLEWARE_CLASSES setting to insert 'django.contrib.auth.middleware.AuthenticationMiddleware' before the CacheMiddleware."
  77
+            if request.user.is_authenticated():
  78
+                # Don't cache user-variable requests from authenticated users.
  79
+                return False
  80
+        return True
  81
+
72 82
     def process_response(self, request, response):
73 83
         """Sets the cache, if needed."""
74  
-        if not hasattr(request, '_cache_update_cache') or not request._cache_update_cache:
  84
+        if not self._should_update_cache(request, response):
75 85
             # We don't need to update the cache, just return.
76 86
             return response
77 87
         if not response.status_code == 200:
@@ -116,17 +126,10 @@ def process_request(self, request):
116 126
         Checks whether the page is already cached and returns the cached
117 127
         version if available.
118 128
         """
119  
-        if self.cache_anonymous_only:
120  
-            assert hasattr(request, 'user'), "The Django cache middleware with CACHE_MIDDLEWARE_ANONYMOUS_ONLY=True requires authentication middleware to be installed. Edit your MIDDLEWARE_CLASSES setting to insert 'django.contrib.auth.middleware.AuthenticationMiddleware' before the CacheMiddleware."
121  
-
122 129
         if not request.method in ('GET', 'HEAD') or request.GET:
123 130
             request._cache_update_cache = False
124 131
             return None # Don't bother checking the cache.
125 132
 
126  
-        if self.cache_anonymous_only and request.user.is_authenticated():
127  
-            request._cache_update_cache = False
128  
-            return None # Don't cache requests from authenticated users.
129  
-
130 133
         # try and get the cached GET response
131 134
         cache_key = get_cache_key(request, self.key_prefix, 'GET', cache=self.cache)
132 135
         if cache_key is None:
10  django/utils/cache.py
@@ -134,6 +134,16 @@ def patch_vary_headers(response, newheaders):
134 134
                           if newheader.lower() not in existing_headers]
135 135
     response['Vary'] = ', '.join(vary_headers + additional_headers)
136 136
 
  137
+def has_vary_header(response, header_query):
  138
+    """
  139
+    Checks to see if the response has a given header name in its Vary header.
  140
+    """
  141
+    if not response.has_header('Vary'):
  142
+        return False
  143
+    vary_headers = cc_delim_re.split(response['Vary'])
  144
+    existing_headers = set([header.lower() for header in vary_headers])
  145
+    return header_query.lower() in existing_headers
  146
+
137 147
 def _i18n_cache_key_suffix(request, cache_key):
138 148
     """If enabled, returns the cache key ending with a locale."""
139 149
     if settings.USE_I18N:
26  tests/regressiontests/cache/tests.py
@@ -4,7 +4,6 @@
4 4
 # Uses whatever cache backend is set in the test settings file.
5 5
 
6 6
 import os
7  
-import shutil
8 7
 import tempfile
9 8
 import time
10 9
 import warnings
@@ -12,10 +11,10 @@
12 11
 from django.conf import settings
13 12
 from django.core import management
14 13
 from django.core.cache import get_cache, DEFAULT_CACHE_ALIAS
15  
-from django.core.cache.backends.base import InvalidCacheBackendError, CacheKeyWarning
  14
+from django.core.cache.backends.base import CacheKeyWarning
16 15
 from django.http import HttpResponse, HttpRequest
17 16
 from django.middleware.cache import FetchFromCacheMiddleware, UpdateCacheMiddleware, CacheMiddleware
18  
-from django.test import RequestFactory
  17
+from django.test import RequestFactory, TestCase
19 18
 from django.test.utils import get_warnings_state, restore_warnings_state
20 19
 from django.utils import translation
21 20
 from django.utils import unittest
@@ -1323,5 +1322,26 @@ def view(request, value):
1323 1322
         response = other_with_timeout_view(request, '18')
1324 1323
         self.assertEquals(response.content, 'Hello World 18')
1325 1324
 
  1325
+class CacheMiddlewareAnonymousOnlyTests(TestCase):
  1326
+    urls = 'regressiontests.cache.urls'
  1327
+
  1328
+    def setUp(self):
  1329
+        self._orig_cache_middleware_anonymous_only = \
  1330
+            getattr(settings, 'CACHE_MIDDLEWARE_ANONYMOUS_ONLY', False)
  1331
+        self._orig_middleware_classes = settings.MIDDLEWARE_CLASSES
  1332
+
  1333
+        settings.MIDDLEWARE_CLASSES = list(settings.MIDDLEWARE_CLASSES)
  1334
+        settings.MIDDLEWARE_CLASSES.insert(0, 'django.middleware.cache.UpdateCacheMiddleware')
  1335
+        settings.MIDDLEWARE_CLASSES += ['django.middleware.cache.FetchFromCacheMiddleware']
  1336
+
  1337
+    def tearDown(self):
  1338
+        settings.CACHE_MIDDLEWARE_ANONYMOUS_ONLY = self._orig_cache_middleware_anonymous_only
  1339
+        settings.MIDDLEWARE_CLASSES = self._orig_middleware_classes
  1340
+
  1341
+    def test_cache_middleware_anonymous_only_does_not_cause_vary_cookie(self):
  1342
+        settings.CACHE_MIDDLEWARE_ANONYMOUS_ONLY = True
  1343
+        response = self.client.get('/')
  1344
+        self.failIf('Cookie' in response.get('Vary', ''))
  1345
+
1326 1346
 if __name__ == '__main__':
1327 1347
     unittest.main()
5  tests/regressiontests/cache/urls.py
... ...
@@ -0,0 +1,5 @@
  1
+from django.conf.urls.defaults import patterns
  2
+
  3
+urlpatterns = patterns('regressiontests.cache.views',
  4
+    (r'^$', 'home'),
  5
+)
4  tests/regressiontests/cache/views.py
... ...
@@ -0,0 +1,4 @@
  1
+from django.http import HttpResponse
  2
+
  3
+def home(request):
  4
+    return HttpResponse('Hello World!')

0 notes on commit 00fda7f

Please sign in to comment.
Something went wrong with that request. Please try again.