From 00fda7f45db2425c1dcb5d927093ede45734d841 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 1 Feb 2011 00:20:31 +0000 Subject: [PATCH] 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 --- django/middleware/cache.py | 21 ++++++++++++--------- django/utils/cache.py | 10 ++++++++++ tests/regressiontests/cache/tests.py | 26 +++++++++++++++++++++++--- tests/regressiontests/cache/urls.py | 5 +++++ tests/regressiontests/cache/views.py | 4 ++++ 5 files changed, 54 insertions(+), 12 deletions(-) create mode 100644 tests/regressiontests/cache/urls.py create mode 100644 tests/regressiontests/cache/views.py diff --git a/django/middleware/cache.py b/django/middleware/cache.py index ab72db3c18..7789c3492c 100644 --- a/django/middleware/cache.py +++ b/django/middleware/cache.py @@ -50,7 +50,7 @@ More details about how the caching works: from django.conf import settings from django.core.cache import get_cache, DEFAULT_CACHE_ALIAS -from django.utils.cache import get_cache_key, learn_cache_key, patch_response_headers, get_max_age +from django.utils.cache import get_cache_key, learn_cache_key, patch_response_headers, get_max_age, has_vary_header class UpdateCacheMiddleware(object): @@ -69,9 +69,19 @@ class UpdateCacheMiddleware(object): self.cache_alias = settings.CACHE_MIDDLEWARE_ALIAS self.cache = get_cache(self.cache_alias) + def _should_update_cache(self, request, response): + if not hasattr(request, '_cache_update_cache') or not request._cache_update_cache: + return False + if self.cache_anonymous_only and has_vary_header(response, 'Cookie'): + 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." + if request.user.is_authenticated(): + # Don't cache user-variable requests from authenticated users. + return False + return True + def process_response(self, request, response): """Sets the cache, if needed.""" - if not hasattr(request, '_cache_update_cache') or not request._cache_update_cache: + if not self._should_update_cache(request, response): # We don't need to update the cache, just return. return response if not response.status_code == 200: @@ -116,17 +126,10 @@ class FetchFromCacheMiddleware(object): Checks whether the page is already cached and returns the cached version if available. """ - if self.cache_anonymous_only: - 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." - if not request.method in ('GET', 'HEAD') or request.GET: request._cache_update_cache = False return None # Don't bother checking the cache. - if self.cache_anonymous_only and request.user.is_authenticated(): - request._cache_update_cache = False - return None # Don't cache requests from authenticated users. - # try and get the cached GET response cache_key = get_cache_key(request, self.key_prefix, 'GET', cache=self.cache) if cache_key is None: diff --git a/django/utils/cache.py b/django/utils/cache.py index ba647ac420..4b3b5af04c 100644 --- a/django/utils/cache.py +++ b/django/utils/cache.py @@ -134,6 +134,16 @@ def patch_vary_headers(response, newheaders): if newheader.lower() not in existing_headers] response['Vary'] = ', '.join(vary_headers + additional_headers) +def has_vary_header(response, header_query): + """ + Checks to see if the response has a given header name in its Vary header. + """ + if not response.has_header('Vary'): + return False + vary_headers = cc_delim_re.split(response['Vary']) + existing_headers = set([header.lower() for header in vary_headers]) + return header_query.lower() in existing_headers + def _i18n_cache_key_suffix(request, cache_key): """If enabled, returns the cache key ending with a locale.""" if settings.USE_I18N: diff --git a/tests/regressiontests/cache/tests.py b/tests/regressiontests/cache/tests.py index 1a1e2438bd..7d520c0ed2 100644 --- a/tests/regressiontests/cache/tests.py +++ b/tests/regressiontests/cache/tests.py @@ -4,7 +4,6 @@ # Uses whatever cache backend is set in the test settings file. import os -import shutil import tempfile import time import warnings @@ -12,10 +11,10 @@ import warnings from django.conf import settings from django.core import management from django.core.cache import get_cache, DEFAULT_CACHE_ALIAS -from django.core.cache.backends.base import InvalidCacheBackendError, CacheKeyWarning +from django.core.cache.backends.base import CacheKeyWarning from django.http import HttpResponse, HttpRequest from django.middleware.cache import FetchFromCacheMiddleware, UpdateCacheMiddleware, CacheMiddleware -from django.test import RequestFactory +from django.test import RequestFactory, TestCase from django.test.utils import get_warnings_state, restore_warnings_state from django.utils import translation from django.utils import unittest @@ -1323,5 +1322,26 @@ class CacheMiddlewareTest(unittest.TestCase): response = other_with_timeout_view(request, '18') self.assertEquals(response.content, 'Hello World 18') +class CacheMiddlewareAnonymousOnlyTests(TestCase): + urls = 'regressiontests.cache.urls' + + def setUp(self): + self._orig_cache_middleware_anonymous_only = \ + getattr(settings, 'CACHE_MIDDLEWARE_ANONYMOUS_ONLY', False) + self._orig_middleware_classes = settings.MIDDLEWARE_CLASSES + + settings.MIDDLEWARE_CLASSES = list(settings.MIDDLEWARE_CLASSES) + settings.MIDDLEWARE_CLASSES.insert(0, 'django.middleware.cache.UpdateCacheMiddleware') + settings.MIDDLEWARE_CLASSES += ['django.middleware.cache.FetchFromCacheMiddleware'] + + def tearDown(self): + settings.CACHE_MIDDLEWARE_ANONYMOUS_ONLY = self._orig_cache_middleware_anonymous_only + settings.MIDDLEWARE_CLASSES = self._orig_middleware_classes + + def test_cache_middleware_anonymous_only_does_not_cause_vary_cookie(self): + settings.CACHE_MIDDLEWARE_ANONYMOUS_ONLY = True + response = self.client.get('/') + self.failIf('Cookie' in response.get('Vary', '')) + if __name__ == '__main__': unittest.main() diff --git a/tests/regressiontests/cache/urls.py b/tests/regressiontests/cache/urls.py new file mode 100644 index 0000000000..b98447bfa3 --- /dev/null +++ b/tests/regressiontests/cache/urls.py @@ -0,0 +1,5 @@ +from django.conf.urls.defaults import patterns + +urlpatterns = patterns('regressiontests.cache.views', + (r'^$', 'home'), +) diff --git a/tests/regressiontests/cache/views.py b/tests/regressiontests/cache/views.py new file mode 100644 index 0000000000..9b72f03f56 --- /dev/null +++ b/tests/regressiontests/cache/views.py @@ -0,0 +1,4 @@ +from django.http import HttpResponse + +def home(request): + return HttpResponse('Hello World!')