Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

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
  • Loading branch information...
commit 3ee3d6b5f3f3b20c509ae1516354914c5e52f773 1 parent 30848df
Malcolm Tredinnick authored November 11, 2007
28  django/core/handlers/base.py
@@ -4,6 +4,10 @@
4 4
 import sys
5 5
 
6 6
 class BaseHandler(object):
  7
+    # Changes that are always applied to a response (in this order).
  8
+    response_fixes = [http.fix_location_header,
  9
+            http.conditional_content_removal]
  10
+
7 11
     def __init__(self):
8 12
         self._request_middleware = self._view_middleware = self._response_middleware = self._exception_middleware = None
9 13
 
@@ -50,10 +54,6 @@ def load_middleware(self):
50 54
 
51 55
     def get_response(self, request):
52 56
         "Returns an HttpResponse object for the given HttpRequest"
53  
-        response = self._real_get_response(request)
54  
-        return fix_location_header(request, response)
55  
-
56  
-    def _real_get_response(self, request):
57 57
         from django.core import exceptions, urlresolvers
58 58
         from django.core.mail import mail_admins
59 59
         from django.conf import settings
@@ -134,15 +134,13 @@ def _get_traceback(self, exc_info=None):
134 134
         import traceback
135 135
         return '\n'.join(traceback.format_exception(*(exc_info or sys.exc_info())))
136 136
 
137  
-def fix_location_header(request, response):
138  
-    """
139  
-    Ensure that we always use an absolute URI in any location header in the
140  
-    response. This is required by RFC 2616, section 14.30.
141  
-
142  
-    Code constructing response objects is free to insert relative paths and
143  
-    this function converts them to absolute paths.
144  
-    """
145  
-    if 'Location' in response and request.get_host():
146  
-        response['Location'] = request.build_absolute_uri(response['Location'])
147  
-    return response
  137
+    def apply_response_fixes(self, request, response):
  138
+        """
  139
+        Applies each of the functions in self.response_fixes to the request and
  140
+        response, modifying the response in the process. Returns the new
  141
+        response.
  142
+        """
  143
+        for func in self.response_fixes:
  144
+            response = func(request, response)
  145
+        return response
148 146
 
1  django/core/handlers/modpython.py
@@ -162,6 +162,7 @@ def __call__(self, req):
162 162
                 # Apply response middleware
163 163
                 for middleware_method in self._response_middleware:
164 164
                     response = middleware_method(request, response)
  165
+                response = self.apply_response_fixes(request, response)
165 166
         finally:
166 167
             dispatcher.send(signal=signals.request_finished)
167 168
 
2  django/core/handlers/wsgi.py
@@ -207,6 +207,7 @@ def __call__(self, environ, start_response):
207 207
                 # Apply response middleware
208 208
                 for middleware_method in self._response_middleware:
209 209
                     response = middleware_method(request, response)
  210
+                response = self.apply_response_fixes(request, response)
210 211
         finally:
211 212
             dispatcher.send(signal=signals.request_finished)
212 213
 
@@ -220,3 +221,4 @@ def __call__(self, environ, start_response):
220 221
             response_headers.append(('Set-Cookie', str(c.output(header=''))))
221 222
         start_response(status, response_headers)
222 223
         return response
  224
+
1  django/http/__init__.py
@@ -5,6 +5,7 @@
5 5
 from urlparse import urljoin
6 6
 from django.utils.datastructures import MultiValueDict, FileDict
7 7
 from django.utils.encoding import smart_str, iri_to_uri, force_unicode
  8
+from utils import *
8 9
 
9 10
 RESERVED_CHARS="!*'();:@&=+$,/?%#[]"
10 11
 
34  django/http/utils.py
... ...
@@ -0,0 +1,34 @@
  1
+"""
  2
+Functions that modify an HTTP request or response in some way.
  3
+"""
  4
+
  5
+# This group of functions are run as part of the response handling, after
  6
+# everything else, including all response middleware. Think of them as
  7
+# "compulsory response middleware". Be careful about what goes here, because
  8
