Merge pull request #48337 from transumption/201810/nginx-etag

nginx: if root is in Nix store, use path's hash as ETag

nixos/tests/nginx.nix

@@ -1,18 +1,19 @@
 # verifies:
 #   1. nginx generates config file with shared http context definitions above
 #      generated virtual hosts config.
+#   2. whether the ETag header is properly generated whenever we're serving
+#      files in Nix store paths
 
-import ./make-test.nix ({ pkgs, ...} : {
+import ./make-test.nix ({ pkgs, ... }: {
   name = "nginx";
   meta = with pkgs.stdenv.lib.maintainers; {
     maintainers = [ mbbx6spp ];
   };
 
-  nodes = {
-    webserver =
-      { ... }:
-      { services.nginx.enable = true;
-        services.nginx.commonHttpConfig = ''
+  nodes = let
+    commonConfig = { pkgs, ... }: {
+      services.nginx.enable = true;
+      services.nginx.commonHttpConfig = ''
         log_format ceeformat '@cee: {"status":"$status",'
           '"request_time":$request_time,'
           '"upstream_response_time":$upstream_response_time,'
@@ -24,20 +25,61 @@ import ./make-test.nix ({ pkgs, ...} : {
           '"request":"$request",'
           '"http_referer":"$http_referer",'
           '"upstream_addr":"$upstream_addr"}';
+      '';
+      services.nginx.virtualHosts."0.my.test" = {
+        extraConfig = ''
+          access_log syslog:server=unix:/dev/log,facility=user,tag=mytag,severity=info ceeformat;
+          location /favicon.ico { allow all; access_log off; log_not_found off; }
         '';
-        services.nginx.virtualHosts."0.my.test" = {
-          extraConfig = ''
-            access_log syslog:server=unix:/dev/log,facility=user,tag=mytag,severity=info ceeformat;
-            location /favicon.ico { allow all; access_log off; log_not_found off; }
-          '';
-        };
       };
+      services.nginx.virtualHosts.localhost = {
+        root = pkgs.runCommand "testdir" {} ''
+          mkdir "$out"
+          echo hello world > "$out/index.html"
+        '';
+      };
+    };
+  in {
+    webserver = commonConfig;
+
+    newwebserver = { pkgs, lib, ... }: {
+      imports = [ commonConfig ];
+      services.nginx.virtualHosts.localhost = {
+        root = lib.mkForce (pkgs.runCommand "testdir2" {} ''
+          mkdir "$out"
+          echo hello world > "$out/index.html"
+        '');
+      };
+    };
   };
 
-  testScript = ''
-    startAll;
+  testScript = { nodes, ... }: let
+    newServerSystem = nodes.newwebserver.config.system.build.toplevel;
+    switch = "${newServerSystem}/bin/switch-to-configuration test";
+  in ''
+    my $url = 'http://localhost/index.html';
+
+    sub checkEtag {
+      my $etag = $webserver->succeed(
+        'curl -v '.$url.' 2>&1 | sed -n -e "s/^< [Ee][Tt][Aa][Gg]: *//p"'
+      );
+      $etag =~ s/\r?\n$//;
+      my $httpCode = $webserver->succeed(
+        'curl -w "%{http_code}" -X HEAD -H \'If-None-Match: '.$etag.'\' '.$url
+      );
+      chomp $httpCode;
+      die "HTTP code is not 304" unless $httpCode == 304;
+      return $etag;
+    }
 
     $webserver->waitForUnit("nginx");
     $webserver->waitForOpenPort("80");
+
+    subtest "check ETag if serving Nix store paths", sub {
+      my $oldEtag = checkEtag;
+      $webserver->succeed('${switch}');
+      my $newEtag = checkEtag;
+      die "Old ETag $oldEtag is the same as $newEtag" if $oldEtag eq $newEtag;
+    };
   '';
 })

pkgs/servers/http/nginx/generic.nix

@@ -1,5 +1,5 @@
 { stdenv, fetchurl, fetchpatch, openssl, zlib, pcre, libxml2, libxslt
-, gd, geoip
+, substituteAll, gd, geoip
 , withDebug ? false
 , withStream ? true
 , withMail ? false
@@ -75,7 +75,12 @@ stdenv.mkDerivation {
 
   preConfigure = (concatMapStringsSep "\n" (mod: mod.preConfigure or "") modules);
 
-  patches = stdenv.lib.optionals (stdenv.hostPlatform != stdenv.buildPlatform) [
+  patches = stdenv.lib.singleton (substituteAll {
+    src = ./nix-etag-1.15.4.patch;
+    preInstall = ''
+      export nixStoreDir="$NIX_STORE" nixStoreDirLen="''${#NIX_STORE}"
+    '';
+  }) ++ stdenv.lib.optionals (stdenv.hostPlatform != stdenv.buildPlatform) [
     (fetchpatch {
       url = "https://raw.githubusercontent.com/openwrt/packages/master/net/nginx/patches/102-sizeof_test_fix.patch";
       sha256 = "0i2k30ac8d7inj9l6bl0684kjglam2f68z8lf3xggcc2i5wzhh8a";

pkgs/servers/http/nginx/nix-etag-1.15.4.patch

@@ -0,0 +1,92 @@
+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(-)
+
+diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
+index c57ec00c..b7992de2 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)
+ {
+     ngx_table_elt_t           *etag;
+     ngx_http_core_loc_conf_t  *clcf;
++    u_char                    *real, *ptr1, *ptr2;
+ 
+     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)
+     etag->hash = 1;
+     ngx_str_set(&etag->key, "ETag");
+ 
+-    etag->value.data = ngx_pnalloc(r->pool, NGX_OFF_T_LEN + NGX_TIME_T_LEN + 3);
+-    if (etag->value.data == NULL) {
++    real = ngx_realpath(clcf->root.data, NULL);
++
++    if (real == NULL) {
+         etag->hash = 0;
+         return NGX_ERROR;
+     }
+ 
+-    etag->value.len = ngx_sprintf(etag->value.data, "\"%xT-%xO\"",
+-                                  r->headers_out.last_modified_time,
+-                                  r->headers_out.content_length_n)
+-                      - etag->value.data;
++    #define NIX_STORE_DIR "@nixStoreDir@"
++    #define NIX_STORE_LEN @nixStoreDirLen@
++
++    if (r->headers_out.last_modified_time == 1
++        && !ngx_strncmp(real, NIX_STORE_DIR, NIX_STORE_LEN)
++        && real[NIX_STORE_LEN] == '/'
++        && real[NIX_STORE_LEN + 1] != '\0')
++    {
++        ptr1 = real + NIX_STORE_LEN;
++        *ptr1 = '"';
++
++        ptr2 = (u_char *) ngx_strchr(ptr1, '-');
++
++        if (ptr2 == NULL) {
++            ngx_free(real);
++            etag->hash = 0;
++            return NGX_ERROR;
++        }
++
++        *ptr2++ = '"';
++        *ptr2 = '\0';
++
++        etag->value.len = ngx_strlen(ptr1);
++        etag->value.data = ngx_pnalloc(r->pool, etag->value.len);
++
++        if (etag->value.data == NULL) {
++            ngx_free(real);
++            etag->hash = 0;
++            return NGX_ERROR;
++        }
++
++        ngx_memcpy(etag->value.data, ptr1, etag->value.len);
++    } else {
++        etag->value.data = ngx_pnalloc(r->pool, NGX_OFF_T_LEN + NGX_TIME_T_LEN + 3);
++
++        if (etag->value.data == NULL) {
++            ngx_free(real);
++            etag->hash = 0;
++            return NGX_ERROR;
++        }
++
++        etag->value.len = ngx_sprintf(etag->value.data, "\"%xT-%xO\"",
++                                      r->headers_out.last_modified_time,
++                                      r->headers_out.content_length_n)
++                          - etag->value.data;
++    }
++
++    ngx_free(real);
+ 
+     r->headers_out.etag = etag;
+ 
+-- 
+2.19.0
+