[PATCH] Upstream: prioritise Cache-Control over Expires

Sergey Kandaurov pluknet at nginx.com
Tue Jun 7 15:58:36 UTC 2022


> On 7 Jun 2022, at 01:09, Maxim Dounin <mdounin at mdounin.ru> wrote:
> 
> Hello!
> 
> On Mon, Jun 06, 2022 at 05:42:20PM +0400, Sergey Kandaurov wrote:
> 
>> On Sun, Apr 24, 2022 at 06:42:47PM +0300, Maxim Dounin wrote:
>>> Hello!
>>> 
>>> On Sun, Apr 24, 2022 at 07:55:17AM +0300, Maxim Dounin wrote:
>>> 
>>> [...]
>>> 
>>>> As far as I can tell, proper behaviour, assuming we parse cache 
>>>> control extensions independently of X-Accel-Expires, can be 
>>>> implemented by using just one flag.
>>> 
>>> No, that's wrong, as with just one flag it wouldn't be possible to 
>>> correctly disable caching of responses with:
>>> 
>>> Cache-Control: private
>>> Cache-Control: max-age=10
>>> 
>>> So it needs at least two flags.  Updated patch below, review and 
>>> testing appreciated.
>>> 
>>> # HG changeset patch
>>> # User Maxim Dounin <mdounin at mdounin.ru>
>>> # Date 1650814681 -10800
>>> #      Sun Apr 24 18:38:01 2022 +0300
>>> # Node ID 940ba4317a97c72d1ee6700cbf58a543fee04c7a
>>> # Parent  a736a7a613ea6e182ff86fbadcb98bb0f8891c0b
>>> Upstream: fixed X-Accel-Expires/Cache-Control/Expires handling.
>>> [..]
>> [..]
>> As the above note covers a corner case of X-Accel-Expires,
>> I believe it's fine to commit as is.
> 
> This patch doesn't try to change handling of X-Accel-Expires in 
> the past.
> 
> And, honestly, the only documented way to _disable_ caching is to 
> use 'X-Accel-Expires: 0'.  The 'X-Accel-Expires: @0' only 
> documented to set the time up to which the response may be cached 
> to 0 seconds the Epoch.  Expires and Cache-Control handling used 
> to disable caching in similar situations, yet this behaviour was 
> questioned more than once.  X-Accel-Expires never disabled caching 
> for absolute times in the past, and this is known to be used as a 
> workaround to cache a pre-expires response[1][2]. 
> 
> Pushed to http://mdounin.ru/hg/nginx.
> 
> [1] https://trac.nginx.org/nginx/ticket/1182
> [2] https://mailman.nginx.org/pipermail/nginx-ru/2013-November/052614.html
> 

Tests:


# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1654617432 -14400
#      Tue Jun 07 19:57:12 2022 +0400
# Node ID ed5fe35807f6c4853be1eb8ac429524e04ac5d67
# Parent  4f238efded81e00c635c146df6ecb585fa0907ec
Tests: added X-Accel-Expires tests.

