Repository navigation
cache: only store GET responses and bypass non-cacheable methods - #4375
Open
Ramya-9353 wants to merge 1 commit into
Open
Ramya-9353 wants to merge 1 commit into
Ramya-9353 wants to merge 1 commit into
Conversation
The cache key does not include the request method and the store paths did not check which method produced the response. A cold OPTIONS request stored the backend's body-less reply under the GET key, and a HEAD request during the stale-while-revalidate window made the background revalidation store a body-less 200 with the original Content-Length, so later GETs for the same URL were served an empty body until the TTL expired. OPTIONS requests were also answered from the cached GET representation. Per RFC 9111 §3 and §4 only GET and HEAD are cacheable. Bypass the cache for methods that are neither cacheable nor unsafe, forward HEAD misses instead of coalescing them, and persist only responses fetched with GET. Signed-off-by: Ramya Eliger <ramya@digiscrypt.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Repro: on a route with
cache("5m", "15s", "30s"), sendOPTIONS /index.htmlagainst a cold key and thenGET /index.html: the GET is served as a HIT withContent-Length: 0, the backend'sAllowheader and no body. Likewise aHEAD /pagewhile the GET entry is inside its stale-while-revalidate window makes the background revalidation store a body-less 200 carrying the originalContent-Length, after which every GET for that URL fails withunexpected EOFuntil the TTL expires. Both were reproduced throughproxytestagainst anhttptestbackend on master.Cause: the cache key omits the request method and none of the store paths (
Response(),coalesce(),doRevalidate()) check which method fetched the response, so body-less OPTIONS and HEAD representations are persisted under the GET key and OPTIONS requests are answered from the cached GET entry, which also breaks CORS preflights on cached routes.Fix: bypass the cache in
Request()for methods that are neither GET/HEAD nor unsafe, forward HEAD misses instead of coalescing them, and letstoreDecision()persist only responses fetched with GET, which also covers revalidation (RFC 9111 §3, §4). Regression tests cover the cold HEAD, the OPTIONS/TRACE/PROPFIND bypass and the store gate; the filter docs now state the method behaviour. The guards do not overlap with the open cache PRs #4369 to #4372, none of which touch method handling.