AnETag alone does not give the response a lifetime

Last-Modified doesn't either, does it?

TIL that if the status code iscacheable by default or a response without explicit freshness has been marked as explicitly cacheable (e.g., with apublic response directive), thenheuristic freshness based onLast-Modified may be applied.

Until now, I always thought this was a browser quirk.

Should the PR target 3.4?

I don't understand the change. When there is an ETag, we should sendprivate, must-revalidate, because that's what an ETag allows: revalidation.

no-cache is the default, which tells a cache that itmust revalidate the resource uponevery request.must-revalidate means that revalidation must occuronce the response has become stale.

As anETag alone does not define a lifetime, my suggestion was to switch fromno-cache tomust-revalidate only if a header is present that defines the lifetime (asExpires orCache-Control: max-age=...) or that allows for a heuristic expiration to be applied (Last-Modified is provided).

Nit-picking, really.

I agree the RFC might have chosen more intention-revealing names.

An ETag without a lifetime is considered as having a zero lifetime by browsers AFAIK.
I don't think no-cache is appropriate: a conditional request is possible today and works, whereas this change would kill this conditional request.

No, it would not.

If the lifetime is considered to be zero,no-cache andmust-revalidate would have the same effect, namely that a revalidation has to happen right from the start/after 0 seconds.

no-store: Prohibits caching, thus no revalidation/conditional requests.

no-cache: Response can be cached, but must be revalidated every time.

must-revalidate: Response may be used without validation as long as it is fresh; must be revalidated afterwards and must not be used when stale.

Oh, so no-cache <=> must-revalidate,maxage=0 got it. Works then.

Tests do not pass apparently.

Failing tests come fromResponseCacheStrategy (here).

With the change from this PR, we now haveCache-Control: no-cache in some cases where previously we hadCache-Control: must-revalidate.

If this change applies to an ESI subrequest,ResponseCacheStrategy now decides (here) that the resulting, ESI-tags-merged response can no longer be cached using the expiration model (here).

It thus adds anExpires header and sets it to the responseDate. This is where the tests break: Now the lifetime is0, previously it wasnull.

I need to think through theResponseCacheStrategy behaviour because itfeels as if there might be a bug. The change here should not make a difference for the decision if something is cacheable or not.

Second, I'll try to remove theExpires header which IMO is not really necessary; not having it might already make tests pass again.

TL;DR: Working on it ;-)

Maybe@nicolas-grekas can confirm that thedeps=high build failure is due to the change being in one component, the fix for the test in another one?

Thank you@mpdude.