nginx: Clear Last-Modified if ETag is from store #76697
Conversation
This is what I've suspected a while ago[1]: > Heads-up everyone: After testing this in a few production instances, > it seems that some browsers still get cache hits for new store paths > (and changed contents) for some reason. I highly suspect that it might > be due to the last-modified header (as mentioned in [2]). > > Going to test this with last-modified disabled for a little while and > if this is the case I think we should improve that patch by disabling > last-modified if serving from a store path. Much earlier[2] when I reviewed the patch, I wrote this: > Other than that, it looks good to me. > > However, I'm not sure what we should do with Last-Modified header. > From RFC 2616, section 13.3.4: > > - If both an entity tag and a Last-Modified value have been > provided by the origin server, SHOULD use both validators in > cache-conditional requests. This allows both HTTP/1.0 and > HTTP/1.1 caches to respond appropriately. > > I'm a bit nervous about the SHOULD here, as user agents in the wild > could possibly just use Last-Modified and use the cached content > instead. Unfortunately, I didn't pursue this any further back then because @pbogdan noted[3] the following: > Hmm, could they (assuming they are conforming): > > * If an entity tag has been provided by the origin server, MUST > use that entity tag in any cache-conditional request (using If- > Match or If-None-Match). Since running with this patch in some deployments, I found that both Firefox and Chrome/Chromium do NOT re-validate against the ETag if the Last-Modified header is still the same. So I wrote a small NixOS VM test with Geckodriver to have a test case which is closer to the real world and I indeed was able to reproduce this. Whether this is actually a bug in Chrome or Firefox is an entirely different issue and even IF it is the fault of the browsers and it is fixed at some point, we'd still need to handle this for older browser versions. Apart from clearing the header, I also recreated the patch by using a plain "git diff" with a small description on top. This should make it easier for future authors to work on that patch. [1]: NixOS#48337 (comment) [2]: NixOS#48337 (comment) [3]: NixOS#48337 (comment) Signed-off-by: aszlig <aszlig@nix.build>
aszlig
added
the
9.needs: port to stable
|
@GrahamcOfBorg test nginx-etag |
|
I'm not sure I understand the issue completely. In the test, we construct two directories,
The PR then fixes this issue by removing the However AFAICT this still doesn't fix caching completely: e.g. if |
Yes, but this is exactly what this is fixing, because this was the behaviour prior to the fix and causes Firefox to load it from cache even if
Correct.
This is correct, however in practice something like this should rarely (if at all) happen and in almost all cases this would result in a different store path. The only thing that I could think of right now, is if you got the wrong directory, run into a HTTP 404 and fix the root directory... however in this case no caching would be involved. So I think this is only a problem if you request something like Do you know a practical example where something like this would happen more frequently? |
This fixes the patch for nginx to clear the Last-Modified header if a static file is served from the Nix store. So far we only used the ETag from the store path, but if the Last-Modified header is always set to "Thu, 01 Jan 1970 00:00:01 GMT", Firefox and Chrome/Chromium seem to ignore the ETag and simply use the cached content instead of revalidating. Alongside the fix, this also adds a dedicated NixOS VM test, which uses WebDriver and Firefox to check whether the content is actually served from the browser's cache and to have a more real-world test case.
|
Backported in f7bc988. |
This is what I've suspected a while ago:
Much earlier when I reviewed the patch, I wrote this:
Unfortunately, I didn't pursue this any further back then because @pbogdan noted the following:
Since running with this patch in some deployments, I found that both Firefox and Chrome/Chromium do NOT re-validate against the ETag if the
Last-Modifiedheader is still the same.So I wrote a small NixOS VM test with Geckodriver to have a test case which is closer to the real world and I indeed was able to reproduce this.
Whether this is actually a bug in Chrome or Firefox is an entirely different issue and even IF it is the fault of the browsers and it is fixed at some point, we'd still need to handle this for older browser versions.
Apart from clearing the header, I also recreated the patch by using a plain
git diffwith a small description on top. This should make it easier for future authors to work on that patch.Cc: @juskrey