Fixed keep-alive support in manage.py runserver.

Ticket #25619 changed the default protocol to HTTP/1.1 but did not
properly implement keep-alive. As a "fix" keep-alive was disabled in
ticket #28440 to prevent clients from hanging (they expect the server to
send more data if the connection is not closed and there is no content
length set).

The combination of those two fixes resulted in yet another problem:
HTTP/1.1 by default allows a client to assume that keep-alive is
supported unless the server disables it via 'Connection: close' -- see
RFC2616 8.1.2.1 for details on persistent connection negotiation. Now if
the client receives a response from Django without 'Connection: close'
and immediately sends a new request (on the same tcp connection) before
our server closes the tcp connection, it will error out at some point
because the connection does get closed a few milli seconds later.

This patch fixes the mentioned issues by always sending 'Connection:
close' if we cannot determine a content length. The code is inefficient
in the sense that it does not allow for persistent connections when
chunked responses are used, but that should not really cause any
problems (Django does not generate those) and it only affects the
development server anyways.

Refs #25619, #28440.
This commit is contained in:
Florian Apolloner 2018-11-04 19:03:20 +01:00 committed by Florian Apolloner
parent 1f726311d1
commit 934acf1126
4 changed files with 78 additions and 21 deletions

View File

@ -74,12 +74,24 @@ class WSGIServer(simple_server.WSGIServer):
class ThreadedWSGIServer(socketserver.ThreadingMixIn, WSGIServer): class ThreadedWSGIServer(socketserver.ThreadingMixIn, WSGIServer):
"""A threaded version of the WSGIServer""" """A threaded version of the WSGIServer"""
pass daemon_threads = True
class ServerHandler(simple_server.ServerHandler): class ServerHandler(simple_server.ServerHandler):
http_version = '1.1' http_version = '1.1'
def cleanup_headers(self):
super().cleanup_headers()
# HTTP/1.1 requires us to support persistent connections, so
# explicitly send close if we do not know the content length to
# prevent clients from reusing the connection.
if 'Content-Length' not in self.headers:
self.headers['Connection'] = 'close'
# Mark the connection for closing if we set it as such above or
# if the application sent the header.
if self.headers.get('Connection') == 'close':
self.request_handler.close_connection = True
def handle_error(self): def handle_error(self):
# Ignore broken pipe errors, otherwise pass on # Ignore broken pipe errors, otherwise pass on
if not is_broken_pipe_error(): if not is_broken_pipe_error():
@ -135,6 +147,16 @@ class WSGIRequestHandler(simple_server.WSGIRequestHandler):
return super().get_environ() return super().get_environ()
def handle(self): def handle(self):
self.close_connection = True
self.handle_one_request()
while not self.close_connection:
self.handle_one_request()
try:
self.connection.shutdown(socket.SHUT_WR)
except (socket.error, AttributeError):
pass
def handle_one_request(self):
"""Copy of WSGIRequestHandler.handle() but with different ServerHandler""" """Copy of WSGIRequestHandler.handle() but with different ServerHandler"""
self.raw_requestline = self.rfile.readline(65537) self.raw_requestline = self.rfile.readline(65537)
if len(self.raw_requestline) > 65536: if len(self.raw_requestline) > 65536:
@ -150,7 +172,7 @@ class WSGIRequestHandler(simple_server.WSGIRequestHandler):
handler = ServerHandler( handler = ServerHandler(
self.rfile, self.wfile, self.get_stderr(), self.get_environ() self.rfile, self.wfile, self.get_stderr(), self.get_environ()
) )
handler.request_handler = self # backpointer for logging handler.request_handler = self # backpointer for logging & connection closing
handler.run(self.server.get_app()) handler.run(self.server.get_app())

View File

