Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: NixOS/nixpkgs
base: 129c73802fdd
Choose a base ref
...
head repository: NixOS/nixpkgs
compare: 845e92835d90
Choose a head ref
  • 2 commits
  • 3 files changed
  • 1 contributor

Commits on Dec 30, 2019

  1. nginx: Clear Last-Modified if ETag is from store

    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]: #48337 (comment)
    [2]: #48337 (comment)
    [3]: #48337 (comment)
    
    Signed-off-by: aszlig <aszlig@nix.build>
    aszlig committed Dec 30, 2019
    Copy the full SHA
    ccf55be View commit details
    Browse the repository at this point in the history

Commits on Jan 2, 2020

  1. Merge Last-Modified fix for nginx (#76697)

    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.
    aszlig committed Jan 2, 2020
    Copy the full SHA
    845e928 View commit details
    Browse the repository at this point in the history