diff --git a/proxy_xae.t b/proxy_xae.t
new file mode 100644
--- /dev/null
+++ b/proxy_xae.t
@@ -0,0 +1,160 @@
+#!/usr/bin/perl
+
+# (C) Sergey Kandaurov
+# (C) Nginx, Inc.
+
+# Tests for proxy Expires / Cache-Control / X-Accel-Expires preference.
+
+###############################################################################
+
+use warnings;
+use strict;
+
+use Test::More;
+
+BEGIN { use FindBin; chdir($FindBin::Bin); }
+
+use lib 'lib';
+use Test::Nginx;
+
+###############################################################################
+
+select STDERR; $| = 1;
+select STDOUT; $| = 1;
+
+my $t = Test::Nginx->new()->has(qw/http proxy cache rewrite map/)->plan(19);
+
+$t->write_file_expand('nginx.conf', <<'EOF');
+
+%%TEST_GLOBALS%%
+
+daemon off;
+
+events {
+}
+
+http {
+    %%TEST_GLOBALS_HTTP%%
+
+    proxy_cache_path   %%TESTDIR%%/cache  levels=1:2
+                       keys_zone=NAME:1m;
+
+    map $arg_e $e {
+        epoch  "Thu, 01 Jan 1970 00:00:01 GMT";
+        max    "Thu, 31 Dec 2037 23:55:55 GMT";
+    }
+
+    server {
+        listen       127.0.0.1:8080;
+        server_name  localhost;
+
+        location / {
+            proxy_pass   http://127.0.0.1:8081;
+            proxy_cache  NAME;
+
+            proxy_cache_background_update  on;
+
+            add_header  X-Cache-Status  $upstream_cache_status;
+        }
+
+        location /ignore {
+            proxy_pass   http://127.0.0.1:8081/return-xae;
+            proxy_cache  NAME;
+
+            proxy_ignore_headers  X-Accel-Expires;
+
+            add_header  X-Cache-Status  $upstream_cache_status;
+        }
+    }
+
+    server {
+        listen       127.0.0.1:8081;
+        server_name  localhost;
+
+        location / {
+            add_header  Expires          $e;
+            add_header  Cache-Control    $arg_c;
+            add_header  X-Accel-Expires  $arg_x;
+            add_header  X-Accel-Expires  $arg_xx;
+
+            return 204;
+        }
+
+        location /rev {
+            add_header  X-Accel-Expires  $arg_x;
+            add_header  Cache-Control    $arg_c;
+            add_header  Expires          $e;
+
+            return 204;
+        }
+    }
+}
+
+EOF
+
+$t->run();
+
+###############################################################################
+
+# Cache-Control is preferred over Expires, and
+# X-Accel-Expires is preferred over both Cache-Control and Expires,
+
+like(get('/?e=max'), qr/HIT/, 'Expires');
+like(get('/?c=max-age=60'), qr/HIT/, 'Cache-Control');
+like(get('/?x=60'), qr/HIT/, 'X-Accel-Expires');
+like(get('/?x=@2145902155'), qr/HIT/, 'X-Accel-Expires at');
+
+like(get('/ignore?x=60&ign=1'), qr/MISS/, 'X-Accel-Expires ignored');
+
+TODO: {
+local $TODO = 'not yet' unless $t->has_version('1.23.0');
+
+like(get('/?x=60&xx=0'), qr/HIT/, 'X-Accel-Expires duplicate');
+
+}
+
+# handling of caching parameters in Expires / Cache-Control / X-Accel-Expires
+
+like(get('/?e=max&c=no-cache'), qr/MISS/, 'Expires on CC off');
+like(get('/?e=max&x=0'), qr/MISS/, 'Expires on X-Accel-Expires off');
+like(get('/?c=max-age=60&x=0'), qr/MISS/, 'CC on X-Accel-Expires off');
+
+TODO: {
+local $TODO = 'not yet' unless $t->has_version('1.23.0');
+
+like(get('/?e=epoch&c=max-age=60'), qr/HIT/, 'Expires off CC on');
+like(get('/?e=epoch&x=60'), qr/HIT/, 'Expires off X-Accel-Expires on');
+like(get('/?c=no-cache&x=60'), qr/HIT/, 'CC off X-Accel-Expires on');
+
+}
+
+# and in the reversed order: X-Accel-Expires / Cache-Control / Expires
+
+like(get('/rev/?e=max&c=no-cache'), qr/MISS/, 'CC off Expires on');
+like(get('/rev?e=max&x=0'), qr/MISS/, 'X-Accel-Expires off Expires on');
+like(get('/rev?c=max-age=60&x=0'), qr/MISS/, 'X-Accel-Expires off CC on');
+
+like(get('/rev?e=epoch&c=max-age=60'), qr/HIT/, 'CC on Expires off');
+like(get('/rev?e=epoch&x=60'), qr/HIT/, 'X-Accel-Expires on Expires off');
+like(get('/rev?c=no-cache&x=60'), qr/HIT/, 'X-Accel-Expires on CC off');
+
+# Cache-Control caching extensions preserved after X-Accel-Expires
+
+TODO: {
+local $TODO = 'not yet' unless $t->has_version('1.23.0');
+
+my $time = time() - 1;
+like(get("/rev?c=max-age=60,stale-while-revalidate=60&x=\@$time"),
+	qr/STALE/, 'Cache-Control extensions after X-Accel-Expires');
+
+}
+
+###############################################################################
+
+sub get {
+	my ($uri) = @_;
+	http_get($uri);
+	http_get($uri);
+}
+
+###############################################################################

-- 
Sergey Kandaurov



More information about the nginx-devel mailing list