[Django] #27344: ConditionalGetMiddleware should not operate on unsafe requests
Django
-----------------------------------------+----------------------
Reporter: Kevin Christopher Henry | Owner: nobody
Type: Uncategorized | Status: assigned
Component: HTTP handling | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------------+----------------------
With unsafe methods (`PUT`, etc.) the appropriate conditional response
would be a 412 Precondition Failed response, which should prevent the
request from being carried out. But by definition
`ConditionalGetMiddleware` acts after the response has been generated, so
it’s too late. The PR below includes a regression test where the
middleware inappropriately changes the response to a 412 after applying
the unsafe operation.
--
Ticket URL: <https://code.djangoproject.com/ticket/27344>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Kevin Christopher | Owner: Kevin
Henry | Christopher Henry
Type: Bug | Status: assigned
Component: HTTP handling | Version: 1.10
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Kevin Christopher Henry):
* needs_better_patch: => 0
* needs_tests: => 0
* owner: nobody => Kevin Christopher Henry
* needs_docs: => 0
* has_patch: 0 => 1
* type: Uncategorized => Bug
Comment:
[https://github.com/django/django/pull/7390 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/27344#comment:1>
Django
Reporter: Kevin Christopher | Owner: Kevin
Henry | Christopher Henry
Component: HTTP handling | Version: 1.10
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Unreviewed => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/27344#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Kevin Christopher | Owner: Kevin
Henry | Christopher Henry
Component: HTTP handling | Version: 1.10
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Kevin Christopher Henry:
Old description:
> With unsafe methods (`PUT`, etc.) the appropriate conditional response
> would be a 412 Precondition Failed response, which should prevent the
> request from being carried out. But by definition
> `ConditionalGetMiddleware` acts after the response has been generated, so
> it’s too late. The PR below includes a regression test where the
> middleware inappropriately changes the response to a 412 after applying
> the unsafe operation.
New description:
With unsafe methods (`PUT`, etc.) the appropriate conditional response
would be a 412 Precondition Failed response, which should prevent the
request from being carried out. But by definition
`ConditionalGetMiddleware` acts after the response has been generated, so
it’s too late. The PR below includes a regression test where the
middleware inappropriately changes the response to a 412 after applying
the unsafe operation.
`ConditionalGetMiddleware` is not suitable for `HEAD` requests either.
`HEAD` responses should return the same headers (including the `ETag`) as
the corresponding `GET` response, but `ConditionalGetMiddleware` will only
see the empty response body of the `HEAD` response and so will compute the
wrong `ETag`. Trying to compare `ETags` in this situation is also
pointless, as [https://tools.ietf.org/html/rfc7232#section-5 pointed out]
in the specification:
> Although conditional request header fields are defined as being usable
with the `HEAD` method (to keep `HEAD`'s semantics consistent with those
of `GET`), there is no point in sending a conditional `HEAD` because a
successful response is around the same size as a 304 (Not Modified)
response and more useful than a 412 (Precondition Failed) response.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/27344#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Kevin Christopher | Owner: Kevin
Henry | Christopher Henry
Component: HTTP handling | Version: 1.10
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"2327fad54e334119f2561ddddf52e5af4bb14d41" 2327fad5]:
{{{
#!CommitTicketReference repository=""
revision="2327fad54e334119f2561ddddf52e5af4bb14d41"
Fixed #27344 -- Made ConditionalGetMiddleware only process GET requests.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27344#comment:4>