[Django] #28440: Runserver does not correctly close connections once a response is sent
Django
-----------------------------------------+------------------------
Reporter: Tom | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
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.
[https://github.com/django/django/blob/master/django/core/servers/basehttp.py#L137-L142
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.
Removing the current `handle` function with `handle_one_request` fixes
this problem, and still seems to use HTTP 1.1.
--
Ticket URL: <https://code.djangoproject.com/ticket/28440>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Tom | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Tom:
Old description:
> 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.
>
> [https://github.com/django/django/blob/master/django/core/servers/basehttp.py#L137-L142
> 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.
>
> Removing the current `handle` function with `handle_one_request` fixes
> this problem, and still seems to use HTTP 1.1.
New description:
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.
[https://github.com/django/django/blob/master/django/core/servers/basehttp.py#L137-L142
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.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28440#comment:1>
Django
Reporter: Tom | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
I don't see the behavior you describe on Linux. Not sure if I interpreted
your suggested patch correctly but:
{{{ #!diff
diff --git a/django/core/servers/basehttp.py
b/django/core/servers/basehttp.py
index d725241..d0309b7 100644
--- a/django/core/servers/basehttp.py
+++ b/django/core/servers/basehttp.py
@@ -134,13 +134,6 @@ class
WSGIRequestHandler(simple_server.WSGIRequestHandler):
return super().get_environ()
- def handle(self):
- """Handle multiple requests if necessary."""
- self.close_connection = 1
- self.handle_one_request()
- while not self.close_connection:
- self.handle_one_request()
-
def handle_one_request(self):
"""Copy of WSGIRequestHandler.handle() but with different
ServerHandler"""
self.raw_requestline = self.rfile.readline(65537)
@@ -160,6 +153,7 @@ class
WSGIRequestHandler(simple_server.WSGIRequestHandler):
handler.request_handler = self # backpointer for logging
handler.run(self.server.get_app())
+ handle = handle_one_request
def run(addr, port, wsgi_handler, ipv6=False, threading=False,
server_cls=WSGIServer):
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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28440#comment:2>
Django
Reporter: Tom | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tom):
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
--
Ticket URL: <https://code.djangoproject.com/ticket/28440#comment:3>
Django
Reporter: Tom | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Thanks. I'm not sure what the issue could be, offhand.
--
Ticket URL: <https://code.djangoproject.com/ticket/28440#comment:4>
Django
Reporter: Tom | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tom):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28440#comment:5>
Django
Reporter: Tom | Owner: Tom
Type: Bug | Status: assigned
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tom):
* owner: nobody => Tom
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/28440#comment:6>
Django
Reporter: Tom | Owner: Tom
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tom):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/28440#comment:7>
Django
Reporter: Tom | Owner: Tom
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tom):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28440#comment:8>
Django
Length
-------------------------------------+-------------------------------------
Reporter: Tom | Owner: Tom
Type: Bug | Status: assigned
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/28440#comment:9>
Django
Length
-------------------------------------+-------------------------------------
Reporter: Tom | Owner: Tom
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"ac756f16c5bbbe544ad82a8f3ab2eac6cccdb62e" ac756f16]:
{{{
#!CommitTicketReference repository=""
revision="ac756f16c5bbbe544ad82a8f3ab2eac6cccdb62e"
Fixed #28440 -- Fixed WSGIServer hang on responses without a Content-
Length.
Disabled keep-alive to fix the regression in
e6065c7b8363202c5eb13ba10c97a8c24d014b45.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28440#comment:10>
Django
Length
-------------------------------------+-------------------------------------
| Forbes
Type: Bug | Status: closed
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
A [https://github.com/django/django/pull/9233 PR] to fix the test on macOS
as reported on [https://groups.google.com/d/topic/django-
developers/zsfgCudRnFI/discussion reported on django-developers],
--
Ticket URL: <https://code.djangoproject.com/ticket/28440#comment:11>
Django
Length
-------------------------------------+-------------------------------------
Reporter: Tom Forbes | Owner: Tom
| Forbes
Type: Bug | Status: closed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"32ade78c55edd6231544607a841a9e7efdcbdb5b" 32ade78c]:
{{{
#!CommitTicketReference repository=""
revision="32ade78c55edd6231544607a841a9e7efdcbdb5b"
Refs #28440 -- Fixed server connection closing test on macOS.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28440#comment:12>
Django
Length
-------------------------------------+-------------------------------------
Reporter: Tom Forbes | Owner: Tom
| Forbes
Type: Bug | Status: closed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"73d025a04215a3738867fbe1f8b0ce6557a3ee03" 73d025a0]:
{{{
#!CommitTicketReference repository=""
revision="73d025a04215a3738867fbe1f8b0ce6557a3ee03"
[2.0.x] Refs #28440 -- Fixed server connection closing test on macOS.
Backport of 32ade78c55edd6231544607a841a9e7efdcbdb5b from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28440#comment:13>
Django
Length
-------------------------------------+-------------------------------------
Reporter: Tom Forbes | Owner: Tom
| Forbes
Type: Bug | Status: closed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner <apollo13@…>):
In [changeset:"934acf1126995f6e6ccba5947ec8f7561633c27f" 934acf11]:
{{{
#!CommitTicketReference repository=""
revision="934acf1126995f6e6ccba5947ec8f7561633c27f"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28440#comment:14>
Django
Length
-------------------------------------+-------------------------------------
Reporter: Tom Forbes | Owner: Tom
| Forbes
Type: Bug | Status: closed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"e1721ece485b35ab5543f134203a8a8ce9f31a7c" e1721ece]:
{{{
#!CommitTicketReference repository=""
revision="e1721ece485b35ab5543f134203a8a8ce9f31a7c"
[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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28440#comment:15>