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 Kevin Christopher Henry)

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 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.

Change History (4)

comment:1 by Kevin Christopher Henry, 8 years ago

Has patch: set
Owner: changed from nobody to Kevin Christopher Henry
Type: UncategorizedBug

comment:2 by Tim Graham, 8 years ago

Triage Stage: UnreviewedReady for checkin

comment:3 by Kevin Christopher Henry, 8 years ago

Description: modified (diff)
Summary: ConditionalGetMiddleware should not operate on unsafe requestsConditionalGetMiddleware should only operate on GET requests

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

Resolution: fixed
Status: assignedclosed

In 2327fad5:

Fixed #27344 -- Made ConditionalGetMiddleware only process GET requests.

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