[Django] #26052: Consider removing conditional_content_removal
Django
------------------------------------------------+------------------------
Reporter: aaugustin | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: HTTP handling | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
It's the last survivor of the "response_fixes" and I think it should go
away.
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26052>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
--------------------------------------+------------------------------------
Reporter: aaugustin | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: HTTP handling | Version: 1.9
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/26052#comment:1>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: HTTP handling | Version: 1.9
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by akki):
I would like to work on this ticket. Can anyone please guide me exactly
what changes would be required.
Thanks
--
Ticket URL: <https://code.djangoproject.com/ticket/26052#comment:2>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: assigned
Component: HTTP handling | Version: 1.9
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by susan):
* owner: nobody => susan
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/26052#comment:3>
Django
--------------------------------------+------------------------------------
Reporter: aaugustin | Owner: susan
Type: Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/6401 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/26052#comment:4>
Django
--------------------------------------+------------------------------------
Reporter: aaugustin | Owner: susan
Type: Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
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`."
--
Ticket URL: <https://code.djangoproject.com/ticket/26052#comment:5>
Django
--------------------------------------+------------------------------------
Reporter: aaugustin | Owner: susan
Type: Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Old description:
> It's the last survivor of the "response_fixes" and I think it should go
> away.
>
> 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.
New description:
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.
--
Comment (by timgraham):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26052#comment:6>
Django
--------------------------------------+------------------------------------
Reporter: aaugustin | Owner: susan
Type: Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
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
[https://github.com/python/cpython/blob/d616dc4ea9d63d9a9b52a95b887b9a49859092e8/Lib/http/server.py#L462-L466
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26052#comment:7>
Django
--------------------------------------+------------------------------------
Reporter: aaugustin | Owner: susan
Type: Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26052#comment:8>
Django
--------------------------------------+------------------------------------
Reporter: aaugustin | Owner: susan
Type: Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 1 => 0
Comment:
Okay, here's a [https://github.com/django/django/pull/6505 PR] based on
that suggestion.
--
Ticket URL: <https://code.djangoproject.com/ticket/26052#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: aaugustin | Owner: susan
Type: | Status: assigned
Cleanup/optimization |
Component: HTTP handling | Version: 1.9
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/26052#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: aaugustin | Owner: susan
Cleanup/optimization |
Component: HTTP handling | Version: 1.9
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"bb0b4b705b508451567bcada9106b91b8fca9e86" bb0b4b70]:
{{{
#!CommitTicketReference repository=""
revision="bb0b4b705b508451567bcada9106b91b8fca9e86"
Fixed #26052 -- Moved conditional_content_removal() processing to the test
client.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26052#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: aaugustin | Owner: susan
Type: | Status: closed
Cleanup/optimization |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by pope1ni):
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
--
Ticket URL: <https://code.djangoproject.com/ticket/26052#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: aaugustin | Owner: susan
Type: | Status: closed
Cleanup/optimization |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Thanks for pointing that out. I think we could replace "Django will
automatically" with "Most web servers". Did you have anything else in
mind?
--
Ticket URL: <https://code.djangoproject.com/ticket/26052#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: aaugustin | Owner: susan
Type: | Status: closed
Cleanup/optimization |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"baf3ec2e296a900a48181e2924753fef2e123731" baf3ec2]:
{{{
#!CommitTicketReference repository=""
revision="baf3ec2e296a900a48181e2924753fef2e123731"
Refs #26052 -- Corrected a sentence for conditional_content_removal()
removal.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26052#comment:14>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization |
Component: HTTP handling | Version: 1.9
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Ed Morley):
> 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26052#comment:15>