[PATCH v2] Upstream: prioritise X-Accel-Expire over Cache-Control and Expire

Vadim Fedorenko vadim.fedorenko at cdnnow.ru
Wed Apr 20 09:18:24 UTC 2022


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 at nginx.org>:
>>
>> # HG changeset patch
>> # User Vadim Fedorenko <vadim.fedorenko at 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 at nginx.org
>> To unsubscribe send an email to nginx-devel-leave at nginx.org



More information about the nginx-devel mailing list