Hi Yugo, Looks like you are right, I will post v3 with updates soon.
Thanks, Vadim On 20.04.2022 05:22, Yugo Horie wrote:
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.fedore...@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-le...@nginx.org
_______________________________________________ nginx-devel mailing list -- nginx-devel@nginx.org To unsubscribe send an email to nginx-devel-le...@nginx.org
