Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #18972 -- Refactored bundled wsgi server's chunking algorithm.

Thanks to amosonn at yahoo.com for the report, @doda for the initial patch and
@datagrok for the revamped logic and test case.
  • Loading branch information...
commit a7960bcb3575fd9fcd5180986ddcdba1caa46dd5 1 parent 80e68ee
Matthew Wood woodm1979 authored charettes committed
Showing with 66 additions and 23 deletions.
  1. +11 −15 django/core/servers/basehttp.py
  2. +55 −8 tests/builtin_server/tests.py
26 django/core/servers/basehttp.py
View
@@ -9,7 +9,7 @@
from __future__ import unicode_literals
-import os
+from io import BytesIO
import socket
import sys
import traceback
@@ -26,7 +26,13 @@
from django.utils.module_loading import import_by_path
from django.utils import six
-__all__ = ['WSGIServer', 'WSGIRequestHandler']
+__all__ = ('WSGIServer', 'WSGIRequestHandler', 'MAX_SOCKET_CHUNK_SIZE')
+
+
+# If data is too large, socket will choke, so write chunks no larger than 32MB
+# at a time. The rationale behind the 32MB can be found on Django's Trac:
+# https://code.djangoproject.com/ticket/5596#comment:4
+MAX_SOCKET_CHUNK_SIZE = 32 * 1024 * 1024 # 32 MB
def get_internal_wsgi_application():
@@ -78,19 +84,9 @@ def write(self, data):
self.bytes_sent += len(data)
# XXX check Content-Length and truncate if too many bytes written?
-
- # If data is too large, socket will choke, so write chunks no larger
- # than 32MB at a time.
- length = len(data)
- if length > 33554432:
- offset = 0
- while offset < length:
- chunk_size = min(33554432, length)
- self._write(data[offset:offset+chunk_size])
- self._flush()
- offset += chunk_size
- else:
- self._write(data)
+ data = BytesIO(data)
+ for chunk in iter(lambda: data.read(MAX_SOCKET_CHUNK_SIZE), b''):
+ self._write(chunk)
self._flush()
def error_output(self, environ, start_response):
63 tests/builtin_server/tests.py
View
@@ -2,22 +2,18 @@
from io import BytesIO
-from django.core.servers.basehttp import ServerHandler
+from django.core.servers.basehttp import ServerHandler, MAX_SOCKET_CHUNK_SIZE
from django.utils.unittest import TestCase
-#
-# Tests for #9659: wsgi.file_wrapper in the builtin server.
-# We need to mock a couple of handlers and keep track of what
-# gets called when using a couple kinds of WSGI apps.
-#
class DummyHandler(object):
- def log_request(*args, **kwargs):
+ def log_request(self, *args, **kwargs):
pass
+
class FileWrapperHandler(ServerHandler):
def __init__(self, *args, **kwargs):
- ServerHandler.__init__(self, *args, **kwargs)
+ super(FileWrapperHandler, self).__init__(*args, **kwargs)
self.request_handler = DummyHandler()
self._used_sendfile = False
@@ -25,17 +21,24 @@ def sendfile(self):
self._used_sendfile = True
return True
+
def wsgi_app(environ, start_response):
start_response(str('200 OK'), [(str('Content-Type'), str('text/plain'))])
return [b'Hello World!']
+
def wsgi_app_file_wrapper(environ, start_response):
start_response(str('200 OK'), [(str('Content-Type'), str('text/plain'))])
return environ['wsgi.file_wrapper'](BytesIO(b'foo'))
+
class WSGIFileWrapperTests(TestCase):
"""
Test that the wsgi.file_wrapper works for the builting server.
+
+ Tests for #9659: wsgi.file_wrapper in the builtin server.
+ We need to mock a couple of handlers and keep track of what
+ gets called when using a couple kinds of WSGI apps.
"""
def test_file_wrapper_uses_sendfile(self):
@@ -53,3 +56,47 @@ def test_file_wrapper_no_sendfile(self):
self.assertFalse(handler._used_sendfile)
self.assertEqual(handler.stdout.getvalue().splitlines()[-1], b'Hello World!')
self.assertEqual(handler.stderr.getvalue(), b'')
+
+
+class WriteChunkCounterHandler(ServerHandler):
+ """
+ Server handler that counts the number of chunks written after headers were
+ sent. Used to make sure large response body chunking works properly.
+ """
+
+ def __init__(self, *args, **kwargs):
+ super(WriteChunkCounterHandler, self).__init__(*args, **kwargs)
+ self.request_handler = DummyHandler()
+ self.headers_written = False
+ self.write_chunk_counter = 0
+
+ def send_headers(self):
+ super(WriteChunkCounterHandler, self).send_headers()
+ self.headers_written = True
+
+ def _write(self, data):
+ if self.headers_written:
+ self.write_chunk_counter += 1
+ self.stdout.write(data)
+
+
+def send_big_data_app(environ, start_response):
+ start_response(str('200 OK'), [(str('Content-Type'), str('text/plain'))])
+ # Return a blob of data that is 1.5 times the maximum chunk size.
+ return [b'x' * (MAX_SOCKET_CHUNK_SIZE + MAX_SOCKET_CHUNK_SIZE // 2)]
+
+
+class ServerHandlerChunksProperly(TestCase):
+ """
+ Test that the ServerHandler chunks data properly.
+
+ Tests for #18972: The logic that performs the math to break data into
+ 32MB (MAX_SOCKET_CHUNK_SIZE) chunks was flawed, BUT it didn't actually
+ cause any problems.
+ """
+
+ def test_chunked_data(self):
+ env = {'SERVER_PROTOCOL': 'HTTP/1.0'}
+ handler = WriteChunkCounterHandler(None, BytesIO(), BytesIO(), env)
+ handler.run(send_big_data_app)
+ self.assertEqual(handler.write_chunk_counter, 2)
Please sign in to comment.
Something went wrong with that request. Please try again.