#5897 closed New feature (fixed)
Add Content-Length header in common middleware
Reported by: | Owned by: | ccahoon | |
---|---|---|---|
Component: | HTTP handling | Version: | dev |
Severity: | Normal | Keywords: | Content-Length middleware |
Cc: | 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
I've modified django.middleware.common.py to add a "Content-Length" header to the HTTP response if the response code is 200 and the "Content-Length" header hasn't already been supplied.
Attachments (1)
Change History (16)
by , 17 years ago
Attachment: | middleware.common.patch added |
---|
comment:1 by , 17 years ago
Triage Stage: | Unreviewed → Ready for checkin |
---|
comment:2 by , 16 years ago
Hmmm, what's the advantage of doing this? I don't necessarily like the overhead of doing a len()
on every request...
comment:3 by , 16 years ago
Triage Stage: | Ready for checkin → Design decision needed |
---|
comment:4 by , 16 years ago
The advantage is following the HTTP spec, which mandates that messages either:
- be length 0 by design (e.g. HEAD)
- use "chunked" encoding
- provide Content-Length
- use the multipart/byteranges media type, or
- close the connection.
Options 1, 2, and 4 are rare, so omitting C-L leaves us with (5) and therefore means conns can't ever use keepalive. I would say that any response with a buffered body, not just 200, should get a C-L response header.
In addition, the len(str) function seems to be nearly O(1) up to about 95MB:
>>> for size in range(10): ... timeit.Timer("len(a)", "a = 'a' * %d" % (10 ** size)).timeit() ... 0.090876075031882553 0.092112894236557996 0.091411897322145691 0.096701326565247825 0.095689815325690875 0.098474945838088412 0.09903919352878654 0.097267250446634979 0.09309835467915617 0.4417090719630572 >>> 10 ** 8 100000000 >>> _ / 1024 97656 >>> _ / 1024 95
comment:5 by , 15 years ago
Owner: | changed from | to
---|
comment:6 by , 15 years ago
Three things:
- This does not interfere with HttpResponseSendFile, which bypasses response middleware.
- Content-Length is a SHOULD in the HTTP/1.0 and HTTP/1.1 specs. / HTTP1.0, Content-Length. /HTTP1.1, Content-Length
- As noted by grahamd, Content-Length is not required by WSGI either (in the response). http://code.djangoproject.com/ticket/7581#comment:12
That said, I am going to investigate further. This is closely related to #7581, which incidentally has a very similar bit of code to the patch provided here.
When dealing with streaming responses, probably intended for chunked transfer encoding, we want to ensure that we don't consume the whole generator/iterator in any effort to send a header that is not strictly required.
comment:7 by , 15 years ago
ConditionalGetMiddleware already does this (amongst other things). http://docs.djangoproject.com/en/dev/ref/middleware/#module-django.middleware.http
Is that not sufficient?
Also, Content-Length should be set for all header codes (Ajax and REST APIs will use a wider selection, 201 for example).
core/handlers/base.py will use http.conditional_content_removal to remove Content-Length for any response codes that shouldn't have it, so setting for all is safe.
comment:8 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:9 by , 13 years ago
Easy pickings: | unset |
---|---|
Resolution: | → wontfix |
Status: | new → closed |
UI/UX: | unset |
Discussion with Alex at sprint: since this is not required by spec, and is already available in ConditionalGetMiddleware for those who want it, we don't want to go exhausting iterator responses by default in CommonMiddleware.
comment:10 by , 10 years ago
Has patch: | unset |
---|---|
Resolution: | wontfix |
Status: | closed → new |
Triage Stage: | Design decision needed → Accepted |
I'm reopening this ticket in the light of the introduction of explicit streaming responses, which removes the only objection to fixing this ticket.
I believe Content-Length
should be added for non-streaming responses. Their content is consumed upon assignment. The overhead of computing response length will be negligible.
comment:11 by , 9 years ago
Because some load balancers, such as Amazon "Elastic Load Balancing", require a content-length header django should at least document where the ConditionalGetMiddleware middleware may be required.
comment:13 by , 8 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:15 by , 8 years ago
Cc: | added |
---|
Patch