Fixed #8622: accessing POST after a POST handling exception no longer throws the server into an infinite loop. Thanks to vung for tracking this one down and fixing it.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@8748 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/core/handlers/modpython.py

@@ -35,6 +35,7 @@ class ModPythonRequest(http.HttpRequest):
             # a common start character for URL patterns. So this is a little
             # naughty, but also pretty harmless.
             self.path_info = u'/'
+        self._post_parse_error = False
 
     def __repr__(self):
         # Since this is called as part of error handling, we need to be very
@@ -43,6 +44,9 @@ class ModPythonRequest(http.HttpRequest):
             get = pformat(self.GET)
         except:
             get = '<could not parse>'
+        if self._post_parse_error:
+            post = '<could not parse>'
+        else:
             try:
                 post = pformat(self.POST)
             except:
@@ -73,7 +77,15 @@ class ModPythonRequest(http.HttpRequest):
         "Populates self._post and self._files"
         if 'content-type' in self._req.headers_in and self._req.headers_in['content-type'].startswith('multipart'):
             self._raw_post_data = ''
+            try:
                 self._post, self._files = self.parse_file_upload(self.META, self._req)
+            except:
+                # See django.core.handlers.wsgi.WSGIHandler for an explanation
+                # of what's going on here.
+                self._post = http.QueryDict('')
+                self._files = datastructures.MultiValueDict()
+                self._post_parse_error = True
+                raise
         else:
             self._post, self._files = http.QueryDict(self.raw_post_data, encoding=self._encoding), datastructures.MultiValueDict()
 

django/core/handlers/wsgi.py

@@ -92,6 +92,7 @@ class WSGIRequest(http.HttpRequest):
         self.META['PATH_INFO'] = path_info
         self.META['SCRIPT_NAME'] = script_name
         self.method = environ['REQUEST_METHOD'].upper()
+        self._post_parse_error = False
 
     def __repr__(self):
         # Since this is called as part of error handling, we need to be very
@@ -100,6 +101,9 @@ class WSGIRequest(http.HttpRequest):
             get = pformat(self.GET)
         except:
             get = '<could not parse>'
+        if self._post_parse_error:
+            post = '<could not parse>'
+        else:
             try:
                 post = pformat(self.POST)
             except:
@@ -127,7 +131,20 @@ class WSGIRequest(http.HttpRequest):
         if self.method == 'POST':
             if self.environ.get('CONTENT_TYPE', '').startswith('multipart'):
                 self._raw_post_data = ''
+                try:
                     self._post, self._files = self.parse_file_upload(self.META, self.environ['wsgi.input'])
+                except:
+                    # An error occured while parsing POST data.  Since when
+                    # formatting the error the request handler might access
+                    # self.POST, set self._post and self._file to prevent
+                    # attempts to parse POST data again.
+                    self._post = http.QueryDict('')
+                    self._files = datastructures.MultiValueDict()
+                    # Mark that an error occured.  This allows self.__repr__ to
+                    # be explicit about it instead of simply representing an
+                    # empty POST
+                    self._post_parse_error = True
+                    raise
             else:
                 self._post, self._files = http.QueryDict(self.raw_post_data, encoding=self._encoding), datastructures.MultiValueDict()
         else:

tests/regressiontests/file_uploads/tests.py

@@ -10,6 +10,7 @@ from django.utils import simplejson
 from django.utils.hashcompat import sha_constructor
 
 from models import FileModel, temp_storage, UPLOAD_TO
+import uploadhandler
 
 class FileUploadTests(TestCase):
     def test_simple_upload(self):
@@ -187,6 +188,47 @@ class FileUploadTests(TestCase):
         self.assertEqual(got.get('file1'), 1)
         self.assertEqual(got.get('file2'), 2)
 
+    def test_file_error_blocking(self):
+        """
+        The server should not block when there are upload errors (bug #8622).
+        This can happen if something -- i.e. an exception handler -- tries to
+        access POST while handling an error in parsing POST. This shouldn't
+        cause an infinite loop!
+        """
+        class POSTAccessingHandler(client.ClientHandler):
+            """A handler that'll access POST during an exception."""
+            def handle_uncaught_exception(self, request, resolver, exc_info):
+                ret = super(POSTAccessingHandler, self).handle_uncaught_exception(request, resolver, exc_info)
+                p = request.POST
+                return ret
+        
+        post_data = {
+            'name': 'Ringo',
+            'file_field': open(__file__),
+        }
+        # Maybe this is a little more complicated that it needs to be; but if
+        # the django.test.client.FakePayload.read() implementation changes then
+        # this test would fail.  So we need to know exactly what kind of error
+        # it raises when there is an attempt to read more than the available bytes:
+        try:
+            client.FakePayload('a').read(2)
+        except Exception, reference_error:
+            pass
+
+        # install the custom handler that tries to access request.POST
+        self.client.handler = POSTAccessingHandler()
+
+        try:
+            response = self.client.post('/file_uploads/upload_errors/', post_data)
+        except reference_error.__class__, err:
+            self.failIf(
+                str(err) == str(reference_error), 
+                "Caught a repeated exception that'll cause an infinite loop in file uploads."
+            )
+        except Exception, err:
+            # CustomUploadError is the error that should have been raised
+            self.assertEqual(err.__class__, uploadhandler.CustomUploadError)
+
 class DirectoryCreationTests(unittest.TestCase):
     """
     Tests for error handling during directory creation

tests/regressiontests/file_uploads/uploadhandler.py

@@ -24,3 +24,11 @@ class QuotaUploadHandler(FileUploadHandler):
             
     def file_complete(self, file_size):
         return None
+
+class CustomUploadError(Exception):
+    pass
+
+class ErroringUploadHandler(FileUploadHandler):
+    """A handler that raises an exception."""
+    def receive_data_chunk(self, raw_data, start):
+        raise CustomUploadError("Oops!")

tests/regressiontests/file_uploads/urls.py

@@ -8,4 +8,5 @@ urlpatterns = patterns('',
     (r'^quota/$',           views.file_upload_quota),
     (r'^quota/broken/$',    views.file_upload_quota_broken),
     (r'^getlist_count/$',   views.file_upload_getlist_count),
+    (r'^upload_errors/$',   views.file_upload_errors),
 )

tests/regressiontests/file_uploads/views.py

@@ -3,7 +3,7 @@ from django.core.files.uploadedfile import UploadedFile
 from django.http import HttpResponse, HttpResponseServerError
 from django.utils import simplejson
 from models import FileModel
-from uploadhandler import QuotaUploadHandler
+from uploadhandler import QuotaUploadHandler, ErroringUploadHandler
 from django.utils.hashcompat import sha_constructor
 
 def file_upload_view(request):
@@ -84,3 +84,7 @@ def file_upload_getlist_count(request):
     for key in request.FILES.keys():
         file_counts[key] = len(request.FILES.getlist(key))
     return HttpResponse(simplejson.dumps(file_counts))
+
+def file_upload_errors(request):
+    request.upload_handlers.insert(0, ErroringUploadHandler())
+    return file_upload_echo(request)