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.
ngx_http_upstream_process_cache_control in ngx_http_upstream.c

4738 if (r->cache->valid_sec != 0 && u->headers_in.x_accel_expires != NULL) {
4739 return NGX_OK;
4740 }
4741
4742 start = h->value.data;
4743 last = start + h->value.len;
4744
4745 if (ngx_strlcasestrn(start, last, (u_char *) "no-cache", 8 - 1) != NULL
4746 || ngx_strlcasestrn(start, last, (u_char *) "no-store", 8 - 1) != NULL
4747 || ngx_strlcasestrn(start, last, (u_char *) "private", 7 - 1) != NULL)
4748 {
4749 u->cacheable = 0;
4750 return NGX_OK;
4751 }

If Cache-Control from upstream has a value of no-store, no-cache or private, u->cacheable = 0; in ngx_http_upstream_process_cache_control.

In the case of before processing ngx_http_upstream_process_accel_expires, if it sets u->cacheable = 0 for this procedure, it does not cache according to Cache-Control. X-Accel-Expires is only overwriting valid_sec.

OTOH, In the case of after processing ngx_http_upstream_process_accel_expires, ngx_http_upstream_process_cache_control returns NGX_OK earlier than the procedure of Cache-Control: no-xxx or private.
In this case, it does cache, the cache is following x-accel-expires values. It ignores Cache-Control.

As this result 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;
     }