+# it's a little fiddly to override this behaviour, so they should be truly
  9
+# universally applicable.
  10
+
  11
+def fix_location_header(request, response):
  12
+    """
  13
+    Ensures that we always use an absolute URI in any location header in the
  14
+    response. This is required by RFC 2616, section 14.30.
  15
+
  16
+    Code constructing response objects is free to insert relative paths and
  17
+    this function converts them to absolute paths.
  18
+    """
  19
+    if 'Location' in response and request.get_host():
  20
+        response['Location'] = request.build_absolute_uri(response['Location'])
  21
+    return response
  22
+
  23
+def conditional_content_removal(request, response):
  24
+    """
  25
+    Removes the content of responses for HEAD requests, 1xx, 204 and 304
  26
+    responses. Ensures compliance with RFC 2616, section 4.3.
  27
+    """
  28
+    if 100 <= response.status_code < 200 or response.status_code in (204, 304):
  29
+       response.content = ''
  30
+       response['Content-Length'] = 0
  31
+    if request.method == 'HEAD':
  32
+        response.content = ''
  33
+    return response
  34
+
18  django/middleware/http.py
@@ -6,8 +6,6 @@ class ConditionalGetMiddleware(object):
6 6
     Last-Modified header, and the request has If-None-Match or
7 7
     If-Modified-Since, the response is replaced by an HttpNotModified.
8 8
 
9  
-    Removes the content from any response to a HEAD request.
10  
-
11 9
     Also sets the Date and Content-Length response-headers.
