Opened 8 years ago
Closed 8 years ago
#27344 closed Bug (fixed)
ConditionalGetMiddleware should only operate on GET requests
Reported by: | Kevin Christopher Henry | Owned by: | Kevin Christopher Henry |
---|---|---|---|
Component: | HTTP handling | Version: | 1.10 |
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 )
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 pointed out in the specification:
Although conditional request header fields are defined as being usable with the
HEAD
method (to keepHEAD
's semantics consistent with those ofGET
), there is no point in sending a conditionalHEAD
because a successful response is around the same size as a 304 (Not Modified) response and more useful than a 412 (Precondition Failed) response.
Change History (4)
comment:1 by , 8 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Type: | Uncategorized → Bug |
comment:2 by , 8 years ago
Triage Stage: | Unreviewed → Ready for checkin |
---|
comment:3 by , 8 years ago
Description: | modified (diff) |
---|---|
Summary: | ConditionalGetMiddleware should not operate on unsafe requests → ConditionalGetMiddleware should only operate on GET requests |
PR