From 7a40fef17ab7918cbb1ddc3ba080f42b420f7a48 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Fri, 5 Jun 2015 14:26:48 +0100 Subject: [PATCH] Fixed #24935 -- Refactored common conditional GET handling. --- django/middleware/common.py | 23 +++---- django/middleware/http.py | 34 ++++------ django/utils/cache.py | 106 ++++++++++++++++++++++++++++++-- django/views/decorators/http.py | 76 +++-------------------- 4 files changed, 130 insertions(+), 109 deletions(-) diff --git a/django/middleware/common.py b/django/middleware/common.py index 6773eff614..376e82b939 100644 --- a/django/middleware/common.py +++ b/django/middleware/common.py @@ -1,4 +1,3 @@ -import hashlib import logging import re @@ -7,6 +6,7 @@ from django.conf import settings from django.core import urlresolvers from django.core.exceptions import PermissionDenied from django.core.mail import mail_managers +from django.utils.cache import get_conditional_response, set_response_etag from django.utils.encoding import force_text logger = logging.getLogger('django.request') @@ -113,20 +113,15 @@ class CommonMiddleware(object): return self.response_redirect_class(self.get_full_path_with_slash(request)) if settings.USE_ETAGS: + if not response.has_header('ETag'): + set_response_etag(response) + if response.has_header('ETag'): - etag = response['ETag'] - elif response.streaming: - etag = None - else: - etag = '"%s"' % hashlib.md5(response.content).hexdigest() - if etag is not None: - if (200 <= response.status_code < 300 - and request.META.get('HTTP_IF_NONE_MATCH') == etag): - cookies = response.cookies - response = http.HttpResponseNotModified() - response.cookies = cookies - else: - response['ETag'] = etag + return get_conditional_response( + request, + etag=response['ETag'], + response=response, + ) return response diff --git a/django/middleware/http.py b/django/middleware/http.py index 60fb194704..7a6f237c96 100644 --- a/django/middleware/http.py +++ b/django/middleware/http.py @@ -1,3 +1,4 @@ +from django.utils.cache import get_conditional_response from django.utils.http import http_date, parse_http_date_safe @@ -14,28 +15,17 @@ class ConditionalGetMiddleware(object): if not response.streaming and not response.has_header('Content-Length'): response['Content-Length'] = str(len(response.content)) - # 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 200 <= response.status_code < 300 and response.has_header('ETag'): - if_none_match = request.META.get('HTTP_IF_NONE_MATCH') - if if_none_match == response['ETag']: - # Setting the status is enough here. The response handling path - # automatically removes content for this status code (in - # http.conditional_content_removal()). - response.status_code = 304 + etag = response.get('ETag') + last_modified = response.get('Last-Modified') + if last_modified: + last_modified = parse_http_date_safe(last_modified) - # 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.status_code == 200 and response.has_header('Last-Modified'): - if_modified_since = request.META.get('HTTP_IF_MODIFIED_SINCE') - if if_modified_since is not None: - if_modified_since = parse_http_date_safe(if_modified_since) - if if_modified_since is not None: - last_modified = parse_http_date_safe(response['Last-Modified']) - if last_modified is not None and last_modified <= if_modified_since: - # Setting the status code is enough here (same reasons as - # above). - response.status_code = 304 + if etag or last_modified: + return get_conditional_response( + request, + etag=etag, + last_modified=last_modified, + response=response, + ) return response diff --git a/django/utils/cache.py b/django/utils/cache.py index 73d9c3dca5..c8ab96fe3d 100644 --- a/django/utils/cache.py +++ b/django/utils/cache.py @@ -19,18 +19,24 @@ An example: i18n middleware would need to distinguish caches by the from __future__ import unicode_literals import hashlib +import logging import re import time from django.conf import settings from django.core.cache import caches +from django.http import HttpResponse, HttpResponseNotModified from django.utils.encoding import force_bytes, force_text, iri_to_uri -from django.utils.http import http_date +from django.utils.http import ( + http_date, parse_etags, parse_http_date_safe, quote_etag, +) from django.utils.timezone import get_current_timezone_name from django.utils.translation import get_language cc_delim_re = re.compile(r'\s*,\s*') +logger = logging.getLogger('django.request') + def patch_cache_control(response, **kwargs): """ @@ -97,9 +103,99 @@ def get_max_age(response): pass -def _set_response_etag(response): +def set_response_etag(response): if not response.streaming: - response['ETag'] = '"%s"' % hashlib.md5(response.content).hexdigest() + response['ETag'] = quote_etag(hashlib.md5(response.content).hexdigest()) + return response + + +def _precondition_failed(request): + logger.warning('Precondition Failed: %s', request.path, + extra={ + 'status_code': 412, + 'request': request, + }, + ) + return HttpResponse(status=412) + + +def _not_modified(request, response=None): + if response: + # We need to keep the cookies, see ticket #4994. + cookies = response.cookies + response = HttpResponseNotModified() + response.cookies = cookies + return response + else: + return HttpResponseNotModified() + + +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) + 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 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 + + # 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 + + 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): + return _not_modified(request, response) + elif (not if_match and + last_modified and if_unmodified_since and + last_modified > if_unmodified_since): + return _precondition_failed(request) + return response @@ -119,9 +215,9 @@ def patch_response_headers(response, cache_timeout=None): cache_timeout = 0 # Can't have max-age negative if settings.USE_ETAGS and not response.has_header('ETag'): if hasattr(response, 'render') and callable(response.render): - response.add_post_render_callback(_set_response_etag) + response.add_post_render_callback(set_response_etag) else: - response = _set_response_etag(response) + response = set_response_etag(response) if not response.has_header('Last-Modified'): response['Last-Modified'] = http_date() if not response.has_header('Expires'): diff --git a/django/views/decorators/http.py b/django/views/decorators/http.py index c6cef1d22a..15e89176fd 100644 --- a/django/views/decorators/http.py +++ b/django/views/decorators/http.py @@ -6,14 +6,11 @@ import logging from calendar import timegm from functools import wraps -from django.http import ( - HttpResponse, HttpResponseNotAllowed, HttpResponseNotModified, -) +from django.http import HttpResponseNotAllowed from django.middleware.http import ConditionalGetMiddleware +from django.utils.cache import get_conditional_response from django.utils.decorators import available_attrs, decorator_from_middleware -from django.utils.http import ( - http_date, parse_etags, parse_http_date_safe, quote_etag, -) +from django.utils.http import http_date, quote_etag conditional_page = decorator_from_middleware(ConditionalGetMiddleware) @@ -56,16 +53,6 @@ require_safe = require_http_methods(["GET", "HEAD"]) require_safe.__doc__ = "Decorator to require that a view only accepts safe methods: GET and HEAD." -def _precondition_failed(request): - logger.warning('Precondition Failed: %s', request.path, - extra={ - 'status_code': 412, - 'request': request - }, - ) - return HttpResponse(status=412) - - def condition(etag_func=None, last_modified_func=None): """ Decorator to support conditional retrieval (or change) for a view @@ -91,29 +78,6 @@ def condition(etag_func=None, last_modified_func=None): def decorator(func): @wraps(func, assigned=available_attrs(func)) def inner(request, *args, **kwargs): - # 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) - 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 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 Django bug #10681. - if_none_match = None - if_match = None - # Compute values (if any) for the requested resource. def get_last_modified(): if last_modified_func: @@ -124,35 +88,11 @@ def condition(etag_func=None, last_modified_func=None): res_etag = etag_func(request, *args, **kwargs) if etag_func else None res_last_modified = get_last_modified() - response = None - 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 (res_etag in etags or - "*" in etags and res_etag)) and - (not if_modified_since or - (res_last_modified and if_modified_since and - res_last_modified <= if_modified_since))): - if request.method in ("GET", "HEAD"): - response = HttpResponseNotModified() - else: - response = _precondition_failed(request) - elif (if_match and ((not res_etag and "*" in etags) or - (res_etag and res_etag not in etags) or - (res_last_modified and if_unmodified_since and - res_last_modified > if_unmodified_since))): - response = _precondition_failed(request) - elif (not if_none_match and request.method in ("GET", "HEAD") and - res_last_modified and if_modified_since and - res_last_modified <= if_modified_since): - response = HttpResponseNotModified() - elif (not if_match and - res_last_modified and if_unmodified_since and - res_last_modified > if_unmodified_since): - response = _precondition_failed(request) + response = get_conditional_response( + request, + etag=res_etag, + last_modified=res_last_modified, + ) if response is None: response = func(request, *args, **kwargs)