Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#28440 closed Bug (fixed)

runserver doesn't close the connection for responses without a Content-Length

Reported by: Tom Forbes Owned by: Tom Forbes
Component: HTTP handling Version: dev
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Tom Forbes)

Using MacOS Python 3.5, runserver does not terminate a connection once a HTTP response is sent. This seems to be caused by #25619 (but could be platform specific?).

This results in tools like curl hanging forever, and browsers continually displaying the loading bar.

This code appears to be the culprit, it seems to be copied from the http.server stdlib module. It handles a response and sends the contents correctly in the first iteration of the loop, but then self.close_connection is still true, so it continues to try and read from the socket whilst the client is also reading from the socket.

Replacing the current handle function with handle_one_request fixes this problem, and still seems to use HTTP 1.1.

Change History (15)

comment:1 Changed 6 years ago by Tom Forbes

Description: modified (diff)

comment:2 Changed 6 years ago by Tim Graham

I don't see the behavior you describe on Linux. Not sure if I interpreted your suggested patch correctly but:

  • django/core/servers/basehttp.py

    diff --git a/django/core/servers/basehttp.py b/django/core/servers/basehttp.py
    index d725241..d0309b7 100644
    a b class WSGIRequestHandler(simple_server.WSGIRequestHandler): 
    134134
    135135        return super().get_environ()
    136136
    137     def handle(self):
    138         """Handle multiple requests if necessary."""
    139         self.close_connection = 1
    140         self.handle_one_request()
    141         while not self.close_connection:
    142             self.handle_one_request()
    143 
    144137    def handle_one_request(self):
    145138        """Copy of WSGIRequestHandler.handle() but with different ServerHandler"""
    146139        self.raw_requestline = self.rfile.readline(65537)
    class WSGIRequestHandler(simple_server.WSGIRequestHandler): 
    160153        handler.request_handler = self      # backpointer for logging
    161154        handler.run(self.server.get_app())
    162155
     156    handle = handle_one_request
    163157
    164158def run(addr, port, wsgi_handler, ipv6=False, threading=False, server_cls=WSGIServer):
    165159    server_address = (addr, port)

gives this test failure:

======================================================================
ERROR: test_protocol (servers.tests.LiveServerViews)
Launched server serves with HTTP 1.1.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/python3.6.2/lib/python3.6/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/opt/python3.6.2/lib/python3.6/unittest/case.py", line 605, in run
    testMethod()
  File "/home/tim/code/django/tests/servers/tests.py", line 64, in test_protocol
    conn.getresponse()
  File "/opt/python3.6.2/lib/python3.6/http/client.py", line 1331, in getresponse
    response.begin()
  File "/opt/python3.6.2/lib/python3.6/http/client.py", line 297, in begin
    version, status, reason = self._read_status()
  File "/opt/python3.6.2/lib/python3.6/http/client.py", line 266, in _read_status
    raise RemoteDisconnected("Remote end closed connection without"
http.client.RemoteDisconnected: Remote end closed connection without response

comment:3 Changed 6 years ago by Tom Forbes

Interesting, at first I could not reproduce this with a fresh project on either MacOS or Ubuntu.

However, if you do a plain startproject/startapp and remove all MIDDLEWARE, INSTALLED_APPS (bar your app) and default template CONTEXT_PROCESSORS it is reproducible.

I've made a demo repository that is reproducible on my Ubuntu VM: https://github.com/orf/28440-django-issue

comment:4 Changed 6 years ago by Tim Graham

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Thanks. I'm not sure what the issue could be, offhand.

comment:5 Changed 6 years ago by Tom Forbes

The issue appears to be that the CommonMiddleware sets the Content-Length header, which causes the server to close the connection. When this header is not present (or the middleware not installed) the server continues to wait.

comment:6 Changed 6 years ago by Tom Forbes

Owner: changed from nobody to Tom Forbes
Status: newassigned

comment:7 Changed 6 years ago by Tom Forbes

So this actually appears to be a bug in the http.server module. If you use HTTP/1.1 with Connection: keep-alive and *dont* send a Content-Length header the connection will hang forever. You can test this with a little tinkering in the http.server module itself, and running python3 -mhttp.server on your machine.

I've made a PR (https://github.com/django/django/pull/8820) to simply disable keep-alive for now.

comment:8 Changed 6 years ago by Tom Forbes

Has patch: set

comment:9 Changed 5 years ago by Tim Graham

Summary: Runserver does not correctly close connections once a response is sentrunserver doesn't close the connection for responses without a Content-Length
Triage Stage: AcceptedReady for checkin

comment:10 Changed 5 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In ac756f16:

Fixed #28440 -- Fixed WSGIServer hang on responses without a Content-Length.

Disabled keep-alive to fix the regression in
e6065c7b8363202c5eb13ba10c97a8c24d014b45.

comment:11 Changed 5 years ago by Tim Graham

A PR to fix the test on macOS as reported on reported on django-developers,

comment:12 Changed 5 years ago by Tim Graham <timograham@…>

In 32ade78c:

Refs #28440 -- Fixed server connection closing test on macOS.

comment:13 Changed 5 years ago by Tim Graham <timograham@…>

In 73d025a0:

[2.0.x] Refs #28440 -- Fixed server connection closing test on macOS.

Backport of 32ade78c55edd6231544607a841a9e7efdcbdb5b from master

comment:14 Changed 4 years ago by Florian Apolloner <apollo13@…>

In 934acf11:

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.

comment:15 Changed 4 years ago by Tim Graham <timograham@…>

In e1721ece:

[2.1.x] Fixed #29849 -- Fixed keep-alive support in 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.

Regression in ac756f16c5bbbe544ad82a8f3ab2eac6cccdb62e.
Backport of 934acf1126995f6e6ccba5947ec8f7561633c27f from master.

Note: See TracTickets for help on using tickets.
Back to Top