From 37c9b81ebc9af3b61345a070a143ee8330a119b4 Mon Sep 17 00:00:00 2001 From: Josh Schneier Date: Tue, 6 Jun 2017 15:37:14 -0400 Subject: [PATCH] Fixed #28104 -- Prevented condition decorator from setting ETag/Last-Modified headers for non-safe requests. --- django/views/decorators/http.py | 18 ++++++++++-------- docs/topics/conditional-view-processing.txt | 12 ++++++++++++ tests/conditional_processing/tests.py | 12 ++++++++---- 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/django/views/decorators/http.py b/django/views/decorators/http.py index f0eeeb00053..81355d7725b 100644 --- a/django/views/decorators/http.py +++ b/django/views/decorators/http.py @@ -70,9 +70,9 @@ def condition(etag_func=None, last_modified_func=None): This decorator will either pass control to the wrapped view function or return an HTTP 304 response (unmodified) or 412 response (precondition - failed), depending upon the request method. In either case, it will add the - generated ETag and Last-Modified headers to the response if it doesn't - already have them. + failed), depending upon the request method. In either case, the decorator + will add the generated ETag and Last-Modified headers to the response if + the headers aren't already set and if the request's method is safe. """ def decorator(func): @wraps(func) @@ -98,11 +98,13 @@ def condition(etag_func=None, last_modified_func=None): if response is None: response = func(request, *args, **kwargs) - # Set relevant headers on the response if they don't already exist. - if res_last_modified and not response.has_header('Last-Modified'): - response['Last-Modified'] = http_date(res_last_modified) - if res_etag and not response.has_header('ETag'): - response['ETag'] = res_etag + # Set relevant headers on the response if they don't already exist + # and if the request method is safe. + if request.method in ('GET', 'HEAD'): + if res_last_modified and not response.has_header('Last-Modified'): + response['Last-Modified'] = http_date(res_last_modified) + if res_etag and not response.has_header('ETag'): + response['ETag'] = res_etag return response diff --git a/docs/topics/conditional-view-processing.txt b/docs/topics/conditional-view-processing.txt index 02bf4635a82..935c33be34b 100644 --- a/docs/topics/conditional-view-processing.txt +++ b/docs/topics/conditional-view-processing.txt @@ -66,6 +66,10 @@ last time the resource was modified, or ``None`` if the resource doesn't exist. The function passed to the ``etag`` decorator should return a string representing the `ETag`_ for the resource, or ``None`` if it doesn't exist. +The decorator sets the ``ETag`` and ``Last-Modified`` headers on the response +if they are not already set by the view and if the request's method is safe +(``GET`` or ``HEAD``). + .. versionchanged:: 1.11 In older versions, the return value from ``etag_func()`` was interpreted as @@ -198,6 +202,14 @@ to compute the ETag and last modification values in all situations. In fact, you **should** use the same functions, so that the same values are returned every time. +.. admonition:: Validator headers with non-safe request methods + + The ``condition`` decorator only sets validator headers (``ETag`` and + ``Last-Modified``) for safe HTTP methods, i.e. ``GET`` and ``HEAD``. If you + wish to return them in other cases, set them in your view. See + :rfc:`7231#section-4.3.4` to learn about the distinction between setting a + validator header in response to requests made with ``PUT`` versus ``POST``. + Comparison with middleware conditional processing ================================================= diff --git a/tests/conditional_processing/tests.py b/tests/conditional_processing/tests.py index fd0b10cef41..349b1cf7fe3 100644 --- a/tests/conditional_processing/tests.py +++ b/tests/conditional_processing/tests.py @@ -19,10 +19,14 @@ class ConditionalGet(SimpleTestCase): def assertFullResponse(self, response, check_last_modified=True, check_etag=True): self.assertEqual(response.status_code, 200) self.assertEqual(response.content, FULL_RESPONSE.encode()) - if check_last_modified: - self.assertEqual(response['Last-Modified'], LAST_MODIFIED_STR) - if check_etag: - self.assertEqual(response['ETag'], ETAG) + if response.request['REQUEST_METHOD'] in ('GET', 'HEAD'): + if check_last_modified: + self.assertEqual(response['Last-Modified'], LAST_MODIFIED_STR) + if check_etag: + self.assertEqual(response['ETag'], ETAG) + else: + self.assertNotIn('Last-Modified', response) + self.assertNotIn('ETag', response) def assertNotModified(self, response): self.assertEqual(response.status_code, 304)