Code

Opened 7 years ago

Closed 3 years ago

#6173 closed New feature (fixed)

Cache middleware should cache HEAD requests

Reported by: eratothene Owned by: arien
Component: Core (Cache system) Version: master
Severity: Normal Keywords: http
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX:

Description

When conditional get middleware was responsible for HEAD request, the side effects of incorrect middleware ordering could cause striped responses to be cached that is why HEAD requests, where not cached before.
Now HEAD requests content stripping is guarunteed to came after every middleware (applied in responce fixes) so it is safe to cache HEAD request now.

Attachments (1)

cache.patch (832 bytes) - added by eratothene 7 years ago.

Download all attachments as: .zip

Change History (6)

Changed 7 years ago by eratothene

comment:1 Changed 7 years ago by eratothene

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Comment on why I have deleted the method check completely: request method is checked in process_request, hence there is no need to check it again.

comment:2 Changed 7 years ago by arien

  • Keywords http added
  • Owner changed from nobody to arien
  • Patch needs improvement set
  • Status changed from new to assigned
  • Summary changed from [PATCH] Cache midleware should cache HEAD requests to Cache middleware should cache HEAD requests
  • Triage Stage changed from Unreviewed to Accepted

Yeah, that'd be nice to have.

One note about the patch: instead of removing the check for the HTTP method, IMHO it'd be better to factor out the logic for deciding if the cache could be used (the not request.method in ('GET', 'HEAD') or request.GET in process_request) and use that in both process_request and process_response.

comment:3 Changed 7 years ago by arien

  • Patch needs improvement unset

Hmm, never mind the last comment; I think I was looking at an earlier version of your patch(?). request._cache_update_cache would now be the central point deciding whether or not to use the cache.

comment:4 Changed 3 years ago by gabrielhurley

  • Severity set to Normal
  • Type set to New feature

comment:5 Changed 3 years ago by julien

  • Easy pickings unset
  • Resolution set to fixed
  • Status changed from assigned to closed

3 years later, this is no longer relevant as CacheMiddleware does not discriminate between request methods any more.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.