Opened 4 years ago

Closed 3 years ago

Last modified 14 months ago

#25619 closed New feature (fixed)

Make runserver use HTTP 1.1

Reported by: Gerben Morsink Owned by: nobody
Component: HTTP handling Version: master
Severity: Normal Keywords:
Cc: k@…, clokep@… 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

And http_version 1.0 is not working with websockets.

I don't know exactly why this is happening, but I don't think it is meant to be like this.

I think HTTP/1.1 is fine to use for the development runserver.

Change History (17)

comment:1 Changed 4 years ago by Tim Graham

What methodology are you using to test? Can you bisect Django's commit history to determine where the behavior changed?

comment:2 Changed 4 years ago by Claude Paroz

Resolution: worksforme
Status: newclosed

I cannot confirm this, I've checked with one of my site with Django 1.8 and the response was HTTP/1.1 200 OK.
By the way, this is probably controlled by the Web server, not Django.

comment:3 Changed 4 years ago by Claude Paroz

Resolution: worksforme
Status: closednew

Oh, sorry, I didn't pay attention to the runserver part...

comment:4 Changed 4 years ago by Claude Paroz

Triage Stage: UnreviewedAccepted
Version: 1.7master

Looks like that change would be required:

  • django/core/servers/basehttp.py

    diff --git a/django/core/servers/basehttp.py b/django/core/servers/basehttp.py
    index 4e2f8dd..1e0f0c2 100644
    a b class WSGIServer(simple_server.WSGIServer, object): 
    8686
    8787# Inheriting from object required on Python 2.
    8888class ServerHandler(simple_server.ServerHandler, object):
     89    http_version = "1.1"
    8990    def handle_error(self):
    9091        # Ignore broken pipe errors, otherwise pass on
    9192        if not is_broken_pipe_error():

However, I didn't find the possible faulty commit.

comment:5 Changed 4 years ago by Claude Paroz

Has patch: set

comment:6 Changed 4 years ago by Tim Graham

Summary: Since Django ~1.6.4 the dev runserver uses http_version 1.0, before it was 1.1Make runserver use HTTP 1.1
Triage Stage: AcceptedReady for checkin
Type: BugNew feature

comment:7 Changed 4 years ago by Tim Graham

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

As noted on the pull request , it looks to me like we'd also need to set WSGIServer.protocol_version() too. Not sure this is trivial given the sentence in the Python docs, "If set to 'HTTP/1.1', the server will permit HTTP persistent connections; however, your server must then include an accurate Content-Length header (using send_header()) in all of its responses to clients." Given that, I'd be surprised if older versions of runserver really used HTTP 1.1.

comment:8 Changed 4 years ago by Claude Paroz

Seems this ticket depends on #5897 (setting Content-Length on non-streaming responses).

comment:9 Changed 4 years ago by Kevin Christopher Henry

Cc: k@… added

This issue bit me recently because Chrome refuses to use ETags if the server responds with HTTP/1.0 (see here, which I can confirm). That led to some frustrating debugging.

So +1 to fixing this and #5897.

comment:10 Changed 4 years ago by Claude Paroz

Patch needs improvement: unset

comment:11 Changed 4 years ago by Tim Graham

Patch needs improvement: set

As noted on the PR, this causes problems when running the selenium tests, at least on Chrome.

comment:12 Changed 3 years ago by Patrick Cloke

Cc: clokep@… added

comment:13 Changed 3 years ago by Claude Paroz

Patch needs improvement: unset

I've recreated a pull request after #20238 has been fixed. Hopefully selenium tests will pass now.

comment:14 Changed 3 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:15 Changed 3 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: newclosed

In e6065c7:

Fixed #25619 -- Made runserver serve with HTTP 1.1 protocol

Thanks Tim Graham for the review.

comment:16 Changed 15 months 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:17 Changed 14 months 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