diff --git a/django/core/handlers/base.py b/django/core/handlers/base.py index 13c7f4193f..1796cae8ea 100644 --- a/django/core/handlers/base.py +++ b/django/core/handlers/base.py @@ -4,6 +4,10 @@ from django import http import sys 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): 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): "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.mail import mail_admins from django.conf import settings @@ -134,15 +134,13 @@ class BaseHandler(object): import traceback return '\n'.join(traceback.format_exception(*(exc_info or sys.exc_info()))) -def fix_location_header(request, response): - """ - Ensure 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 apply_response_fixes(self, request, response): + """ + Applies each of the functions in self.response_fixes to the request and + response, modifying the response in the process. Returns the new + response. + """ + for func in self.response_fixes: + response = func(request, response) + return response diff --git a/django/core/handlers/modpython.py b/django/core/handlers/modpython.py index 2a3e03f3dd..e81a65be4d 100644 --- a/django/core/handlers/modpython.py +++ b/django/core/handlers/modpython.py @@ -162,6 +162,7 @@ class ModPythonHandler(BaseHandler): # Apply response middleware for middleware_method in self._response_middleware: response = middleware_method(request, response) + response = self.apply_response_fixes(request, response) finally: dispatcher.send(signal=signals.request_finished) diff --git a/django/core/handlers/wsgi.py b/django/core/handlers/wsgi.py index d06eee73f2..94575ca369 100644 --- a/django/core/handlers/wsgi.py +++ b/django/core/handlers/wsgi.py @@ -207,6 +207,7 @@ class WSGIHandler(BaseHandler): # Apply response middleware for middleware_method in self._response_middleware: response = middleware_method(request, response) + response = self.apply_response_fixes(request, response) finally: dispatcher.send(signal=signals.request_finished) @@ -220,3 +221,4 @@ class WSGIHandler(BaseHandler): response_headers.append(('Set-Cookie', str(c.output(header='')))) start_response(status, response_headers) return response + diff --git a/django/http/__init__.py b/django/http/__init__.py index 51744a0866..47f9736ce2 100644 --- a/django/http/__init__.py +++ b/django/http/__init__.py @@ -5,6 +5,7 @@ from urllib import urlencode from urlparse import urljoin from django.utils.datastructures import MultiValueDict, FileDict from django.utils.encoding import smart_str, iri_to_uri, force_unicode +from utils import * RESERVED_CHARS="!*'();:@&=+$,/?%#[]" diff --git a/django/http/utils.py b/django/http/utils.py new file mode 100644 index 0000000000..d08a9e0237 --- /dev/null +++ b/django/http/utils.py @@ -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 + diff --git a/django/middleware/http.py b/django/middleware/http.py index d080ebcf0f..2ef46c6b61 100644 --- a/django/middleware/http.py +++ b/django/middleware/http.py @@ -6,8 +6,6 @@ class ConditionalGetMiddleware(object): Last-Modified header, and the request has If-None-Match or 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. """ def process_response(self, request, response): @@ -18,19 +16,17 @@ class ConditionalGetMiddleware(object): if response.has_header('ETag'): if_none_match = request.META.get('HTTP_IF_NONE_MATCH', None) if if_none_match == response['ETag']: - response.status_code = 304 - response.content = '' - response['Content-Length'] = '0' + # Setting the status is enough here. The response handling path + # automatically removes content for this status code (in + # http.conditional_content_removal()). + response.status = 304 if response.has_header('Last-Modified'): if_modified_since = request.META.get('HTTP_IF_MODIFIED_SINCE', None) if if_modified_since == response['Last-Modified']: - response.status_code = 304 - response.content = '' - response['Content-Length'] = '0' - - if request.method == 'HEAD': - response.content = '' + # Setting the status code is enough here (same reasons as + # above). + response.status = 304 return response diff --git a/django/test/client.py b/django/test/client.py index 6a05d9dd9c..bbd8239c33 100644 --- a/django/test/client.py +++ b/django/test/client.py @@ -42,7 +42,7 @@ class ClientHandler(BaseHandler): # Apply response middleware for middleware_method in self._response_middleware: response = middleware_method(request, response) - + response = self.apply_response_fixes(request, response) finally: dispatcher.send(signal=signals.request_finished) diff --git a/tests/regressiontests/test_client_regress/models.py b/tests/regressiontests/test_client_regress/models.py index 60fd909f43..b5d9ae63b9 100644 --- a/tests/regressiontests/test_client_regress/models.py +++ b/tests/regressiontests/test_client_regress/models.py @@ -31,12 +31,12 @@ class AssertContainsTests(TestCase): self.assertContains(response, 'once', 2) except AssertionError, e: self.assertEquals(str(e), "Found 1 instances of 'once' in response (expected 2)") - + try: self.assertContains(response, 'twice', 1) except AssertionError, e: self.assertEquals(str(e), "Found 2 instances of 'twice' in response (expected 1)") - + try: self.assertContains(response, 'thrice') except AssertionError, e: @@ -46,37 +46,37 @@ class AssertContainsTests(TestCase): self.assertContains(response, 'thrice', 3) except AssertionError, e: self.assertEquals(str(e), "Found 0 instances of 'thrice' in response (expected 3)") - + class AssertTemplateUsedTests(TestCase): fixtures = ['testdata.json'] - + def test_no_context(self): "Template usage assertions work then templates aren't in use" response = self.client.get('/test_client_regress/no_template_view/') # Check that the no template case doesn't mess with the template assertions self.assertTemplateNotUsed(response, 'GET Template') - + try: self.assertTemplateUsed(response, 'GET Template') except AssertionError, e: self.assertEquals(str(e), "No templates used to render the response") - def test_single_context(self): + def test_single_context(self): "Template assertions work when there is a single context" response = self.client.get('/test_client/post_view/', {}) - # + # try: self.assertTemplateNotUsed(response, 'Empty GET Template') except AssertionError, e: self.assertEquals(str(e), "Template 'Empty GET Template' was used unexpectedly in rendering the response") - + try: - self.assertTemplateUsed(response, 'Empty POST Template') + self.assertTemplateUsed(response, 'Empty POST Template') except AssertionError, e: self.assertEquals(str(e), "Template 'Empty POST Template' was not a template used to render the response. Actual template(s) used: Empty GET Template") - + def test_multiple_context(self): "Template assertions work when there are multiple contexts" post_data = { @@ -99,37 +99,37 @@ class AssertTemplateUsedTests(TestCase): self.assertEquals(str(e), "Template 'base.html' was used unexpectedly in rendering the response") try: - self.assertTemplateUsed(response, "Valid POST Template") + self.assertTemplateUsed(response, "Valid POST Template") except AssertionError, e: self.assertEquals(str(e), "Template 'Valid POST Template' was not a template used to render the response. Actual template(s) used: form_view.html, base.html") class AssertRedirectsTests(TestCase): def test_redirect_page(self): - "An assertion is raised if the original page couldn't be retrieved as expected" + "An assertion is raised if the original page couldn't be retrieved as expected" # This page will redirect with code 301, not 302 - response = self.client.get('/test_client/permanent_redirect_view/') + response = self.client.get('/test_client/permanent_redirect_view/') try: self.assertRedirects(response, '/test_client/get_view/') except AssertionError, e: self.assertEquals(str(e), "Response didn't redirect as expected: Response code was 301 (expected 302)") - + def test_lost_query(self): "An assertion is raised if the redirect location doesn't preserve GET parameters" response = self.client.get('/test_client/redirect_view/', {'var': 'value'}) try: self.assertRedirects(response, '/test_client/get_view/') 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): "An assertion is raised if the response redirects to another target" - response = self.client.get('/test_client/permanent_redirect_view/') + response = self.client.get('/test_client/permanent_redirect_view/') try: # Should redirect to get_view self.assertRedirects(response, '/test_client/some_view/') except AssertionError, e: self.assertEquals(str(e), "Response didn't redirect as expected: Response code was 301 (expected 302)") - + def test_target_page(self): "An assertion is raised if the response redirect target cannot be retrieved as expected" response = self.client.get('/test_client/double_redirect_view/') @@ -138,7 +138,7 @@ class AssertRedirectsTests(TestCase): self.assertRedirects(response, 'http://testserver/test_client/permanent_redirect_view/') except AssertionError, e: self.assertEquals(str(e), "Couldn't retrieve redirection page '/test_client/permanent_redirect_view/': response code was 301 (expected 200)") - + class AssertFormErrorTests(TestCase): def test_unknown_form(self): "An assertion is raised if the form name is unknown" @@ -157,7 +157,7 @@ class AssertFormErrorTests(TestCase): self.assertFormError(response, 'wrong_form', 'some_field', 'Some error.') except AssertionError, e: self.assertEqual(str(e), "The form 'wrong_form' was not used to render the response") - + def test_unknown_field(self): "An assertion is raised if the field name is unknown" post_data = { @@ -175,7 +175,7 @@ class AssertFormErrorTests(TestCase): self.assertFormError(response, 'form', 'some_field', 'Some error.') except AssertionError, e: self.assertEqual(str(e), "The form 'form' in context 0 does not contain the field 'some_field'") - + def test_noerror_field(self): "An assertion is raised if the field doesn't have any errors" post_data = { @@ -193,7 +193,7 @@ class AssertFormErrorTests(TestCase): self.assertFormError(response, 'form', 'value', 'Some error.') except AssertionError, e: self.assertEqual(str(e), "The field 'value' on form 'form' in context 0 contains no errors") - + def test_unknown_error(self): "An assertion is raised if the field doesn't contain the provided error" post_data = { @@ -211,7 +211,7 @@ class AssertFormErrorTests(TestCase): self.assertFormError(response, 'form', 'email', 'Some error.') except AssertionError, e: self.assertEqual(str(e), "The field 'email' on form 'form' in context 0 does not contain the error 'Some error.' (actual errors: [u'Enter a valid e-mail address.'])") - + def test_unknown_nonfield_error(self): """ Checks that an assertion is raised if the form's non field errors @@ -231,7 +231,7 @@ class AssertFormErrorTests(TestCase): try: self.assertFormError(response, 'form', None, 'Some error.') except AssertionError, e: - self.assertEqual(str(e), "The form 'form' in context 0 does not contain the non-field error 'Some error.' (actual errors: )") + self.assertEqual(str(e), "The form 'form' in context 0 does not contain the non-field error 'Some error.' (actual errors: )") class FileUploadTests(TestCase): def test_simple_upload(self): @@ -256,8 +256,8 @@ class LoginTests(TestCase): # Get a redirection page with the second client. response = c.get("/test_client_regress/login_protected_redirect_view/") - - # At this points, the self.client isn't logged in. - # Check that assertRedirects uses the original client, not the + + # At this points, the self.client isn't logged in. + # Check that assertRedirects uses the original client, not the # default client. self.assertRedirects(response, "http://testserver/test_client_regress/get_view/")