Fixed #15785 -- Stopped HttpRequest.read() from reading beyond the end of a wsgi.input stream and removed some redundant code in the multipartparser. Thanks, tomchristie, grahamd and isagalaev.

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

5 files changed, 114 insertions, 15 deletions

diff --git a/tests/regressiontests/file_uploads/tests.py b/tests/regressiontests/file_uploads/tests.py
index d42e0279a7..6d79e2a451 100644
--- a/tests/regressiontests/file_uploads/tests.py
+++ b/tests/regressiontests/file_uploads/tests.py
@@ -9,7 +9,7 @@ from StringIO import StringIO
 
 from django.core.files import temp as tempfile
 from django.core.files.uploadedfile import SimpleUploadedFile
-from django.http.multipartparser import MultiPartParser
+from django.http.multipartparser import MultiPartParser, MultiPartParserError
 from django.test import TestCase, client
 from django.utils import simplejson
 from django.utils import unittest
@@ -176,6 +176,47 @@ class FileUploadTests(TestCase):
         got = simplejson.loads(self.client.request(**r).content)
         self.assertTrue(len(got['file']) < 256, "Got a long file name (%s characters)." % len(got['file']))
 
+    def test_truncated_multipart_handled_gracefully(self):
+        """
+        If passed an incomplete multipart message, MultiPartParser does not
+        attempt to read beyond the end of the stream, and simply will handle
+        the part that can be parsed gracefully.
+        """
+        payload = "\r\n".join([
+            '--' + client.BOUNDARY,
+            'Content-Disposition: form-data; name="file"; filename="foo.txt"',
+            'Content-Type: application/octet-stream',
+            '',
+            'file contents'
+            '--' + client.BOUNDARY + '--',
+            '',
+        ])
+        payload = payload[:-10]
+        r = {
+            'CONTENT_LENGTH': len(payload),
+            'CONTENT_TYPE': client.MULTIPART_CONTENT,
+            'PATH_INFO': '/file_uploads/echo/',
+            'REQUEST_METHOD': 'POST',
+            'wsgi.input': client.FakePayload(payload),
+        }
+        got = simplejson.loads(self.client.request(**r).content)
+        self.assertEquals(got, {})
+
+    def test_empty_multipart_handled_gracefully(self):
+        """
+        If passed an empty multipart message, MultiPartParser will return
+        an empty QueryDict.
+        """
+        r = {
+            'CONTENT_LENGTH': 0,
+            'CONTENT_TYPE': client.MULTIPART_CONTENT,
+            'PATH_INFO': '/file_uploads/echo/',
+            'REQUEST_METHOD': 'POST',
+            'wsgi.input': client.FakePayload(''),
+        }
+        got = simplejson.loads(self.client.request(**r).content)
+        self.assertEquals(got, {})
+
     def test_custom_upload_handler(self):
         # A small file (under the 5M quota)
         smallfile = tempfile.NamedTemporaryFile()
diff --git a/tests/regressiontests/requests/tests.py b/tests/regressiontests/requests/tests.py
index 91f8b2f805..8bc81ffcf2 100644
--- a/tests/regressiontests/requests/tests.py
+++ b/tests/regressiontests/requests/tests.py
@@ -195,7 +195,10 @@ class RequestsTests(unittest.TestCase):
         self.assertEqual(stream.read(), '')
 
     def test_stream(self):
-        request = WSGIRequest({'REQUEST_METHOD': 'POST', 'wsgi.input': StringIO('name=value')})
+        payload = 'name=value'
+        request = WSGIRequest({'REQUEST_METHOD': 'POST',
+                               'CONTENT_LENGTH': len(payload),
+                               'wsgi.input': StringIO(payload)})
         self.assertEqual(request.read(), 'name=value')
 
     def test_read_after_value(self):
@@ -203,7 +206,10 @@ class RequestsTests(unittest.TestCase):
         Reading from request is allowed after accessing request contents as
         POST or raw_post_data.
         """
-        request = WSGIRequest({'REQUEST_METHOD': 'POST', 'wsgi.input': StringIO('name=value')})
+        payload = 'name=value'
+        request = WSGIRequest({'REQUEST_METHOD': 'POST',
+                               'CONTENT_LENGTH': len(payload),
+                               'wsgi.input': StringIO(payload)})
         self.assertEqual(request.POST, {u'name': [u'value']})
         self.assertEqual(request.raw_post_data, 'name=value')
         self.assertEqual(request.read(), 'name=value')
@@ -213,7 +219,10 @@ class RequestsTests(unittest.TestCase):
         Construction of POST or raw_post_data is not allowed after reading
         from request.
         """
-        request = WSGIRequest({'REQUEST_METHOD': 'POST', 'wsgi.input': StringIO('name=value')})
+        payload = 'name=value'
+        request = WSGIRequest({'REQUEST_METHOD': 'POST',
+                               'CONTENT_LENGTH': len(payload),
+                               'wsgi.input': StringIO(payload)})
         self.assertEqual(request.read(2), 'na')
         self.assertRaises(Exception, lambda: request.raw_post_data)
         self.assertEqual(request.POST, {})
@@ -261,14 +270,20 @@ class RequestsTests(unittest.TestCase):
         self.assertEqual(request.POST, {})
 
     def test_read_by_lines(self):
-        request = WSGIRequest({'REQUEST_METHOD': 'POST', 'wsgi.input': StringIO('name=value')})
+        payload = 'name=value'
+        request = WSGIRequest({'REQUEST_METHOD': 'POST',
+                               'CONTENT_LENGTH': len(payload),
+                               'wsgi.input': StringIO(payload)})
         self.assertEqual(list(request), ['name=value'])
 
     def test_POST_after_raw_post_data_read(self):
         """
         POST should be populated even if raw_post_data is read first
         """