@ -4,8 +4,7 @@ Tests for django.core.servers.
import errno import errno
import os import os
import socket import socket
import sys from http.client import HTTPConnection
from http.client import HTTPConnection, RemoteDisconnected
from urllib.error import HTTPError from urllib.error import HTTPError
from urllib.parse import urlencode from urllib.parse import urlencode
from urllib.request import urlopen from urllib.request import urlopen
@ -57,29 +56,60 @@ class LiveServerViews(LiveServerBase):
with self.urlopen('/example_view/') as f: with self.urlopen('/example_view/') as f:
self.assertEqual(f.version, 11) self.assertEqual(f.version, 11)
@override_settings(MIDDLEWARE=[])
def test_closes_connection_without_content_length(self): def test_closes_connection_without_content_length(self):
""" """
The server doesn't support keep-alive because Python's http.server A HTTP 1.1 server is supposed to support keep-alive. Since our
module that it uses hangs if a Content-Length header isn't set (for development server is rather simple we support it only in cases where
example, if CommonMiddleware isn't enabled or if the response is a we can detect a content length from the response. This should be doable
StreamingHttpResponse) (#28440 / https://bugs.python.org/issue31076). for all simple views and streaming responses where an iterable with
length of one is passed. The latter follows as result of `set_content_length`
from https://github.com/python/cpython/blob/master/Lib/wsgiref/handlers.py.
If we cannot detect a content length we explicitly set the `Connection`
header to `close` to notify the client that we do not actually support
it.
""" """
conn = HTTPConnection(LiveServerViews.server_thread.host, LiveServerViews.server_thread.port, timeout=1) conn = HTTPConnection(LiveServerViews.server_thread.host, LiveServerViews.server_thread.port, timeout=1)
try: try:
conn.request('GET', '/example_view/', headers={'Connection': 'keep-alive'}) conn.request('GET', '/streaming_example_view/', headers={'Connection': 'keep-alive'})
response = conn.getresponse().read() response = conn.getresponse()
conn.request('GET', '/example_view/', headers={'Connection': 'close'}) self.assertTrue(response.will_close)
# macOS may give ConnectionResetError. self.assertEqual(response.read(), b'Iamastream')
with self.assertRaises((RemoteDisconnected, ConnectionResetError)): self.assertEqual(response.status, 200)
try: self.assertEqual(response.getheader('Connection'), 'close')
conn.getresponse()
except ConnectionAbortedError: conn.request('GET', '/streaming_example_view/', headers={'Connection': 'close'})
if sys.platform == 'win32': response = conn.getresponse()
self.skipTest('Ignore nondeterministic failure on Windows.') self.assertTrue(response.will_close)
self.assertEqual(response.read(), b'Iamastream')
self.assertEqual(response.status, 200)
self.assertEqual(response.getheader('Connection'), 'close')
finally:
conn.close()
def test_keep_alive_on_connection_with_content_length(self):
"""
See `test_closes_connection_without_content_length` for details. This
is a follow up test, which ensure that we do not close the connection
if not needed, hence allowing us to take advantage of keep-alive.
"""
conn = HTTPConnection(LiveServerViews.server_thread.host, LiveServerViews.server_thread.port)
try:
conn.request('GET', '/example_view/', headers={"Connection": "keep-alive"})
response = conn.getresponse()
self.assertFalse(response.will_close)
self.assertEqual(response.read(), b'example view')
self.assertEqual(response.status, 200)
self.assertIsNone(response.getheader('Connection'))
conn.request('GET', '/example_view/', headers={"Connection": "close"})
response = conn.getresponse()
self.assertFalse(response.will_close)
self.assertEqual(response.read(), b'example view')
self.assertEqual(response.status, 200)
self.assertIsNone(response.getheader('Connection'))
finally: finally:
conn.close() conn.close()
self.assertEqual(response, b'example view')
def test_404(self): def test_404(self):
with self.assertRaises(HTTPError) as err: with self.assertRaises(HTTPError) as err:

View File

@ -4,6 +4,7 @@ from . import views
urlpatterns = [ urlpatterns = [
url(r'^example_view/$', views.example_view), url(r'^example_view/$', views.example_view),
url(r'^streaming_example_view/$', views.streaming_example_view),
url(r'^model_view/$', views.model_view), url(r'^model_view/$', views.model_view),
url(r'^create_model_instance/$', views.create_model_instance), url(r'^create_model_instance/$', views.create_model_instance),
url(r'^environ_view/$', views.environ_view), url(r'^environ_view/$', views.environ_view),

View File

@ -1,6 +1,6 @@
from urllib.request import urlopen from urllib.request import urlopen
from django.http import HttpResponse from django.http import HttpResponse, StreamingHttpResponse
from .models import Person from .models import Person
@ -9,6 +9,10 @@ def example_view(request):
return HttpResponse('example view') return HttpResponse('example view')
def streaming_example_view(request):
return StreamingHttpResponse((b'I', b'am', b'a', b'stream'))
def model_view(request): def model_view(request):
people = Person.objects.all() people = Person.objects.all()
return HttpResponse('\n'.join(person.name for person in people)) return HttpResponse('\n'.join(person.name for person in people))