Opened 11 months ago
Closed 11 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 , 11 months ago
Cc: | added |
---|---|
Severity: | Normal → Release blocker |
Triage Stage: | Unreviewed → Accepted |
follow-up: 9 comment:2 by , 11 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 , 11 months ago
Cc: | added |
---|---|
Component: | Uncategorized → HTTP handling |
comment:4 by , 11 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 , 11 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 , 11 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"]
follow-up: 8 comment:7 by , 11 months ago
As for me this is a new feature request for supporting HTTP ranges.
comment:8 by , 11 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.
follow-up: 10 comment:9 by , 11 months 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?
comment:11 by , 11 months ago
Resolution: | invalid |
---|---|
Status: | closed → new |
Reopening based on discussion at: https://forum.djangoproject.com/t/optionally-do-not-drop-content-length-for-head-requests/26305
comment:12 by , 11 months ago
Has patch: | set |
---|
PR ready for review: https://github.com/django/django/pull/17643#issuecomment-1869135570
comment:13 by , 11 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:14 by , 11 months ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:15 by , 11 months ago
Triage Stage: | Accepted → Unreviewed |
---|
You cannot accept your own tickets.
comment:16 by , 11 months ago
Cc: | 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 , 11 months ago
Patch needs improvement: | set |
---|
comment:18 by , 11 months ago
Cc: | added |
---|
comment:19 by , 11 months ago
Patch needs improvement: | unset |
---|
comment:20 by , 11 months ago
Type: | New feature → Cleanup/optimization |
---|
comment:21 by , 11 months ago
Patch needs improvement: | set |
---|
comment:22 by , 11 months ago
Patch needs improvement: | unset |
---|
comment:23 by , 11 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
I did some research on the topic and from the corresponding RFC it seems that this report is valid and should be accepted:
The removal of the
Content-Length
seems to be located in this code:django/core/servers/basehttp.py
if (self.environ["REQUEST_METHOD"] == "HEAD"and "Content-Length" in self.headers):del self.headers["Content-Length"]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