#14560 closed (fixed)
UpdateCacheMiddleware does not save responses for HEAD requests
| Reported by: | Maxim Bublis | Owned by: | nobody |
|---|---|---|---|
| Component: | Core (Cache system) | Version: | dev |
| Severity: | Keywords: | cache HEAD middleware | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
If you are sending HEAD request to your application, response won't be cached.
But if you are sending GET request first, response will be cached and following HEAD request will be answered directly from cache.
According to the comment in UpdateCacheMiddleware's process_response() method, this behaviour depends on inexistent HTTPMiddleware. Due to the fact that response content for HEAD requests is throwed away by response fixers after all middlewares are applied, it could be safely dropped.
Patch that fixes caching of HEAD requests is attached to this ticket.
Attachments (1)
Change History (7)
comment:1 by , 14 years ago
| Keywords: | middleware added |
|---|---|
| Needs tests: | set |
| Patch needs improvement: | set |
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 14 years ago
While writing patch I was guided by that comment. I don't think it is really necessary to split caching logic for GET and HEAD requests because response body for HEAD requests are always throwing away in WSGIHandler. If it is still necessary I could provide better patch.
comment:3 by , 14 years ago
Oh, I didn't see the comments, but nonetheless - it's not mentioned in the docs so we cannot rely on that. I am not worried about using a GET response to answer a HEAD request, that should work (for the reasons you mention) - I am worried about people optimizing their view code (skipping expensive template rendering for HEAD requests) and the middleware using HEAD responses to answer GET request.
The ideal patch would:
- change this comment to say that GET and HEAD responses' headers must be the same
- add this to the docs
- provide tests
- do all of the above
by , 14 years ago
| Attachment: | fix_head_cache.diff added |
|---|
Patch that fixes caching of HEAD requests, provides tests and docs.
comment:4 by , 14 years ago
| Needs tests: | unset |
|---|---|
| Patch needs improvement: | unset |
If we want to cache HEAD requests, we need to store it as a separate cache key since view can (with reasonable use-cases) render different body output for HEAD and GET requests. It is probably OK to storing GET result in cache and use that even for HEAD requests (especially since that is current behavior), but we have to avoid it the other way around.
So if the request being stored in cache is HEAD, store it under different key and when looking for HEAD request in
FetchFromCacheMiddleware, look at both keys (either usingget_many()or two cache requests, depending on what's faster for memcached).