Opened 10 years ago

Closed 9 years ago

Last modified 8 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, 10 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Akshesh Doshi, 10 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 10 years ago by Akshesh Doshi (previous) (diff)

comment:3 by Susan Tan, 9 years ago

Owner: changed from nobody to Susan Tan
Status: newassigned

comment:4 by Tim Graham, 9 years ago

Has patch: set

comment:5 by Tim Graham, 9 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, 9 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, 9 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, 9 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, 9 years ago

Patch needs improvement: unset

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

comment:10 by Claude Paroz, 9 years ago

Triage Stage: AcceptedReady for checkin

comment:11 by Tim Graham <timograham@…>, 9 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, 9 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, 9 years ago

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

Version 0, edited 9 years ago by Tim Graham (next)

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

In baf3ec2:

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

comment:15 by Ed Morley, 8 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