-        request = WSGIRequest({'REQUEST_METHOD': 'POST', 'wsgi.input': StringIO('name=value')})
+        payload = 'name=value'
+        request = WSGIRequest({'REQUEST_METHOD': 'POST',
+                               'CONTENT_LENGTH': len(payload),
+                               'wsgi.input': StringIO(payload)})
         raw_data = request.raw_post_data
         self.assertEqual(request.POST, {u'name': [u'value']})
 
@@ -277,7 +292,10 @@ class RequestsTests(unittest.TestCase):
         POST should be populated even if raw_post_data is read first, and then
         the stream is read second.
         """
-        request = WSGIRequest({'REQUEST_METHOD': 'POST', 'wsgi.input': StringIO('name=value')})
+        payload = 'name=value'
+        request = WSGIRequest({'REQUEST_METHOD': 'POST',
+                               'CONTENT_LENGTH': len(payload),
+                               'wsgi.input': StringIO(payload)})
         raw_data = request.raw_post_data
         self.assertEqual(request.read(1), u'n')
         self.assertEqual(request.POST, {u'name': [u'value']})
diff --git a/tests/regressiontests/test_client_regress/models.py b/tests/regressiontests/test_client_regress/models.py
index d23ed614f5..8e840e4982 100644
--- a/tests/regressiontests/test_client_regress/models.py
+++ b/tests/regressiontests/test_client_regress/models.py
@@ -913,14 +913,44 @@ class ResponseTemplateDeprecationTests(TestCase):
         response = self.client.get("/test_client_regress/request_methods/")
         self.assertEqual(response.template, None)
 
-class RawPostDataTest(TestCase):
-    "Access to request.raw_post_data from the test client."
-    def test_raw_post_data(self):
-        # Refs #14753
-        try:
-            response = self.client.get("/test_client_regress/raw_post_data/")
-        except AssertionError:
-            self.fail("Accessing request.raw_post_data from a view fetched with GET by the test client shouldn't fail.")
+
+class ReadLimitedStreamTest(TestCase):
+    """
+    Tests that ensure that HttpRequest.raw_post_data, HttpRequest.read() and
+    HttpRequest.read(BUFFER) have proper LimitedStream behaviour.
+
+    Refs #14753, #15785
+    """
+    def test_raw_post_data_from_empty_request(self):
+        """HttpRequest.raw_post_data on a test client GET request should return
+        the empty string."""
+        self.assertEquals(self.client.get("/test_client_regress/raw_post_data/").content, '')
+
+    def test_read_from_empty_request(self):
+        """HttpRequest.read() on a test client GET request should return the
+        empty string."""
+        self.assertEquals(self.client.get("/test_client_regress/read_all/").content, '')
+
+    def test_read_numbytes_from_empty_request(self):
+        """HttpRequest.read(LARGE_BUFFER) on a test client GET request should
+        return the empty string."""
+        self.assertEquals(self.client.get("/test_client_regress/read_buffer/").content, '')
+
+    def test_read_from_nonempty_request(self):
+        """HttpRequest.read() on a test client PUT request with some payload
+        should return that payload."""
+        payload = 'foobar'
+        self.assertEquals(self.client.put("/test_client_regress/read_all/",
+                                          data=payload,
+                                          content_type='text/plain').content, payload)
+
+    def test_read_numbytes_from_nonempty_request(self):
+        """HttpRequest.read(LARGE_BUFFER) on a test client PUT request with
+        some payload should return that payload."""
+        payload = 'foobar'
+        self.assertEquals(self.client.put("/test_client_regress/read_buffer/",
+                                          data=payload,
+                                          content_type='text/plain').content, payload)
 
 
 class RequestFactoryStateTest(TestCase):
diff --git a/tests/regressiontests/test_client_regress/urls.py b/tests/regressiontests/test_client_regress/urls.py
index 121f4df638..454f35b824 100644
--- a/tests/regressiontests/test_client_regress/urls.py
+++ b/tests/regressiontests/test_client_regress/urls.py
@@ -27,5 +27,7 @@ urlpatterns = patterns('',
     (r'^check_headers/$', views.check_headers),
     (r'^check_headers_redirect/$', RedirectView.as_view(url='/test_client_regress/check_headers/')),
     (r'^raw_post_data/$', views.raw_post_data),
+    (r'^read_all/$', views.read_all),
+    (r'^read_buffer/$', views.read_buffer),
     (r'^request_context_view/$', views.request_context_view),
 )
diff --git a/tests/regressiontests/test_client_regress/views.py b/tests/regressiontests/test_client_regress/views.py
index c40f34fe56..b3982938b7 100644
--- a/tests/regressiontests/test_client_regress/views.py
+++ b/tests/regressiontests/test_client_regress/views.py
@@ -96,6 +96,14 @@ def raw_post_data(request):
     "A view that is requested with GET and accesses request.raw_post_data. Refs #14753."
     return HttpResponse(request.raw_post_data)
 
+def read_all(request):
+    "A view that is requested with accesses request.read()."
+    return HttpResponse(request.read())
+
+def read_buffer(request):
+    "A view that is requested with accesses request.read(LARGE_BUFFER)."
+    return HttpResponse(request.read(99999))
+
 def request_context_view(request):
     # Special attribute that won't be present on a plain HttpRequest
     request.special_path = request.path