Opened 6 months ago

Closed 6 months 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, 6 months ago

Cc: Sarah Boyce Jannik Schürg added
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

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, 6 months 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, 6 months ago

Cc: Nick Pope added
Component: UncategorizedHTTP handling

comment:4 by Paul Bailey, 6 months 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, 6 months 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, 6 months 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, 6 months ago

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

in reply to:  7 comment:8 by Paul Bailey, 6 months 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, 6 months ago

Resolution: invalid
Severity: Release blockerNormal
Status: newclosed
Triage Stage: AcceptedUnreviewed
Type: BugNew feature
Version: 5.0dev

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, 6 months ago

sounds good

comment:11 by Paul Bailey, 6 months ago

Resolution: invalid
Status: closednew

comment:13 by Paul Bailey, 6 months ago

Owner: changed from nobody to Paul Bailey
Status: newassigned

comment:14 by Paul Bailey, 6 months ago

Triage Stage: UnreviewedAccepted

comment:15 by Mariusz Felisiak, 6 months ago

Triage Stage: AcceptedUnreviewed

You cannot accept your own tickets.

comment:16 by HAMA Barhamou, 6 months ago

Cc: HAMA Barhamou added
Triage Stage: UnreviewedAccepted

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, 6 months ago

Patch needs improvement: set

comment:18 by Mariusz Felisiak, 6 months ago

Cc: Florian Apolloner added

comment:19 by Paul Bailey, 6 months ago

Patch needs improvement: unset

comment:20 by Mariusz Felisiak, 6 months ago

Type: New featureCleanup/optimization

comment:21 by Mariusz Felisiak, 6 months ago

Patch needs improvement: set

comment:22 by Paul Bailey, 6 months ago

Patch needs improvement: unset

comment:23 by Mariusz Felisiak, 6 months ago

Triage Stage: AcceptedReady for checkin

comment:24 by GitHub <noreply@…>, 6 months ago

Resolution: fixed
Status: assignedclosed

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