Refreshing of stale response assumed to always be successful

[gterzian]

Step 7.4 of https://fetch.spec.whatwg.org/#http-network-or-cache-fetch says to:

• If the revalidatingFlag is set and forwardResponse’s status is 304, then:
  • Update storedResponse’s header list using forwardResponse’s header list, as per the "Freshening Stored Responses upon Validation" chapter of HTTP Caching
  • Set response to storedResponse.

However, Freshening Stored Responses upon Validation is subject to successfully selecting a stored response based on strong/weak/no validators found in the headers of the 304 response.

Some cases contain a MUST NOT directive preventing the cache from using the 304 to update any stored responses, for example:

If none of the stored
responses contain the same strong validator, then the cache MUST
NOT use the new response to update any stored responses.

---

There are a few potential issues that can be identified:

1. Fetch assumes that the 304 forwardResponse will update the previously set storedResponse, while in fact the cache might have to select a different stored resource for update, based on the validators found in the 304 response.
2. Step 4.2 always does a Set response to storedResponse. , while in fact the cache might not have been able to refresh storedResponse, or any other cached resource.
3. If the cache fails to refresh any stored response based on the 304, it seems that fetch should do a "normal" network fetch of the resource, and such hook is not found in the algorithm. Instead step 7.5 says to Set response to forwardResponse. (if response is null), so if an implementation were to not update response as part of Step 4, due to a failure to select a resource for update, it would seem that the 304 would then be set to be the response.

It would seem that a solution could be:

a. making Step 7.4 conditional upon selecting a resource for update based on the the 304 response, and setting storedResponse to the optional result from that operation,
b. and if unsuccesful, the algorithm should make a new network request that is not a validation request, and set forwardResponse to the result of this new HTTP-network fetch.

Please let me know what you think.

[mnot]

SGTM

[gterzian]

Ok I will follow-up with a PR

[mnot]

I probably should have mentioned; for figuring out what to align to, you'll want to refer to this, not 7234.

[gterzian]

@mnot thanks for pointing that out. That doc seems to clarify in more details which headers to update.

So I'm trying to understand how to best phrase this in Fetch, also in the light of 4.3.3.

My first question: is it possible for a server to respond with a 304, but with a different strong validator than the one found in the request? Even if possible, should the cache actually do something in such a case? That is actually the case I was thinking about initially, and my hunch was that the cache in such a case(received a 304, but cannot find any matching stored response) should make a new request that is not a validation request(See the current wording in the PR).

However, looking more closely at 4.3.3, it seems that the server in such a case would instead respond with a full response, and the cache could then use it(and MAY replace the stored once).

And that seems already covered by Fetch, via step 7.5 of https://fetch.spec.whatwg.org/#http-network-or-cache-fetch.

So perhaps receiving a 304, but being unable to match it with a stored response for update, is an edge case and perhaps a bug on the server(which should really send a full response back if it is unable to send a 304 matching the validator found in the validation request), and not something we should address in fetch? It doesn't seem to be addressed in the cache spec either...

[gterzian]

One thing we can definitely do is flesh out 7.4 a bit more to align it with the https://httpwg.org/http-core/draft-ietf-httpbis-cache-latest.html#rfc.section.4.3.3, to handle the following case:

1. revalidating flag is set, but we get a "full response" => "the cache MUST use the full response to satisfy the request and MAY replace the stored response"
2. revalidating flag is set, but we get an 5xx back => "either forward this response to the requesting client, or act as if the server failed to respond. In the latter case, the cache MAY send a previously stored response"

And I think the "full response" seems to answer my previous question: a server should not send a 304 back if it would not match the validators found on the request. Although that is perhaps something the cache spec should address, essentially what to do with a 304 response that doesn't seem to match any of the stored responses, but I don't think it's something Fetch should address first.

[gterzian]

@annevk perhaps the most effective change would be to remove the reference to 304 in Step 7.4 of https://fetch.spec.whatwg.org/#http-network-or-cache-fetch, and instead write something like: "if revalidationFlag is set: set response to the result of handling forwardResponse according to https://httpwg.org/http-core/draft-ietf-httpbis-cache-latest.html#rfc.section.4.3.3"?

[annevk]

We should handle server errors in a standardized manner. Returning the 304 as-is in that case seems reasonable. I do think it makes sense for Fetch to mention 304 specifically, so it's clear when CORS checks and such happen relative to it being handled.

[gterzian]

Ok so on second thoughts, I think the fetch wording is actually pretty good as is. The 304 is handled explicitly, and the other cases are dealt with implicitly with Step 7.5, which would also cover the server error case(which would be forwarded as the response).

The edge case I mentioned initially(a 304 that does not match any stored response) doesn't seem to be handled by the http-cache spec yet, so I don't see how it could be handled explicitly in fetch either.

One thing we could do is maybe update the links to point to https://httpwg.org/http-core/draft-ietf-httpbis-cache-latest.html ?

[annevk]

How does that handle it? I think it's better to wait for that to be published as RFC, unless this is particularly pressing.

[gterzian]

Ok, should we then close this issue and the associated PR?

[annevk]

I think we should keep it open to track the issue where we set the revalidatingFlag and get a 304 but somehow not have a valid cached response. That needs addressing somehow and I don't think it can be fixed solely upstream given our current logic.

[mnot]

It's certainly possible for a 304 to contain a non-matching Etag, but the cache shouldn't update the response. Whether or not the client retries the request is up to it -- but usually it will, if it still wants a response. HTTP doesn't specify it because it's pretty generic, but I could see Fetch doing so (if you can get everyone to agree).

[mnot]

^ above comment made without refreshing the issue ...

[gterzian]

HTTP doesn't specify it because it's pretty generic, but I could see Fetch doing so (if you can get everyone to agree).

Ok thanks for clarifying, so then Fetch is indeed the right place to handle this(opposite of what I suggested earlier).

Whether or not the client retries the request is up to it -- but usually it will

Just to clarify "retrying" in this context, would it mean sending the initial request, but stripped of any re-validation headers(essentially switching to a "normal" request for the full resource)?

This is sort of what I tried to sketch over at #1041

[mnot]

Yep. The details are what headers to strip; you could just strip the offending one, or all of them.