Skip to content

Commit

Permalink
Merge Last-Modified fix for nginx (#76697)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aszlig committed Jan 2, 2020
2 parents 129c738 + ccf55be commit 845e928
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 13 deletions.
1 change: 1 addition & 0 deletions nixos/tests/all-tests.nix
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ in
nfs4 = handleTest ./nfs { version = 4; };
nghttpx = handleTest ./nghttpx.nix {};
nginx = handleTest ./nginx.nix {};
nginx-etag = handleTest ./nginx-etag.nix {};
nginx-sso = handleTest ./nginx-sso.nix {};
nix-ssh-serve = handleTest ./nix-ssh-serve.nix {};
nixos-generate-config = handleTest ./nixos-generate-config.nix {};
Expand Down
89 changes: 89 additions & 0 deletions nixos/tests/nginx-etag.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import ./make-test-python.nix {
name = "nginx-etag";

nodes = {
server = { pkgs, lib, ... }: {
networking.firewall.enable = false;
services.nginx.enable = true;
services.nginx.virtualHosts.server = {
root = pkgs.runCommandLocal "testdir" {} ''
mkdir "$out"
cat > "$out/test.js" <<EOF
document.getElementById('foobar').setAttribute('foo', 'bar');
EOF
cat > "$out/index.html" <<EOF
<!DOCTYPE html>
<div id="foobar">test</div>
<script src="test.js"></script>
EOF
'';
};

nesting.clone = lib.singleton {
services.nginx.virtualHosts.server = {
root = lib.mkForce (pkgs.runCommandLocal "testdir2" {} ''
mkdir "$out"
cat > "$out/test.js" <<EOF
document.getElementById('foobar').setAttribute('foo', 'yay');
EOF
cat > "$out/index.html" <<EOF
<!DOCTYPE html>
<div id="foobar">test</div>
<script src="test.js"></script>
EOF
'');
};
};
};

client = { pkgs, lib, ... }: {
virtualisation.memorySize = 512;
environment.systemPackages = let
testRunner = pkgs.writers.writePython3Bin "test-runner" {
libraries = [ pkgs.python3Packages.selenium ];
} ''
import os
import time
from selenium.webdriver import Firefox
from selenium.webdriver.firefox.options import Options
options = Options()
options.add_argument('--headless')
driver = Firefox(options=options)
driver.implicitly_wait(20)
driver.get('http://server/')
driver.find_element_by_xpath('//div[@foo="bar"]')
open('/tmp/passed_stage1', 'w')
while not os.path.exists('/tmp/proceed'):
time.sleep(0.5)
driver.get('http://server/')
driver.find_element_by_xpath('//div[@foo="yay"]')
open('/tmp/passed', 'w')
'';
in [ pkgs.firefox-unwrapped pkgs.geckodriver testRunner ];
};
};

testScript = { nodes, ... }: let
inherit (nodes.server.config.system.build) toplevel;
newSystem = "${toplevel}/fine-tune/child-1";
in ''
start_all()
server.wait_for_unit("nginx.service")
client.wait_for_unit("multi-user.target")
client.execute("test-runner &")
client.wait_for_file("/tmp/passed_stage1")
server.succeed(
"${newSystem}/bin/switch-to-configuration test >&2"
)
client.succeed("touch /tmp/proceed")
client.wait_for_file("/tmp/passed")
'';
}
18 changes: 5 additions & 13 deletions pkgs/servers/http/nginx/nix-etag-1.15.4.patch
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
From f6a978f024d01202f954483423af1b2d5d5159a6 Mon Sep 17 00:00:00 2001
From: Yegor Timoshenko <yegortimoshenko@riseup.net>
Date: Fri, 28 Sep 2018 03:27:04 +0000
Subject: [PATCH] If root is in Nix store, set ETag to its path hash

---
src/http/ngx_http_core_module.c | 56 +++++++++++++++++++++++++++++----
1 file changed, 50 insertions(+), 6 deletions(-)
This patch makes it possible to serve static content from Nix store paths, by
using the hash of the store path for the ETag header.

diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
index c57ec00c..b7992de2 100644
index cb49ef74..f88dc77c 100644
--- a/src/http/ngx_http_core_module.c
+++ b/src/http/ngx_http_core_module.c
@@ -1583,6 +1583,7 @@ ngx_http_set_etag(ngx_http_request_t *r)
Expand All @@ -19,7 +13,7 @@ index c57ec00c..b7992de2 100644

clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);

@@ -1598,16 +1599,61 @@ ngx_http_set_etag(ngx_http_request_t *r)
@@ -1598,16 +1599,62 @@ ngx_http_set_etag(ngx_http_request_t *r)
etag->hash = 1;
ngx_str_set(&etag->key, "ETag");

Expand Down Expand Up @@ -68,6 +62,7 @@ index c57ec00c..b7992de2 100644
+ }
+
+ ngx_memcpy(etag->value.data, ptr1, etag->value.len);
+ ngx_http_clear_last_modified(r);
+ } else {
+ etag->value.data = ngx_pnalloc(r->pool, NGX_OFF_T_LEN + NGX_TIME_T_LEN + 3);
+
Expand All @@ -87,6 +82,3 @@ index c57ec00c..b7992de2 100644

r->headers_out.etag = etag;

--
2.19.0

0 comments on commit 845e928

Please sign in to comment.