From 22e303887b7f807b39239880e33b9018566e0137 Mon Sep 17 00:00:00 2001 From: Kevin Christopher Henry Date: Mon, 12 Sep 2016 23:26:24 -0400 Subject: [PATCH] Refs #27083 -- Updated conditional header comparison to match RFC 7232. --- django/utils/cache.py | 138 ++++++++++++++++---------- docs/releases/1.11.txt | 5 +- tests/conditional_processing/tests.py | 84 +++++++++++++--- tests/conditional_processing/urls.py | 1 + tests/conditional_processing/views.py | 10 +- 5 files changed, 166 insertions(+), 72 deletions(-) diff --git a/django/utils/cache.py b/django/utils/cache.py index ad732327e84..ac469195daa 100644 --- a/django/utils/cache.py +++ b/django/utils/cache.py @@ -131,72 +131,102 @@ def _not_modified(request, response=None): def get_conditional_response(request, etag=None, last_modified=None, response=None): - # Get HTTP request headers - if_modified_since = request.META.get('HTTP_IF_MODIFIED_SINCE') - if if_modified_since: - if_modified_since = parse_http_date_safe(if_modified_since) + # Only return conditional responses on successful requests. + if response and not (200 <= response.status_code < 300): + return response + + # Get HTTP request headers. + if_match_etags = parse_etags(request.META.get('HTTP_IF_MATCH', '')) if_unmodified_since = request.META.get('HTTP_IF_UNMODIFIED_SINCE') if if_unmodified_since: if_unmodified_since = parse_http_date_safe(if_unmodified_since) - if_none_match = request.META.get('HTTP_IF_NONE_MATCH') - if_match = request.META.get('HTTP_IF_MATCH') - etags = [] - if if_none_match or if_match: - # There can be more than one ETag in the request, so we - # consider the list of values. - try: - etags = parse_etags(if_none_match or if_match) - except ValueError: - # In case of an invalid ETag, ignore all ETag headers. - # Apparently Opera sends invalidly quoted headers at times - # (we should be returning a 400 response, but that's a - # little extreme) -- this is bug #10681. - if_none_match = None - if_match = None + if_none_match_etags = parse_etags(request.META.get('HTTP_IF_NONE_MATCH', '')) + if_modified_since = request.META.get('HTTP_IF_MODIFIED_SINCE') + if if_modified_since: + if_modified_since = parse_http_date_safe(if_modified_since) - # If-None-Match must be ignored if original result would be anything - # other than a 2XX or 304 status. 304 status would result in no change. - # http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.26 - if response and not (200 <= response.status_code < 300): - if_none_match = None - if_match = None + # Step 1 of section 6 of RFC 7232: Test the If-Match precondition. + if (if_match_etags and not _if_match_passes(etag, if_match_etags)): + return _precondition_failed(request) - # If-Modified-Since must be ignored if the original result was not a 200. - # http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.25 - if response and response.status_code != 200: - if_modified_since = None - if_unmodified_since = None + # Step 2: Test the If-Unmodified-Since precondition. + if (not if_match_etags and if_unmodified_since and + not _if_unmodified_since_passes(last_modified, if_unmodified_since)): + return _precondition_failed(request) - if not ((if_match and if_modified_since) or - (if_none_match and if_unmodified_since) or - (if_modified_since and if_unmodified_since) or - (if_match and if_none_match)): - # We only get here if no undefined combinations of headers are - # specified. - if ((if_none_match and (etag in etags or '*' in etags and etag)) and - (not if_modified_since or - (last_modified and if_modified_since and last_modified <= if_modified_since))): - if request.method in ('GET', 'HEAD'): - return _not_modified(request, response) - else: - return _precondition_failed(request) - elif (if_match and ( - (not etag and '*' in etags) or (etag and etag not in etags) or - (last_modified and if_unmodified_since and last_modified > if_unmodified_since) - )): - return _precondition_failed(request) - elif (not if_none_match and request.method in ('GET', 'HEAD') and - last_modified and if_modified_since and - last_modified <= if_modified_since): + # Step 3: Test the If-None-Match precondition. + if (if_none_match_etags and not _if_none_match_passes(etag, if_none_match_etags)): + if request.method in ('GET', 'HEAD'): return _not_modified(request, response) - elif (not if_match and - last_modified and if_unmodified_since and - last_modified > if_unmodified_since): + else: return _precondition_failed(request) + # Step 4: Test the If-Modified-Since precondition. + if (not if_none_match_etags and if_modified_since and + not _if_modified_since_passes(last_modified, if_modified_since)): + if request.method in ('GET', 'HEAD'): + return _not_modified(request, response) + + # Step 5: Test the If-Range precondition (not supported). + # Step 6: Return original response since there isn't a conditional response. return response +def _if_match_passes(target_etag, etags): + """ + Test the If-Match comparison as defined in section 3.1 of RFC 7232. + """ + if not target_etag: + # If there isn't an ETag, then there can't be a match. + return False + elif etags == ['*']: + # The existence of an ETag means that there is "a current + # representation for the target resource", even if the ETag is weak, + # so there is a match to '*'. + return True + elif target_etag.startswith('W/'): + # A weak ETag can never strongly match another ETag. + return False + else: + # Since the ETag is strong, this will only return True if there's a + # strong match. + return target_etag in etags + + +def _if_unmodified_since_passes(last_modified, if_unmodified_since): + """ + Test the If-Unmodified-Since comparison as defined in section 3.4 of + RFC 7232. + """ + return last_modified and last_modified <= if_unmodified_since + + +def _if_none_match_passes(target_etag, etags): + """ + Test the If-None-Match comparison as defined in section 3.2 of RFC 7232. + """ + if not target_etag: + # If there isn't an ETag, then there isn't a match. + return True + elif etags == ['*']: + # The existence of an ETag means that there is "a current + # representation for the target resource", so there is a match to '*'. + return False + else: + # The comparison should be weak, so look for a match after stripping + # off any weak indicators. + target_etag = target_etag.strip('W/') + etags = (etag.strip('W/') for etag in etags) + return target_etag not in etags + + +def _if_modified_since_passes(last_modified, if_modified_since): + """ + Test the If-Modified-Since comparison as defined in section 3.3 of RFC 7232. + """ + return not last_modified or last_modified > if_modified_since + + def patch_response_headers(response, cache_timeout=None): """ Adds some useful headers to the given HttpResponse object: diff --git a/docs/releases/1.11.txt b/docs/releases/1.11.txt index af3c5159db3..307ed9ab520 100644 --- a/docs/releases/1.11.txt +++ b/docs/releases/1.11.txt @@ -513,8 +513,9 @@ Miscellaneous * The admin's widget for ``IntegerField`` uses ``type="number"`` rather than ``type="text"``. -* ETags are now parsed according to the :rfc:`7232` Conditional Requests - specification rather than the syntax from :rfc:`2616`. +* Conditional HTTP headers are now parsed and compared according to the + :rfc:`7232` Conditional Requests specification rather than the older + :rfc:`2616`. * In the admin templates, ``

