Fixed #5898 -- Changed a few response processing paths to make things harder to get wrong and easier to get right. Previous behaviour wasn't buggy, but it was harder to use than necessary.
We now have automatic HEAD processing always (previously required ConditionalGetMiddleware), middleware benefits from the Location header rewrite, so they can use relative URLs as well, and responses with response codes 1xx, 204 or 304 will always have their content removed, in accordance with the HTTP spec (so it's much harder to indavertently deliver invalid responses). Based on a patch and diagnosis from regexbot@gmail.com. git-svn-id: http://code.djangoproject.com/svn/django/trunk@6662 bcc190cf-cafb-0310-a4f2-bffc1f526a37
This commit is contained in:
parent
30848dfe34
commit
3ee3d6b5f3
|
@ -4,6 +4,10 @@ from django import http
|
||||||
import sys
|
import sys
|
||||||
|
|
||||||
class BaseHandler(object):
|
class BaseHandler(object):
|
||||||
|
# Changes that are always applied to a response (in this order).
|
||||||
|
response_fixes = [http.fix_location_header,
|
||||||
|
http.conditional_content_removal]
|
||||||
|
|
||||||
def __init__(self):
|
def __init__(self):
|
||||||
self._request_middleware = self._view_middleware = self._response_middleware = self._exception_middleware = None
|
self._request_middleware = self._view_middleware = self._response_middleware = self._exception_middleware = None
|
||||||
|
|
||||||
|
@ -50,10 +54,6 @@ class BaseHandler(object):
|
||||||
|
|
||||||
def get_response(self, request):
|
def get_response(self, request):
|
||||||
"Returns an HttpResponse object for the given HttpRequest"
|
"Returns an HttpResponse object for the given HttpRequest"
|
||||||
response = self._real_get_response(request)
|
|
||||||
return fix_location_header(request, response)
|
|
||||||
|
|
||||||
def _real_get_response(self, request):
|
|
||||||
from django.core import exceptions, urlresolvers
|
from django.core import exceptions, urlresolvers
|
||||||
from django.core.mail import mail_admins
|
from django.core.mail import mail_admins
|
||||||
from django.conf import settings
|
from django.conf import settings
|
||||||
|
@ -134,15 +134,13 @@ class BaseHandler(object):
|
||||||
import traceback
|
import traceback
|
||||||
return '\n'.join(traceback.format_exception(*(exc_info or sys.exc_info())))
|
return '\n'.join(traceback.format_exception(*(exc_info or sys.exc_info())))
|
||||||
|
|
||||||
def fix_location_header(request, response):
|
def apply_response_fixes(self, request, response):
|
||||||
"""
|
"""
|
||||||
Ensure that we always use an absolute URI in any location header in the
|
Applies each of the functions in self.response_fixes to the request and
|
||||||
response. This is required by RFC 2616, section 14.30.
|
response, modifying the response in the process. Returns the new
|
||||||
|
response.
|
||||||
Code constructing response objects is free to insert relative paths and
|
|
||||||
this function converts them to absolute paths.
|
|
||||||
"""
|
"""
|
||||||
if 'Location' in response and request.get_host():
|
for func in self.response_fixes:
|
||||||
response['Location'] = request.build_absolute_uri(response['Location'])
|
response = func(request, response)
|
||||||
return response
|
return response
|
||||||
|
|
||||||
|
|
|
@ -162,6 +162,7 @@ class ModPythonHandler(BaseHandler):
|
||||||
# Apply response middleware
|
# Apply response middleware
|
||||||
for middleware_method in self._response_middleware:
|
for middleware_method in self._response_middleware:
|
||||||
response = middleware_method(request, response)
|
response = middleware_method(request, response)
|
||||||
|
response = self.apply_response_fixes(request, response)
|
||||||
finally:
|
finally:
|
||||||
dispatcher.send(signal=signals.request_finished)
|
dispatcher.send(signal=signals.request_finished)
|
||||||
|
|
||||||
|
|
|
@ -207,6 +207,7 @@ class WSGIHandler(BaseHandler):
|
||||||
# Apply response middleware
|
# Apply response middleware
|
||||||
for middleware_method in self._response_middleware:
|
for middleware_method in self._response_middleware:
|
||||||
response = middleware_method(request, response)
|
response = middleware_method(request, response)
|
||||||
|
response = self.apply_response_fixes(request, response)
|
||||||
finally:
|
finally:
|
||||||
dispatcher.send(signal=signals.request_finished)
|
dispatcher.send(signal=signals.request_finished)
|
||||||
|
|
||||||
|
@ -220,3 +221,4 @@ class WSGIHandler(BaseHandler):
|
||||||
response_headers.append(('Set-Cookie', str(c.output(header=''))))
|
response_headers.append(('Set-Cookie', str(c.output(header=''))))
|
||||||
start_response(status, response_headers)
|
start_response(status, response_headers)
|
||||||
return response
|
return response
|
||||||
|
|
||||||
|
|
|
@ -5,6 +5,7 @@ from urllib import urlencode
|
||||||
from urlparse import urljoin
|
from urlparse import urljoin
|
||||||
from django.utils.datastructures import MultiValueDict, FileDict
|
from django.utils.datastructures import MultiValueDict, FileDict
|
||||||
from django.utils.encoding import smart_str, iri_to_uri, force_unicode
|
from django.utils.encoding import smart_str, iri_to_uri, force_unicode
|
||||||
|
from utils import *
|
||||||
|
|
||||||
RESERVED_CHARS="!*'();:@&=+$,/?%#[]"
|
RESERVED_CHARS="!*'();:@&=+$,/?%#[]"
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,34 @@
|
||||||
|
"""
|
||||||
|
Functions that modify an HTTP request or response in some way.
|
||||||
|
"""
|
||||||
|
|
||||||
|
# This group of functions are run as part of the response handling, after
|
||||||
|
# everything else, including all response middleware. Think of them as
|
||||||
|
# "compulsory response middleware". Be careful about what goes here, because
|
||||||
|
# it's a little fiddly to override this behaviour, so they should be truly
|
||||||
|
# universally applicable.
|
||||||
|
|
||||||
|
def fix_location_header(request, response):
|
||||||
|
"""
|
||||||
|
Ensures that we always use an absolute URI in any location header in the
|
||||||
|
response. This is required by RFC 2616, section 14.30.
|
||||||
|
|
||||||
|
Code constructing response objects is free to insert relative paths and
|
||||||
|
this function converts them to absolute paths.
|
||||||
|
"""
|
||||||
|
if 'Location' in response and request.get_host():
|
||||||
|
response['Location'] = request.build_absolute_uri(response['Location'])
|
||||||
|
return response
|
||||||
|
|
||||||
|
def conditional_content_removal(request, response):
|
||||||
|
"""
|
||||||
|
Removes the content of responses for HEAD requests, 1xx, 204 and 304
|
||||||
|
responses. Ensures compliance with RFC 2616, section 4.3.
|
||||||
|
"""
|
||||||
|
if 100 <= response.status_code < 200 or response.status_code in (204, 304):
|
||||||
|
response.content = ''
|
||||||
|
response['Content-Length'] = 0
|
||||||
|
if request.method == 'HEAD':
|
||||||
|
response.content = ''
|
||||||
|
return response
|
||||||
|
|
|
@ -6,8 +6,6 @@ class ConditionalGetMiddleware(object):
|
||||||
Last-Modified header, and the request has If-None-Match or
|
Last-Modified header, and the request has If-None-Match or
|
||||||
If-Modified-Since, the response is replaced by an HttpNotModified.
|
If-Modified-Since, the response is replaced by an HttpNotModified.
|
||||||
|
|
||||||
Removes the content from any response to a HEAD request.
|
|
||||||
|
|
||||||
Also sets the Date and Content-Length response-headers.
|
Also sets the Date and Content-Length response-headers.
|
||||||
"""
|
"""
|
||||||
def process_response(self, request, response):
|
def process_response(self, request, response):
|
||||||
|
@ -18,19 +16,17 @@ class ConditionalGetMiddleware(object):
|
||||||
if response.has_header('ETag'):
|
if response.has_header('ETag'):
|
||||||
if_none_match = request.META.get('HTTP_IF_NONE_MATCH', None)
|
if_none_match = request.META.get('HTTP_IF_NONE_MATCH', None)
|
||||||
if if_none_match == response['ETag']:
|
if if_none_match == response['ETag']:
|
||||||
response.status_code = 304
|
# Setting the status is enough here. The response handling path
|
||||||
response.content = ''
|
# automatically removes content for this status code (in
|
||||||
response['Content-Length'] = '0'
|
# http.conditional_content_removal()).
|
||||||
|
response.status = 304
|
||||||
|
|
||||||
if response.has_header('Last-Modified'):
|
if response.has_header('Last-Modified'):
|
||||||
if_modified_since = request.META.get('HTTP_IF_MODIFIED_SINCE', None)
|
if_modified_since = request.META.get('HTTP_IF_MODIFIED_SINCE', None)
|
||||||
if if_modified_since == response['Last-Modified']:
|
if if_modified_since == response['Last-Modified']:
|
||||||
response.status_code = 304
|
# Setting the status code is enough here (same reasons as
|
||||||
response.content = ''
|
# above).
|
||||||
response['Content-Length'] = '0'
|
response.status = 304
|
||||||
|
|
||||||
if request.method == 'HEAD':
|
|
||||||
response.content = ''
|
|
||||||
|
|
||||||
return response
|
return response
|
||||||
|
|
||||||
|
|
|
@ -42,7 +42,7 @@ class ClientHandler(BaseHandler):
|
||||||
# Apply response middleware
|
# Apply response middleware
|
||||||
for middleware_method in self._response_middleware:
|
for middleware_method in self._response_middleware:
|
||||||
response = middleware_method(request, response)
|
response = middleware_method(request, response)
|
||||||
|
response = self.apply_response_fixes(request, response)
|
||||||
finally:
|
finally:
|
||||||
dispatcher.send(signal=signals.request_finished)
|
dispatcher.send(signal=signals.request_finished)
|
||||||
|
|
||||||
|
|
|
@ -119,7 +119,7 @@ class AssertRedirectsTests(TestCase):
|
||||||
try:
|
try:
|
||||||
self.assertRedirects(response, '/test_client/get_view/')
|
self.assertRedirects(response, '/test_client/get_view/')
|
||||||
except AssertionError, e:
|
except AssertionError, e:
|
||||||
self.assertEquals(str(e), "Response redirected to 'http://testserver/test_client/get_view/?var=value', expected '/test_client/get_view/'")
|
self.assertEquals(str(e), "Response redirected to 'http://testserver/test_client/get_view/?var=value', expected 'http://testserver/test_client/get_view/'")
|
||||||
|
|
||||||
def test_incorrect_target(self):
|
def test_incorrect_target(self):
|
||||||
"An assertion is raised if the response redirects to another target"
|
"An assertion is raised if the response redirects to another target"
|
||||||
|
|
Loading…
Reference in New Issue