12 10
     """
13 11
     def process_response(self, request, response):
@@ -18,19 +16,17 @@ def process_response(self, request, response):
18 16
         if response.has_header('ETag'):
19 17
             if_none_match = request.META.get('HTTP_IF_NONE_MATCH', None)
20 18
             if if_none_match == response['ETag']:
21  
-                response.status_code = 304
22  
-                response.content = ''
23  
-                response['Content-Length'] = '0'
  19
+                # Setting the status is enough here. The response handling path
  20
+                # automatically removes content for this status code (in
  21
+                # http.conditional_content_removal()).
  22
+                response.status = 304
24 23
 
25 24
         if response.has_header('Last-Modified'):
26 25
             if_modified_since = request.META.get('HTTP_IF_MODIFIED_SINCE', None)
27 26
             if if_modified_since == response['Last-Modified']:
28  
-                response.status_code = 304
29  
-                response.content = ''
30  
-                response['Content-Length'] = '0'
31  
-
32  
-        if request.method == 'HEAD':
33  
-            response.content = ''
  27
+                # Setting the status code is enough here (same reasons as
  28
+                # above).
  29
+                response.status = 304
34 30
 
35 31
         return response
36 32
 
2  django/test/client.py
@@ -42,7 +42,7 @@ def __call__(self, environ):
42 42
             # Apply response middleware
43 43
             for middleware_method in self._response_middleware:
44 44
                 response = middleware_method(request, response)
45  
-
  45
+            response = self.apply_response_fixes(request, response)
46 46
         finally:
47 47
             dispatcher.send(signal=signals.request_finished)
48 48
 
52  tests/regressiontests/test_client_regress/models.py
@@ -31,12 +31,12 @@ def test_contains(self):
31 31
             self.assertContains(response, 'once', 2)
32 32
         except AssertionError, e:
33 33
             self.assertEquals(str(e), "Found 1 instances of 'once' in response (expected 2)")
34  
-        
  34
+
35 35
         try:
36 36
             self.assertContains(response, 'twice', 1)
37 37
         except AssertionError, e:
38 38
             self.assertEquals(str(e), "Found 2 instances of 'twice' in response (expected 1)")
39  
-        
  39
+
40 40
         try:
41 41
             self.assertContains(response, 'thrice')
42 42
         except AssertionError, e:
@@ -46,37 +46,37 @@ def test_contains(self):
46 46
             self.assertContains(response, 'thrice', 3)
47 47
         except AssertionError, e:
48 48
             self.assertEquals(str(e), "Found 0 instances of 'thrice' in response (expected 3)")
49  
-        
  49
+
50 50
 class AssertTemplateUsedTests(TestCase):
51 51
     fixtures = ['testdata.json']
52  
-    
  52
+
53 53
     def test_no_context(self):
54 54
         "Template usage assertions work then templates aren't in use"
55 55
         response = self.client.get('/test_client_regress/no_template_view/')
56 56
 
57 57
         # Check that the no template case doesn't mess with the template assertions
58 58
         self.assertTemplateNotUsed(response, 'GET Template')
59  
-        
  59
+
60 60
         try:
61 61
             self.assertTemplateUsed(response, 'GET Template')
62 62
         except AssertionError, e:
63 63
             self.assertEquals(str(e), "No templates used to render the response")
64 64
 
65  
-    def test_single_context(self):        
  65
+    def test_single_context(self):
66 66
         "Template assertions work when there is a single context"
67 67
         response = self.client.get('/test_client/post_view/', {})
68 68
 
69  
-        # 
  69
+        #
70 70
         try:
71 71
             self.assertTemplateNotUsed(response, 'Empty GET Template')
72 72
         except AssertionError, e:
73 73
             self.assertEquals(str(e), "Template 'Empty GET Template' was used unexpectedly in rendering the response")
74  
-            
  74
+
75 75
         try:
76  
-            self.assertTemplateUsed(response, 'Empty POST Template')        
  76
+            self.assertTemplateUsed(response, 'Empty POST Template')
77 77
         except AssertionError, e:
78 78
             self.assertEquals(str(e), "Template 'Empty POST Template' was not a template used to render the response. Actual template(s) used: Empty GET Template")
79  
-    
  79
+
80 80
     def test_multiple_context(self):
81 81
         "Template assertions work when there are multiple contexts"
82 82
         post_data = {
@@ -99,37 +99,37 @@ def test_multiple_context(self):
99 99
             self.assertEquals(str(e), "Template 'base.html' was used unexpectedly in rendering the response")
100 100
 
101 101
         try:
102  
-            self.assertTemplateUsed(response, "Valid POST Template")        
  102
+            self.assertTemplateUsed(response, "Valid POST Template")
103 103
         except AssertionError, e:
104 104
             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")
105 105
 
106 106
 class AssertRedirectsTests(TestCase):
107 107
     def test_redirect_page(self):
108  
-        "An assertion is raised if the original page couldn't be retrieved as expected"        
  108
+        "An assertion is raised if the original page couldn't be retrieved as expected"
109 109
         # This page will redirect with code 301, not 302
110  
-        response = self.client.get('/test_client/permanent_redirect_view/')        
  110
+        response = self.client.get('/test_client/permanent_redirect_view/')
111 111
         try:
112 112
             self.assertRedirects(response, '/test_client/get_view/')
113 113
         except AssertionError, e:
114 114
             self.assertEquals(str(e), "Response didn't redirect as expected: Response code was 301 (expected 302)")
115  
-    
  115
+
116 116
     def test_lost_query(self):
117 117
         "An assertion is raised if the redirect location doesn't preserve GET parameters"
118 118
         response = self.client.get('/test_client/redirect_view/', {'var': 'value'})
119 119
         try:
120 120
             self.assertRedirects(response, '/test_client/get_view/')
121 121
         except AssertionError, e:
122  
-            self.assertEquals(str(e), "Response redirected to 'http://testserver/test_client/get_view/?var=value', expected '/test_client/get_view/'")
  122
+            self.assertEquals(str(e), "Response redirected to 'http://testserver/test_client/get_view/?var=value', expected 'http://testserver/test_client/get_view/'")
123 123
 
124 124
     def test_incorrect_target(self):
125 125
         "An assertion is raised if the response redirects to another target"
126  
-        response = self.client.get('/test_client/permanent_redirect_view/')        
  126
+        response = self.client.get('/test_client/permanent_redirect_view/')
127 127
         try:
128 128
             # Should redirect to get_view
129 129
             self.assertRedirects(response, '/test_client/some_view/')
130 130
         except AssertionError, e:
131 131
             self.assertEquals(str(e), "Response didn't redirect as expected: Response code was 301 (expected 302)")
132  
-        
  132
+
133 133
     def test_target_page(self):
134 134
         "An assertion is raised if the response redirect target cannot be retrieved as expected"
135 135
         response = self.client.get('/test_client/double_redirect_view/')
@@ -138,7 +138,7 @@ def test_target_page(self):
138 138
             self.assertRedirects(response, 'http://testserver/test_client/permanent_redirect_view/')
139 139
         except AssertionError, e:
140 140
             self.assertEquals(str(e), "Couldn't retrieve redirection page '/test_client/permanent_redirect_view/': response code was 301 (expected 200)")
141  
-            
  141
+
142 142
 class AssertFormErrorTests(TestCase):
143 143
     def test_unknown_form(self):
144 144
         "An assertion is raised if the form name is unknown"
@@ -157,7 +157,7 @@ def test_unknown_form(self):
157 157
             self.assertFormError(response, 'wrong_form', 'some_field', 'Some error.')
158 158
         except AssertionError, e:
159 159
             self.assertEqual(str(e), "The form 'wrong_form' was not used to render the response")
160  
-        
  160
+
161 161
     def test_unknown_field(self):
162 162
         "An assertion is raised if the field name is unknown"
163 163
         post_data = {
@@ -175,7 +175,7 @@ def test_unknown_field(self):
175 175
             self.assertFormError(response, 'form', 'some_field', 'Some error.')
176 176
         except AssertionError, e:
177 177
             self.assertEqual(str(e), "The form 'form' in context 0 does not contain the field 'some_field'")
178  
-        
  178
+
179 179
     def test_noerror_field(self):
180 180
         "An assertion is raised if the field doesn't have any errors"
181 181
         post_data = {
@@ -193,7 +193,7 @@ def test_noerror_field(self):
193 193
             self.assertFormError(response, 'form', 'value', 'Some error.')
194 194
         except AssertionError, e:
195 195
             self.assertEqual(str(e), "The field 'value' on form 'form' in context 0 contains no errors")
196  
-        
  196
+
197 197
     def test_unknown_error(self):
198 198
         "An assertion is raised if the field doesn't contain the provided error"
199 199
         post_data = {
@@ -211,7 +211,7 @@ def test_unknown_error(self):
211 211
             self.assertFormError(response, 'form', 'email', 'Some error.')
212 212
         except AssertionError, e:
213 213
             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.'])")
214  
-    
  214
+
215 215
     def test_unknown_nonfield_error(self):
216 216
         """