`` is replaced with a ``

`` tag to allow including lists inside help text. diff --git a/tests/conditional_processing/tests.py b/tests/conditional_processing/tests.py index 2faacbf6e20..e4af9447e02 100644 --- a/tests/conditional_processing/tests.py +++ b/tests/conditional_processing/tests.py @@ -12,6 +12,7 @@ LAST_MODIFIED_NEWER_STR = 'Mon, 18 Oct 2010 16:56:23 GMT' LAST_MODIFIED_INVALID_STR = 'Mon, 32 Oct 2010 16:56:23 GMT' EXPIRED_LAST_MODIFIED_STR = 'Sat, 20 Oct 2007 23:21:47 GMT' ETAG = '"b4246ffc4f62314ca13147c9d4f76974"' +WEAK_ETAG = 'W/"b4246ffc4f62314ca13147c9d4f76974"' # weak match to ETAG EXPIRED_ETAG = '"7fae4cd4b0f81e7d2914700043aa8ed6"' @@ -38,9 +39,13 @@ class ConditionalGet(SimpleTestCase): self.client.defaults['HTTP_IF_MODIFIED_SINCE'] = LAST_MODIFIED_STR response = self.client.get('/condition/') self.assertNotModified(response) + response = self.client.put('/condition/') + self.assertFullResponse(response) self.client.defaults['HTTP_IF_MODIFIED_SINCE'] = LAST_MODIFIED_NEWER_STR response = self.client.get('/condition/') self.assertNotModified(response) + response = self.client.put('/condition/') + self.assertFullResponse(response) self.client.defaults['HTTP_IF_MODIFIED_SINCE'] = LAST_MODIFIED_INVALID_STR response = self.client.get('/condition/') self.assertFullResponse(response) @@ -66,6 +71,8 @@ class ConditionalGet(SimpleTestCase): self.client.defaults['HTTP_IF_NONE_MATCH'] = ETAG response = self.client.get('/condition/') self.assertNotModified(response) + response = self.client.put('/condition/') + self.assertEqual(response.status_code, 412) self.client.defaults['HTTP_IF_NONE_MATCH'] = EXPIRED_ETAG response = self.client.get('/condition/') self.assertFullResponse(response) @@ -75,16 +82,68 @@ class ConditionalGet(SimpleTestCase): response = self.client.get('/condition/') self.assertNotModified(response) + def test_weak_if_none_match(self): + """ + If-None-Match comparisons use weak matching, so weak and strong ETags + with the same value result in a 304 response. + """ + self.client.defaults['HTTP_IF_NONE_MATCH'] = ETAG + response = self.client.get('/condition/weak_etag/') + self.assertNotModified(response) + response = self.client.put('/condition/weak_etag/') + self.assertEqual(response.status_code, 412) + + self.client.defaults['HTTP_IF_NONE_MATCH'] = WEAK_ETAG + response = self.client.get('/condition/weak_etag/') + self.assertNotModified(response) + response = self.client.put('/condition/weak_etag/') + self.assertEqual(response.status_code, 412) + response = self.client.get('/condition/') + self.assertNotModified(response) + response = self.client.put('/condition/') + self.assertEqual(response.status_code, 412) + + def test_all_if_none_match(self): + self.client.defaults['HTTP_IF_NONE_MATCH'] = '*' + response = self.client.get('/condition/') + self.assertNotModified(response) + response = self.client.put('/condition/') + self.assertEqual(response.status_code, 412) + response = self.client.get('/condition/no_etag/') + self.assertFullResponse(response, check_last_modified=False, check_etag=False) + def test_if_match(self): self.client.defaults['HTTP_IF_MATCH'] = ETAG - response = self.client.put('/condition/etag/') - self.assertEqual(response.status_code, 200) + response = self.client.put('/condition/') + self.assertFullResponse(response) self.client.defaults['HTTP_IF_MATCH'] = EXPIRED_ETAG - response = self.client.put('/condition/etag/') + response = self.client.put('/condition/') + self.assertEqual(response.status_code, 412) + + def test_weak_if_match(self): + """ + If-Match comparisons use strong matching, so any comparison involving + a weak ETag return a 412 response. + """ + self.client.defaults['HTTP_IF_MATCH'] = ETAG + response = self.client.get('/condition/weak_etag/') + self.assertEqual(response.status_code, 412) + + self.client.defaults['HTTP_IF_MATCH'] = WEAK_ETAG + response = self.client.get('/condition/weak_etag/') + self.assertEqual(response.status_code, 412) + response = self.client.get('/condition/') + self.assertEqual(response.status_code, 412) + + def test_all_if_match(self): + self.client.defaults['HTTP_IF_MATCH'] = '*' + response = self.client.get('/condition/') + self.assertFullResponse(response) + response = self.client.get('/condition/no_etag/') self.assertEqual(response.status_code, 412) def test_both_headers(self): - # see http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.3.4 + # see https://tools.ietf.org/html/rfc7232#section-6 self.client.defaults['HTTP_IF_MODIFIED_SINCE'] = LAST_MODIFIED_STR self.client.defaults['HTTP_IF_NONE_MATCH'] = ETAG response = self.client.get('/condition/') @@ -93,7 +152,7 @@ class ConditionalGet(SimpleTestCase): self.client.defaults['HTTP_IF_MODIFIED_SINCE'] = EXPIRED_LAST_MODIFIED_STR self.client.defaults['HTTP_IF_NONE_MATCH'] = ETAG response = self.client.get('/condition/') - self.assertFullResponse(response) + self.assertNotModified(response) self.client.defaults['HTTP_IF_MODIFIED_SINCE'] = LAST_MODIFIED_STR self.client.defaults['HTTP_IF_NONE_MATCH'] = EXPIRED_ETAG @@ -114,7 +173,7 @@ class ConditionalGet(SimpleTestCase): self.client.defaults['HTTP_IF_UNMODIFIED_SINCE'] = EXPIRED_LAST_MODIFIED_STR self.client.defaults['HTTP_IF_MATCH'] = ETAG response = self.client.get('/condition/') - self.assertEqual(response.status_code, 412) + self.assertFullResponse(response) self.client.defaults['HTTP_IF_UNMODIFIED_SINCE'] = EXPIRED_LAST_MODIFIED_STR self.client.defaults['HTTP_IF_MATCH'] = EXPIRED_ETAG @@ -169,7 +228,7 @@ class ConditionalGet(SimpleTestCase): response = self.client.get('/condition/last_modified/') self.assertEqual(response.status_code, 412) response = self.client.get('/condition/etag/') - self.assertFullResponse(response, check_last_modified=False) + self.assertEqual(response.status_code, 412) def test_single_condition_8(self): self.client.defaults['HTTP_IF_UNMODIFIED_SINCE'] = LAST_MODIFIED_STR @@ -181,7 +240,7 @@ class ConditionalGet(SimpleTestCase): response = self.client.get('/condition/last_modified2/') self.assertEqual(response.status_code, 412) response = self.client.get('/condition/etag2/') - self.assertFullResponse(response, check_last_modified=False) + self.assertEqual(response.status_code, 412) def test_single_condition_head(self): self.client.defaults['HTTP_IF_MODIFIED_SINCE'] = LAST_MODIFIED_STR @@ -204,17 +263,12 @@ class ConditionalGet(SimpleTestCase): self.client.defaults['HTTP_IF_NONE_MATCH'] = ETAG response = self.client.get('/condition/unquoted_etag/') self.assertNotModified(response) + response = self.client.put('/condition/unquoted_etag/') + self.assertEqual(response.status_code, 412) self.client.defaults['HTTP_IF_NONE_MATCH'] = EXPIRED_ETAG response = self.client.get('/condition/unquoted_etag/') self.assertFullResponse(response, check_last_modified=False) - def test_all_if_none_match(self): - self.client.defaults['HTTP_IF_NONE_MATCH'] = '*' - response = self.client.get('/condition/etag/') - self.assertNotModified(response) - response = self.client.get('/condition/no_etag/') - self.assertFullResponse(response, check_last_modified=False, check_etag=False) - def test_invalid_etag(self): self.client.defaults['HTTP_IF_NONE_MATCH'] = '"""' response = self.client.get('/condition/etag/') diff --git a/tests/conditional_processing/urls.py b/tests/conditional_processing/urls.py index c49835264ea..4b092a5ae15 100644 --- a/tests/conditional_processing/urls.py +++ b/tests/conditional_processing/urls.py @@ -9,5 +9,6 @@ urlpatterns = [ url('^condition/etag/$', views.etag_view1), url('^condition/etag2/$', views.etag_view2), url('^condition/unquoted_etag/$', views.etag_view_unquoted), + url('^condition/weak_etag/$', views.etag_view_weak), url('^condition/no_etag/$', views.etag_view_none), ] diff --git a/tests/conditional_processing/views.py b/tests/conditional_processing/views.py index 47288dbc563..2de57abc943 100644 --- a/tests/conditional_processing/views.py +++ b/tests/conditional_processing/views.py @@ -1,7 +1,7 @@ from django.http import HttpResponse from django.views.decorators.http import condition, etag, last_modified -from .tests import ETAG, FULL_RESPONSE, LAST_MODIFIED +from .tests import ETAG, FULL_RESPONSE, LAST_MODIFIED, WEAK_ETAG @condition(lambda r: ETAG, lambda r: LAST_MODIFIED) @@ -37,6 +37,14 @@ def etag_view_unquoted(request): return HttpResponse(FULL_RESPONSE) +@condition(etag_func=lambda r: WEAK_ETAG) +def etag_view_weak(request): + """ + Use an etag_func() that returns a weak ETag. + """ + return HttpResponse(FULL_RESPONSE) + + @condition(etag_func=lambda r: None) def etag_view_none(request): """