Fixed #24935 -- Refactored common conditional GET handling.

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)