Prioritize `X-Accel-Expires` even if specific Cache-Control flags

[yugo-horie.jocdn.co.jp@…]

We also have written the same issue in nginx forum.
​https://forum.nginx.org/read.php?2,219520,293374#msg-293374
it seems not to override entirely Cache-Control by X-Accel-Expires. And It has differential behavor according to order of upstream header.

We introduced the new flag in upstream.h to identify whether the specific Cache-Control flags (such as no-store, no-cache, private, stale-while-revalidate, stale-if-error) has already processed before upstream_process_accel_expires, and state of u-> cacheable is also not change ​unnecessarily.

diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c
index ded833c..5605e28 100644
--- a/src/http/ngx_http_upstream.c
+++ b/src/http/ngx_http_upstream.c
@@ -4747,6 +4747,7 @@ ngx_http_upstream_process_cache_control(ngx_http_request_t *r,
         || ngx_strlcasestrn(start, last, (u_char *) "private", 7 - 1) != NULL)
     {
         u->cacheable = 0;
+        u->prior_accel_expire = 1;
         return NGX_OK;
     }

@@ -4772,6 +4773,7 @@ ngx_http_upstream_process_cache_control(ngx_http_request_t *r,
             }

             u->cacheable = 0;
+            u->prior_accel_expire = 1;
             return NGX_OK;
         }

@@ -4800,6 +4802,7 @@ ngx_http_upstream_process_cache_control(ngx_http_request_t *r,
             }

             u->cacheable = 0;
+            u->prior_accel_expire = 1;
             return NGX_OK;
         }

@@ -4823,6 +4826,7 @@ ngx_http_upstream_process_cache_control(ngx_http_request_t *r,
             }

             u->cacheable = 0;
+            u->prior_accel_expire = 1;
             return NGX_OK;
         }

@@ -4897,6 +4901,10 @@ ngx_http_upstream_process_accel_expires(ngx_http_request_t *r,
     if (r->cache == NULL) {
         return NGX_OK;
     }
+    if (u->prior_accel_expire) {
+        u->prior_accel_expire = 0;
+        u->cacheable = 1;
+    }

     len = h->value.len;
     p = h->value.data;
diff --git a/src/http/ngx_http_upstream.h b/src/http/ngx_http_upstream.h
index 3db7b06..59710c1 100644
--- a/src/http/ngx_http_upstream.h
+++ b/src/http/ngx_http_upstream.h
@@ -386,6 +386,7 @@ struct ngx_http_upstream_s {

     unsigned                         store:1;
     unsigned                         cacheable:1;
+    unsigned                         prior_accel_expire:1;
     unsigned                         accel:1;
     unsigned                         ssl:1;
 #if (NGX_HTTP_CACHE)

if you wanna be not to introduce new flag, we also prepare another patch for this to change .

@@ -4904,18 +4912,15 @@ ngx_http_upstream_process_accel_expires(ngx_http_request_t *r,
     if (p[0] != '@') {
         n = ngx_atoi(p, len);

-        switch (n) {
-        case 0:
+        if (n == 0) {
             u->cacheable = 0;
-            /* fall through */
-
-        case NGX_ERROR:
-            return NGX_OK;
-
-        default:
-            r->cache->valid_sec = ngx_time() + n;
+        } else if (n > 0) {
+            u->cacheable = 1;
+        } else if (n == NGX_ERROR) {
             return NGX_OK;
         }
+        r->cache->valid_sec = ngx_time() + n;
+        return NGX_OK;
     }

[Maxim Dounin]

Please see ​Contributing Changes for a recommended way to submit patches. Trac isn't the best place to review patches.

Overall, the approach looks too fragile. Further, it doesn't seem to solve stale-while-revalidate and stale-if-error times, which also behave differently depending on the headers order, as well as similar problem with Expires vs. Cache-Control and non-cacheable flag.

Closing this as a duplicate of #964, which is basically about the same issue in Expires vs. Cache-Control.