From 15644cb255f673c07d9bafc3595edaaa4cdc3a90 Mon Sep 17 00:00:00 2001 From: Jacob Kaplan-Moss Date: Sat, 30 Aug 2008 19:56:14 +0000 Subject: [PATCH] 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 | 20 +++++++-- django/core/handlers/wsgi.py | 25 +++++++++-- tests/regressiontests/file_uploads/tests.py | 42 +++++++++++++++++++ .../file_uploads/uploadhandler.py | 10 ++++- tests/regressiontests/file_uploads/urls.py | 1 + tests/regressiontests/file_uploads/views.py | 6 ++- 6 files changed, 94 insertions(+), 10 deletions(-) diff --git a/django/core/handlers/modpython.py b/django/core/handlers/modpython.py index aa3fb23e39..8a5dba99a5 100644 --- a/django/core/handlers/modpython.py +++ b/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,10 +44,13 @@ class ModPythonRequest(http.HttpRequest): get = pformat(self.GET) except: get = '' - try: - post = pformat(self.POST) - except: + if self._post_parse_error: post = '' + else: + try: + post = pformat(self.POST) + except: + post = '' try: cookies = pformat(self.COOKIES) 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 = '' - self._post, self._files = self.parse_file_upload(self.META, self._req) + 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() diff --git a/django/core/handlers/wsgi.py b/django/core/handlers/wsgi.py index 765600de95..5ccbcc6b55 100644 --- a/django/core/handlers/wsgi.py +++ b/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,10 +101,13 @@ class WSGIRequest(http.HttpRequest): get = pformat(self.GET) except: get = '' - try: - post = pformat(self.POST) - except: + if self._post_parse_error: post = '' + else: + try: + post = pformat(self.POST) + except: + post = '' try: cookies = pformat(self.COOKIES) 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 = '' - self._post, self._files = self.parse_file_upload(self.META, self.environ['wsgi.input']) + 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: diff --git a/tests/regressiontests/file_uploads/tests.py b/tests/regressiontests/file_uploads/tests.py index 7c8b53ea89..6fcd8a99aa 100644 --- a/tests/regressiontests/file_uploads/tests.py +++ b/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 diff --git a/tests/regressiontests/file_uploads/uploadhandler.py b/tests/regressiontests/file_uploads/uploadhandler.py index 54f82f626c..9f3960a8d8 100644 --- a/tests/regressiontests/file_uploads/uploadhandler.py +++ b/tests/regressiontests/file_uploads/uploadhandler.py @@ -23,4 +23,12 @@ class QuotaUploadHandler(FileUploadHandler): return raw_data def file_complete(self, file_size): - return None \ No newline at end of file + 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!") diff --git a/tests/regressiontests/file_uploads/urls.py b/tests/regressiontests/file_uploads/urls.py index cc4cc80fdd..607e1d1034 100644 --- a/tests/regressiontests/file_uploads/urls.py +++ b/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), ) diff --git a/tests/regressiontests/file_uploads/views.py b/tests/regressiontests/file_uploads/views.py index b1de169cf0..a989069a30 100644 --- a/tests/regressiontests/file_uploads/views.py +++ b/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)