From cda07b4cfaf92fe82f58d3d6ca6e64ea40502b0d Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 1 Feb 2011 00:31:34 +0000 Subject: [PATCH] [1.2.X] 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. Backport of r15381 from trunk. git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.2.X@15382 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/middleware/cache.py | 22 +++++++++++++--------- django/utils/cache.py | 10 ++++++++++ tests/regressiontests/cache/tests.py | 27 +++++++++++++++++++++++++-- tests/regressiontests/cache/urls.py | 5 +++++ tests/regressiontests/cache/views.py | 4 ++++ 5 files changed, 57 insertions(+), 11 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 3f602fe652e02..32d15b4095089 100644 --- a/django/middleware/cache.py +++ b/django/middleware/cache.py @@ -50,7 +50,8 @@ from django.conf import settings from django.core.cache import cache -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): """ @@ -66,9 +67,19 @@ def __init__(self): self.key_prefix = settings.CACHE_MIDDLEWARE_KEY_PREFIX self.cache_anonymous_only = getattr(settings, 'CACHE_MIDDLEWARE_ANONYMOUS_ONLY', False) + 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 request.method != 'GET': @@ -112,17 +123,10 @@ def process_request(self, request): 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. - cache_key = get_cache_key(request, self.key_prefix) if cache_key is None: request._cache_update_cache = True diff --git a/django/utils/cache.py b/django/utils/cache.py index 6cfd893668aea..58d087b0799c3 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 7167e2fbf82b9..e53194d82c6bc 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 unittest @@ -13,9 +12,10 @@ from django.conf import settings from django.core import management from django.core.cache import get_cache -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 +from django.test import TestCase from django.test.utils import get_warnings_state, restore_warnings_state from django.utils import translation from django.utils.cache import patch_vary_headers, get_cache_key, learn_cache_key @@ -648,5 +648,28 @@ def set_cache(request, lang, msg): get_cache_data = FetchFromCacheMiddleware().process_request(request) self.assertEqual(get_cache_data.content, es_message) + +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 0000000000000..b98447bfa3dd1 --- /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 0000000000000..9b72f03f5633c --- /dev/null +++ b/tests/regressiontests/cache/views.py @@ -0,0 +1,4 @@ +from django.http import HttpResponse + +def home(request): + return HttpResponse('Hello World!')