Opened 3 years ago

Closed 3 years ago

#35051 closed Cleanup/optimization (fixed)

HEAD Responses Drop Headers

Reported by: Paul Bailey Owned by: Paul Bailey
Component: HTTP handling Version: dev
Severity: Normal Keywords:
Cc: Sarah Boyce, Jannik Schürg, Nick Pope, HAMA Barhamou, Florian Apolloner 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

When using runserver headers are dropped for head requests, in particular content-length. Because my HEAD request is serving a large body content, I do not include it in the body since it is just dropped.

Headers from runserver:

Headers({'date': 'Tue, 19 Dec 2023 12:52:23 GMT', 'server': 'WSGIServer/0.2 CPython/3.11.2', 'accept-ranges': 'bytes', 'content-type': 
'text/html; charset=utf-8', 'x-frame-options': 'DENY', 'x-content-type-options': 'nosniff', 'referrer-policy': 'same-origin', 'cross-or
igin-opener-policy': 'same-origin'})

Headers from uvicorn asgi server:

Headers({'date': 'Tue, 19 Dec 2023 12:54:49 GMT', 'server': 'uvicorn', 'accept-ranges': 'bytes', 'content-length': '121283919', 'content-type': 'text/html; charset=utf-8', 'x-frame-options': 'DENY', 'x-content-type-options': 'nosniff', 'referrer-policy': 'same-origin', 'cross-origin-opener-policy': 'same-origin'})

Notice the uvicorn properly includes the content-length header that was set in the view.

View source snippet:

if request.method == 'HEAD':
    print('HEAD Request')
    return HttpResponse(headers={
        'Accept-Ranges': 'bytes',
        'Content-Length': str(file_size)
      })

Change History (24)

comment:1 by Natalia Bidart, 3 years ago

Cc: Sarah Boyce Jannik Schürg added
Severity: Normal → Release blocker
Triage Stage: Unreviewed → Accepted

I did some research on the topic and from ​the corresponding RFC it seems that this report is valid and should be accepted:

A server MAY send a Content-Length header field in a response to a HEAD request (Section 9.3.2); a server MUST NOT send Content-Length in such a response unless its field value equals the decimal number of octets that would have been sent in the content of a response if the same request had used the GET method.

The removal of the Content-Length seems to be located in this code:

  • django/core/servers/basehttp.py

    diff --git a/django/core/servers/basehttp.py b/django/core/servers/basehttp.py
    index 6afe17cec4..e327974708 100644
    a b class ServerHandler(simple_server.ServerHandler):  
    131131
    132132    def cleanup_headers(self):
    133133        super().cleanup_headers()
    134         if (
    135             self.environ["REQUEST_METHOD"] == "HEAD"
    136             and "Content-Length" in self.headers
    137         ):
    138             del self.headers["Content-Length"]
    139134        # HTTP/1.1 requires support for persistent connections. Send 'close' if
    140135        # the content length is unknown to prevent clients from reusing the
    141136        # connection.

This code was added while fixing ticket #28054, and while it's correct not to return the body of the response, it seems that the Content-Length should be kept.

Regression in 8acc433e415cd771f69dfe84e57878a83641e78b

comment:2 by Mariusz Felisiak, 3 years ago

I don't agree, we intentionally drop the Content-Length. Please check the entire discussion in PR, e.g. this ​comment. As far as I'm aware, the current implementation is RFC compliant. I'd mark this ticket as invalid.

comment:3 by Mariusz Felisiak, 3 years ago

Cc: Nick Pope added
Component: Uncategorized → HTTP handling

comment:4 by Paul Bailey, 3 years ago

The use case for this is that your GET request has a large body and so you do not want to have to produce two large bodies, one for the HEAD, one for GET. Instead you want to match the exact headers a GET request would have without producing the body content for the HEAD request. This is essential for producing HTTP Range Requests in Django without a lot of overhead.

​https://developer.mozilla.org/en-US/docs/Web/HTTP/Range_requests

As per the docs above, a HTTP HEAD should return Content-Length

comment:5 by Paul Bailey, 3 years ago

The discussion linked is over Content-Length being returned for the body of the HEAD response, so this is always "Content-Length: 0" which doesn't match the GET request.

The easiest thing to do at the time was to just remove Content-Length since it is not required. However, for HTTP Range Requests Content-Length is required.

So I think the proper thing to do is to allow it if Content-Length is not Zero. If the header is not 0 then that means it was set by the user and should be allowed through.

comment:6 by Paul Bailey, 3 years ago

something like:

    def cleanup_headers(self):
        super().cleanup_headers()
        if (
            self.environ["REQUEST_METHOD"] == "HEAD"
            and "Content-Length" in self.headers
            and str(self.headers["Content-Length"]) ==  "0"
        ):
            del self.headers["Content-Length"]

comment:7 by Mariusz Felisiak, 3 years ago

As for me this is a new feature request for supporting HTTP ranges.

in reply to:  7 comment:8 by Paul Bailey, 3 years ago

Replying to Mariusz Felisiak:

As for me this is a new feature request for supporting HTTP ranges.

I would imagine Content-Length in HEAD requests is also needed for other streaming mechanisms.

in reply to:  2 ; comment:9 by Natalia Bidart, 3 years ago

Resolution: → invalid
Severity: Release blocker → Normal
Status: new → closed
Triage Stage: Accepted → Unreviewed
Type: Bug → New feature
Version: 5.0 → dev

Replying to Mariusz Felisiak:

I don't agree, we intentionally drop the Content-Length. Please check the entire discussion in PR, e.g. this ​comment. As far as I'm aware, the current implementation is RFC compliant. I'd mark this ticket as invalid.

Thank you Mariusz for the pointer to the specific message from Nick, it provides a very complete and clear reasoning for the change.

With that in mind, I agree that this is not a valid bug. Additionally, I agree that the optional return of Content-Length for HEAD requests, enabling specific use cases, should be approached as a new feature, which should be presented and discussed in the ​Django Forum (following ​the documented guidelines for requesting features). Paul, would you be willing to start a new topic explaining the current situation and outlining potential use cases for the feature?

in reply to:  9 comment:10 by Paul Bailey, 3 years ago

sounds good

comment:11 by Paul Bailey, 3 years ago

Resolution: invalid
Status: closed → new

comment:13 by Paul Bailey, 3 years ago

Owner: changed from nobody to Paul Bailey
Status: new → assigned

comment:14 by Paul Bailey, 3 years ago

Triage Stage: Unreviewed → Accepted

comment:15 by Mariusz Felisiak, 3 years ago

Triage Stage: Accepted → Unreviewed

You cannot accept your own tickets.

comment:16 by HAMA Barhamou, 3 years ago

Cc: HAMA Barhamou added
Triage Stage: Unreviewed → Accepted

Hello, after reading all the comments on the tiket, I think we can accept it and start revising the proposed path. Merry Christmas to all Christians

comment:17 by HAMA Barhamou, 3 years ago

Patch needs improvement: set

comment:18 by Mariusz Felisiak, 3 years ago

Cc: Florian Apolloner added

comment:19 by Paul Bailey, 3 years ago

Patch needs improvement: unset

comment:20 by Mariusz Felisiak, 3 years ago

Type: New feature → Cleanup/optimization

comment:21 by Mariusz Felisiak, 3 years ago

Patch needs improvement: set

comment:22 by Paul Bailey, 3 years ago

Patch needs improvement: unset

comment:23 by Mariusz Felisiak, 3 years ago

Triage Stage: Accepted → Ready for checkin

comment:24 by GitHub <noreply@…>, 3 years ago

Resolution: → fixed
Status: assigned → closed

In 9d52e072:

Fixed #35051 -- Prevented runserver from removing non-zero Content-Length for HEAD requests.

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