Hi, Vadim
We're pleased to have you share your patch!

src/http/ngx_http_upstream.c
```
4803 if (n == 0) {
4804 u->cacheable = 0;
4805 return NGX_OK;
4806 }
```
We consider that X-Accel-Expires seems not to prioritize
Cache-Control:max-age=0, when it parses after Cache-Control:max-age=0.
Because u->cacheable has been 0 according to this code it cannot apply
the X-Accel-Expires values with
`ngx_http_upstream_validate_cache_headers`.
Could we set u->headers_in.cache_control_c to 0 or only return NGX_OK
to do nothing here instead of u->cacheable set 0?

Best,

Yugo Horie


2022年4月20日(水) 7:20 Vadim Fedorenko via nginx-devel <nginx-devel@nginx.org>:
>
> # HG changeset patch
> # User Vadim Fedorenko <vadim.fedorenko@cdnnow.ru>
> # Date 1650406016 -10800
> # Wed Apr 20 01:06:56 2022 +0300
> # Node ID a75449f4b5c98df0e5a0041eeaa1be5c82d92fea
> # Parent a736a7a613ea6e182ff86fbadcb98bb0f8891c0b
> Upstream: prioritise X-Accel-Expire over Cache-Control and Expire
>
> RFC7234 explicitly says that cache recipient MUST ignore Expires
> header if response includes Cache-Control header with max-age or
> s-maxage directives. Previously Cache-Control was ignored if it
> was after Expires in reply headers.
>
> At the same time documentation states that there is special header
> X-Accel-Expires that controls the behavior of nginx and should be
> prioritized over other cache-specific headers and this patch
> implements this priority.
>
> More informantion could be found in ticket #964.
> ---
> src/http/ngx_http_upstream.c | 62 +++++++++++++++++++++++++-----------
> src/http/ngx_http_upstream.h | 6 ++++
> 2 files changed, 50 insertions(+), 18 deletions(-)
>
> diff -r a736a7a613ea -r a75449f4b5c9 src/http/ngx_http_upstream.c
> --- a/src/http/ngx_http_upstream.c Tue Feb 08 17:35:27 2022 +0300
> +++ b/src/http/ngx_http_upstream.c Wed Apr 20 01:06:56 2022 +0300
> @@ -2350,6 +2350,32 @@
>
>
> static void
> +ngx_http_upstream_validate_cache_headers(ngx_http_request_t *r, ngx_http_upstream_t *u)
> +{
> + ngx_http_upstream_headers_in_t *uh = &u->headers_in;
> + if (uh->x_accel_expires != NULL &&
> + !(u->conf->ignore_headers & NGX_HTTP_UPSTREAM_IGN_XA_EXPIRES)) {
> + u->cacheable = uh->x_accel_expires_c;
> + r->cache->valid_sec = uh->x_accel_expires_n;
> + return;
> + }
> +
> + if (uh->cache_control.elts != NULL &&
> + !(u->conf->ignore_headers & NGX_HTTP_UPSTREAM_IGN_CACHE_CONTROL)) {
> + u->cacheable = uh->cache_control_c;
> + r->cache->valid_sec = uh->cache_control_n;
> + return;
> + }
> +
> + if (uh->expires != NULL &&
> + !(u->conf->ignore_headers & NGX_HTTP_UPSTREAM_IGN_EXPIRES)) {
> + u->cacheable = uh->expires_c;
> + r->cache->valid_sec = uh->expires_n;
> + }
> +}
> +
> +
> +static void
> ngx_http_upstream_process_header(ngx_http_request_t *r, ngx_http_upstream_t *u)
> {
> ssize_t n;
> @@ -2468,6 +2494,11 @@
>
> continue;
> }
> +#if (NGX_HTTP_CACHE)
> + if (u->cacheable) {
> + ngx_http_upstream_validate_cache_headers(r, u);
> + }
> +#endif
>
> break;
> }
> @@ -4735,10 +4766,6 @@
> return NGX_OK;
> }
>
> - if (r->cache->valid_sec != 0 && u->headers_in.x_accel_expires != NULL) {
> - return NGX_OK;
> - }
> -
> start = h->value.data;
> last = start + h->value.len;
>
> @@ -4746,7 +4773,7 @@
> || ngx_strlcasestrn(start, last, (u_char *) "no-store", 8 - 1) != NULL
> || ngx_strlcasestrn(start, last, (u_char *) "private", 7 - 1) != NULL)
> {
> - u->cacheable = 0;
> + u->headers_in.cache_control_c = 0;
> return NGX_OK;
> }
>
> @@ -4771,7 +4798,6 @@
> continue;
> }
>
> - u->cacheable = 0;
> return NGX_OK;
> }
>
> @@ -4780,7 +4806,8 @@
> return NGX_OK;
> }
>
> - r->cache->valid_sec = ngx_time() + n;
> + u->headers_in.cache_control_c = 1;
> + u->headers_in.cache_control_n = ngx_time() + n;
> }
>
> p = ngx_strlcasestrn(start, last, (u_char *) "stale-while-revalidate=",
> @@ -4856,18 +4883,18 @@
> return NGX_OK;
> }
>
> - if (r->cache->valid_sec != 0) {
> + expires = ngx_parse_http_time(h->value.data, h->value.len);
> +
> + if (expires == NGX_ERROR || expires < ngx_time()) {
> return NGX_OK;
> }
>
> - expires = ngx_parse_http_time(h->value.data, h->value.len);
> -
> - if (expires == NGX_ERROR || expires < ngx_time()) {
> - u->cacheable = 0;
> - return NGX_OK;
> - }
> -
> - r->cache->valid_sec = expires;
> + if (u->headers_in.expires_c) {
> + expires = ngx_min(expires, u->headers_in.expires_n);
> + }
> + u->headers_in.expires_n = expires;
> + u->headers_in.expires_c = 1;
> +
> }
> #endif
>
> @@ -4906,14 +4933,12 @@
>
> switch (n) {
> case 0:
> - u->cacheable = 0;
> - /* fall through */
> -
> case NGX_ERROR:
> return NGX_OK;
>
> default:
> - r->cache->valid_sec = ngx_time() + n;
> + u->headers_in.x_accel_expires_c = 1;
> + u->headers_in.x_accel_expires_n = ngx_time() + n;
> return NGX_OK;
> }
> }
> @@ -4924,7 +4949,8 @@
> n = ngx_atoi(p, len);
>
> if (n != NGX_ERROR) {
> - r->cache->valid_sec = n;
> + u->headers_in.x_accel_expires_c = 1;
> + u->headers_in.x_accel_expires_n = ngx_time() + n;
> }
> }
> #endif
> diff -r a736a7a613ea -r a75449f4b5c9 src/http/ngx_http_upstream.h
> --- a/src/http/ngx_http_upstream.h Tue Feb 08 17:35:27 2022 +0300
> +++ b/src/http/ngx_http_upstream.h Wed Apr 20 01:06:56 2022 +0300
> @@ -294,9 +294,15 @@
>
> off_t content_length_n;
> time_t last_modified_time;
> + ngx_int_t cache_control_n;
> + ngx_int_t expires_n;
> + ngx_int_t x_accel_expires_n;
>
> unsigned connection_close:1;
> unsigned chunked:1;
> + unsigned cache_control_c;
> + unsigned expires_c;
> + unsigned x_accel_expires_c;
> } ngx_http_upstream_headers_in_t;
>
>
> _______________________________________________
> nginx-devel mailing list -- nginx-devel@nginx.org
> To unsubscribe send an email to nginx-devel-leave@nginx.org
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org