Skip to content

Excessive caching of some response cases, and lack of caching of some others #4334

@fredericDelaporte

Description

@fredericDelaporte

Bug Description

The cache interceptor may cache responses it should not:

  • Temporary redirect responses (status 307) without explicit caching directive, if the cacheByDefault option is set to a positive number. 307 is not a heuristically cacheable status code, it should not be cached without an explicit response directive telling to do so.
  • 206 responses (Partial Content) can be cached since cache: allow caching heuristically cacheable error status codes #4318, but they should be cached only if the cache "understand" their semantic, which does not look to be the case. In short, it should at least include the Range request header in the cache key, but I have not found code which would do that. And my test shows the trouble. See Storing Incomplete Responses.

Some cacheable case remains not cached, while I see no harm in caching them:

  • There are still some cases of cacheable response statuses that are not cached despite having an explicit caching directive, like 302 Found. In short, all non heuristically cacheable statuses excepted 307 are never cached, although final ones can be cached if there is an explicit caching directive in the response.
    (This point is not an implementation bug as of the RFC9111, since caching them is only optional, as all caching. But why not caching them?)
    Doing this point should take care of not caching 304 responses, unless handling their semantic, meaning, including if-none-match/if-modified-since request headers in their cache key.

Reproducible By

  • Setup a cache interceptor with the cacheByDefault set to 60, for example.
    Query twice an endpoint replying a 307 without an explicit caching directive in less than 60 seconds.
    Server is hit only once.
  • Query with a range request an endpoint replying the partial content with a caching directive allowing to cache the response.
    Query it again but with a different range before the cache expires.
    The partial content for the first request will be served to the second call.
  • Query an endpoint replying a 302 with an explicit caching directive allowing to cache it.
    Query it again before the cache expires.
    The server will be hit twice instead of only once.

Expected Behavior

  • Do not cache non heuristically cacheable responses without an explicit cache directive.
  • Do not cache cacheable responses if their semantic is not properly handled by the cache (206, 304).
  • Cache other cacheable responses.

Environment

Windows 11, Node 22.16, testing directly from Undici sources.

Additional context

See also #4318 (comment).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions