Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #15785 -- Stopped HttpRequest.read() from reading beyond the en…

…d 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
  • Loading branch information...
commit a6cd78662e72188555027a459cbff2564d72a221 1 parent 0278947
@jezdez jezdez authored
View
25 django/core/handlers/wsgi.py
@@ -135,26 +135,11 @@ def __init__(self, environ):
self.META['SCRIPT_NAME'] = script_name
self.method = environ['REQUEST_METHOD'].upper()
self._post_parse_error = False
- if type(socket._fileobject) is type and isinstance(self.environ['wsgi.input'], socket._fileobject):
- # Under development server 'wsgi.input' is an instance of
- # socket._fileobject which hangs indefinitely on reading bytes past
- # available count. To prevent this it's wrapped in LimitedStream
- # that doesn't read past Content-Length bytes.
- #
- # This is not done for other kinds of inputs (like flup's FastCGI
- # streams) beacuse they don't suffer from this problem and we can
- # avoid using another wrapper with its own .read and .readline
- # implementation.
- #
- # The type check is done because for some reason, AppEngine
- # implements _fileobject as a function, not a class.
- try:
- content_length = int(self.environ.get('CONTENT_LENGTH', 0))
- except (ValueError, TypeError):
- content_length = 0
- self._stream = LimitedStream(self.environ['wsgi.input'], content_length)
- else:
- self._stream = self.environ['wsgi.input']
+ try:
+ content_length = int(self.environ.get('CONTENT_LENGTH'))
+ except (ValueError, TypeError):
+ content_length = 0
+ self._stream = LimitedStream(self.environ['wsgi.input'], content_length)
self._read_started = False
def get_full_path(self):
View
12 django/http/__init__.py
@@ -308,17 +308,7 @@ def _get_raw_post_data(self):
if not hasattr(self, '_raw_post_data'):
if self._read_started:
raise Exception("You cannot access raw_post_data after reading from request's data stream")
- try:
- content_length = int(self.META.get('CONTENT_LENGTH', 0))
- except (ValueError, TypeError):
- # If CONTENT_LENGTH was empty string or not an integer, don't
- # error out. We've also seen None passed in here (against all
- # specs, but see ticket #8259), so we handle TypeError as well.
- content_length = 0
- if content_length:
- self._raw_post_data = self.read(content_length)
- else:
- self._raw_post_data = self.read()
+ self._raw_post_data = self.read()
self._stream = StringIO(self._raw_post_data)
return self._raw_post_data
raw_post_data = property(_get_raw_post_data)
View
36 django/http/multipartparser.py
@@ -33,7 +33,7 @@ class MultiPartParser(object):
A rfc2388 multipart/form-data parser.
``MultiValueDict.parse()`` reads the input stream in ``chunk_size`` chunks
- and returns a tuple of ``(MultiValueDict(POST), MultiValueDict(FILES))``. If
+ and returns a tuple of ``(MultiValueDict(POST), MultiValueDict(FILES))``.
"""
def __init__(self, META, input_data, upload_handlers, encoding=None):
"""
@@ -65,14 +65,11 @@ def __init__(self, META, input_data, upload_handlers, encoding=None):
raise MultiPartParserError('Invalid boundary in multipart: %s' % boundary)
- #
# Content-Length should contain the length of the body we are about
# to receive.
- #
try:
content_length = int(META.get('HTTP_CONTENT_LENGTH', META.get('CONTENT_LENGTH',0)))
except (ValueError, TypeError):
- # For now set it to 0; we'll try again later on down.
content_length = 0
if content_length < 0:
@@ -110,12 +107,10 @@ def parse(self):
if self._content_length == 0:
return QueryDict(MultiValueDict(), encoding=self._encoding), MultiValueDict()
- limited_input_data = LimitBytes(self._input_data, self._content_length)
-
# See if the handler will want to take care of the parsing.
# This allows overriding everything if somebody wants it.
for handler in handlers:
- result = handler.handle_raw_input(limited_input_data,
+ result = handler.handle_raw_input(self._input_data,
self._meta,
self._content_length,
self._boundary,
@@ -128,7 +123,7 @@ def parse(self):
self._files = MultiValueDict()
# Instantiate the parser and stream:
- stream = LazyStream(ChunkIter(limited_input_data, self._chunk_size))
+ stream = LazyStream(ChunkIter(self._input_data, self._chunk_size))
# Whether or not to signal a file-completion at the beginning of the loop.
old_field_name = None
@@ -225,10 +220,10 @@ def parse(self):
exhaust(stream)
except StopUpload, e:
if not e.connection_reset:
- exhaust(limited_input_data)
+ exhaust(self._input_data)
else:
# Make sure that the request data is all fed
- exhaust(limited_input_data)
+ exhaust(self._input_data)
# Signal that the upload has completed.
for handler in handlers:
@@ -390,27 +385,6 @@ def next(self):
def __iter__(self):
return self
-class LimitBytes(object):
- """ Limit bytes for a file object. """
- def __init__(self, fileobject, length):
- self._file = fileobject
- self.remaining = length
-
- def read(self, num_bytes=None):
- """
- Read data from the underlying file.
- If you ask for too much or there isn't anything left,
- this will raise an InputStreamExhausted error.
- """
- if self.remaining <= 0:
- raise InputStreamExhausted()
- if num_bytes is None:
- num_bytes = self.remaining
- else:
- num_bytes = min(num_bytes, self.remaining)
- self.remaining -= num_bytes
- return self._file.read(num_bytes)
-
class InterBoundaryIter(object):
"""
A Producer that will iterate over boundaries.
View
43 tests/regressiontests/file_uploads/tests.py
@@ -9,7 +9,7 @@
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 @@ def test_filename_overflow(self):
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()
View
30 tests/regressiontests/requests/tests.py
@@ -195,7 +195,10 @@ def test_limited_stream(self):
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 @@ def test_read_after_value(self):
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 @@ def test_value_after_read(self):
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 @@ def test_POST_multipart_with_content_length_zero(self):
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 @@ def test_POST_after_raw_post_data_read_and_stream_read(self):
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']})
View
46 tests/regressiontests/test_client_regress/models.py
@@ -913,14 +913,44 @@ def test_response_no_template(self):
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):
View
2  tests/regressiontests/test_client_regress/urls.py
@@ -27,5 +27,7 @@
(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),
)
View
8 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
Please sign in to comment.
Something went wrong with that request. Please try again.