nginx: Clear Last-Modified if ETag is from store

[aszlig]

This is what I've suspected a while ago:

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 #48337 (comment)).

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 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 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.

Cc: @juskrey

[aszlig]

@GrahamcOfBorg test nginx-etag

[gebner]

I'm not sure I understand the issue completely. In the test, we construct two directories, /nix/store/aaaaaaaaaa-testdir and /nix/store/bbbbbbbbbb-testdir2, each containing an index.html and a test.js file. Then the following happens:

1. We start nginx with root /nix/store/aaaaaaaaaa-testdir.
2. Firefox requests /test.js and receives Last-Modified: 1 and ETag: aaaaaaaaaa
3. We change the nginx configuration to root /nix/store/bbbbbbbbbb-testdir2, which contains a test.js file with different content.
4. Firefox requests /test.js and receives Last-Modified: 1 and ETag: bbbbbbbbbb
5. The Last-Modified header did not change, so firefox uses the cached version of test.js. (Which is bad, since test.js now points to a different file.)

The PR then fixes this issue by removing the Last-Modified header, right?

However AFAICT this still doesn't fix caching completely: e.g. if testdir and testdir2 were both created by the same derivation. That is, we change the root from /nix/store/cccccccccc-combined/testdir to /nix/store/cccccccccc-combined/testdir2. Then nginx would always respond with ETag: cccccccccc no matter whether we're in testdir or testdir2 and firefox would still use the cached version.

[aszlig]

@gebner:

2. Firefox requests `/test.js` and receives `Last-Modified: 1` and `ETag: aaaaaaaaaa`

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 ETag is bbbbbbbbbb. So the fix simply strips the Last-Modified header.

The PR then fixes this issue by removing the Last-Modified header, right?

Correct.

However AFAICT this still doesn't fix caching completely: e.g. if testdir and testdir2 were both created by the same derivation. That is, we change the root from /nix/store/cccccccccc-combined/testdir to /nix/store/cccccccccc-combined/testdir2. Then nginx would always respond with ETag: cccccccccc no matter whether we're in testdir or testdir2 and firefox would still use the cached version.

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 /index.html from testdir first, realise you got the wrong path, change it to testdir2 (which also has an index.html) and then you'd end up getting cached content from the browser.

Do you know a practical example where something like this would happen more frequently?

[domenkozar]

Needs a backport for 19.09.

[aszlig]

Backported in f7bc988.