Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#26052 closed Cleanup/optimization (fixed)

Consider removing conditional_content_removal

Reported by: Aymeric Augustin Owned by: Susan Tan
Component: HTTP handling Version: 1.9
Severity: Normal Keywords:
Cc: 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 (last modified by Tim Graham)

It's the last survivor of the "response_fixes" and I think it should go away. Its origin is #5898.

Graham Dumpleton has described better than I would why frameworks should leave the task stripping the body of HEAD requests to servers: http://blog.dscpl.com.au/2009/10/wsgi-issues-with-http-head-requests.html (first five paragraphs).

We should check whether mod_wsgi, gunicorn and uwsgi properly strip the body of HEAD requests before proceeding. We should also move that behavior into runserver -- that is, on the other side of the WSGI.

Change History (15)

comment:1 by Tim Graham, 8 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Akshesh Doshi, 8 years ago

I would like to work on this ticket. Can anyone please guide me exactly what changes would be required in the code.

Thanks

Last edited 8 years ago by Akshesh Doshi (previous) (diff)

comment:3 by Susan Tan, 8 years ago

Owner: changed from nobody to Susan Tan
Status: newassigned

comment:4 by Tim Graham, 8 years ago

Has patch: set

comment:5 by Tim Graham, 8 years ago

Patch needs improvement: set

I think the PR needs improvement as described in the ticket description, "mod_wsgi, gunicorn and uwsgi properly strip the body of HEAD requests before proceeding. We should also move that behavior into runserver."

comment:6 by Tim Graham, 8 years ago

Description: modified (diff)

I observed that gunicorn and runserver (based on Python's server) don't send content for HEAD requests. I didn't check mod_wsgi or uwsgi since they are marginally more complicated to setup a test project. If we go ahead and remove this, one place where developers might see a difference would be in the test client. Do you think it's worth trying to keep it in that context? I lean toward simply removing it and reacting if anyone complains.

comment:7 by Tim Graham, 8 years ago

Some data about the rest of the behavior of conditional_content_removal (discarding the content of responses with 1xx, 204, and 304 status codes):

Python's BaseHTTPRequestHandler (which runserver uses) also has discards the response content for 204 and 304 status codes. I observed the same behavior from gunicorn as well.

I also don't get any response data on HTTP 1xx responses regardless of whether or not conditional_content_removal is active.

comment:8 by Claude Paroz, 8 years ago

I tested the HEAD behaviour of mod_wsgi without conditional_content_removal and confirms it strips the content.

If it's not too hard to simulate this behavior in the test client, I think we should do it at that level.

comment:9 by Tim Graham, 8 years ago

Patch needs improvement: unset

Okay, here's a PR based on that suggestion.

comment:10 by Claude Paroz, 8 years ago

Triage Stage: AcceptedReady for checkin

comment:11 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In bb0b4b70:

Fixed #26052 -- Moved conditional_content_removal() processing to the test client.

in reply to:  11 comment:12 by Nick Pope, 8 years ago

Tim,

I think that some documentation might need a little rewording after this change:

https://github.com/django/django/blob/bb0b4b7/docs/topics/http/decorators.txt#L47-L52

comment:13 by Tim Graham, 8 years ago

Thanks for pointing that out. I think we could replace "Django will automatically" with "Most web servers" or "Web servers should". Did you have anything else in mind?

Last edited 8 years ago by Tim Graham (previous) (diff)

comment:14 by Tim Graham <timograham@…>, 8 years ago

In baf3ec2:

Refs #26052 -- Corrected a sentence for conditional_content_removal() removal.

comment:15 by Ed Morley, 7 years ago

Python's BaseHTTPRequestHandler (which runserver uses) also has ​discards the response content for 204 and 304 status codes

Unfortunately the linked BaseHTTPRequestHandler handling appears to just be for error conditions (send_error()) so this change caused runserver.py to start returning a body for HTTP 200 HEAD requests.

I've filed #28054.

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