217 217
         Checks that an assertion is raised if the form's non field errors
@@ -231,7 +231,7 @@ def test_unknown_nonfield_error(self):
231 231
         try:
232 232
             self.assertFormError(response, 'form', None, 'Some error.')
233 233
         except AssertionError, e:
234  
-            self.assertEqual(str(e), "The form 'form' in context 0 does not contain the non-field error 'Some error.' (actual errors: )")        
  234
+            self.assertEqual(str(e), "The form 'form' in context 0 does not contain the non-field error 'Some error.' (actual errors: )")
235 235
 
236 236
 class FileUploadTests(TestCase):
237 237
     def test_simple_upload(self):
@@ -256,8 +256,8 @@ def test_login_different_client(self):
256 256
 
257 257
         # Get a redirection page with the second client.
258 258
         response = c.get("/test_client_regress/login_protected_redirect_view/")
259  
-        
260  
-        # At this points, the self.client isn't logged in. 
261  
-        # Check that assertRedirects uses the original client, not the 
  259
+
  260
+        # At this points, the self.client isn't logged in.
  261
+        # Check that assertRedirects uses the original client, not the
262 262
         # default client.
263 263
         self.assertRedirects(response, "http://testserver/test_client_regress/get_view/")

0 notes on commit 3ee3d6b

Please sign in to comment.
Something went wrong with that request. Please try again.