Mercurial > hg > nginx
changeset 9295:c5623963c29e
Upstream: fixed proxy_no_cache when caching errors.
Caching errors, notably intercepted errors and internally generated
502/504 errors, as well as handling of cache revalidation with 304,
did not take into account u->conf->no_cache predicates configured.
As a result, an error might be cached even if configuration explicitly
says not to. Fix is to check u->conf->no_cache in these cases.
To simplify usage in multiple places, checking u->conf->no_cache is now
done in a separate function. As a minor optimization, u->conf->no_cache
is only checked if u->cacheable is set.
As a side effect, this change also fixes caching errors after
proxy_cache_bypass. Also, during cache revalidation u->cacheable is
now tested, so 304 responses which disable caching won't extend
cacheability of stored responses.
Additionally, when caching internally generated 502/504 errors
u->cacheable is now explicitly updated from u->headers_in.no_cache and
u->headers_in.expired, restoring the behaviour before 8041:0784ab86ad08
(1.23.0) when an error happens while reading the response headers.
Reported by Kirill A. Korinsky,
https://freenginx.org/pipermail/nginx/2024-April/000082.html
author | Maxim Dounin <mdounin@mdounin.ru> |
---|---|
date | Tue, 25 Jun 2024 21:44:50 +0300 |
parents | ea0eef2dd12c |
children | af5b47569cb2 |
files | src/http/ngx_http_upstream.c |
diffstat | 1 files changed, 89 insertions(+), 47 deletions(-) [+] |
line wrap: on
line diff
--- a/src/http/ngx_http_upstream.c +++ b/src/http/ngx_http_upstream.c @@ -21,6 +21,8 @@ static ngx_int_t ngx_http_upstream_cache ngx_http_request_t *r, ngx_http_upstream_t *u); static ngx_int_t ngx_http_upstream_cache_check_range(ngx_http_request_t *r, ngx_http_upstream_t *u); +static ngx_int_t ngx_http_upstream_no_cache(ngx_http_request_t *r, + ngx_http_upstream_t *u); static ngx_int_t ngx_http_upstream_cache_status(ngx_http_request_t *r, ngx_http_variable_value_t *v, uintptr_t data); static ngx_int_t ngx_http_upstream_cache_key(ngx_http_request_t *r, @@ -2632,6 +2634,12 @@ ngx_http_upstream_test_next(ngx_http_req updating = r->cache->updating_sec; error = r->cache->error_sec; + if (ngx_http_upstream_no_cache(r, u) != NGX_OK) { + ngx_http_upstream_finalize_request(r, u, + NGX_HTTP_INTERNAL_SERVER_ERROR); + return NGX_OK; + } + rc = u->reinit_request(r); if (rc != NGX_OK) { @@ -2650,30 +2658,33 @@ ngx_http_upstream_test_next(ngx_http_req rc = NGX_HTTP_INTERNAL_SERVER_ERROR; } - if (valid == 0) { - valid = r->cache->valid_sec; - updating = r->cache->updating_sec; - error = r->cache->error_sec; - } - - if (valid == 0) { - valid = ngx_http_file_cache_valid(u->conf->cache_valid, - u->headers_in.status_n); + if (u->cacheable) { + + if (valid == 0) { + valid = r->cache->valid_sec; + updating = r->cache->updating_sec; + error = r->cache->error_sec; + } + + if (valid == 0) { + valid = ngx_http_file_cache_valid(u->conf->cache_valid, + u->headers_in.status_n); + if (valid) { + valid = now + valid; + } + } + if (valid) { - valid = now + valid; + r->cache->valid_sec = valid; + r->cache->updating_sec = updating; + r->cache->error_sec = error; + + r->cache->date = now; + + ngx_http_file_cache_update_header(r); } } - if (valid) { - r->cache->valid_sec = valid; - r->cache->updating_sec = updating; - r->cache->error_sec = error; - - r->cache->date = now; - - ngx_http_file_cache_update_header(r); - } - ngx_http_upstream_finalize_request(r, u, rc); return NGX_OK; } @@ -2745,8 +2756,10 @@ ngx_http_upstream_intercept_errors(ngx_h if (r->cache) { - if (u->headers_in.no_cache || u->headers_in.expired) { - u->cacheable = 0; + if (ngx_http_upstream_no_cache(r, u) != NGX_OK) { + ngx_http_upstream_finalize_request(r, u, + NGX_HTTP_INTERNAL_SERVER_ERROR); + return NGX_OK; } if (u->cacheable) { @@ -3159,29 +3172,8 @@ ngx_http_upstream_send_response(ngx_http r->cache->file.fd = NGX_INVALID_FILE; } - switch (ngx_http_test_predicates(r, u->conf->no_cache)) { - - case NGX_ERROR: + if (ngx_http_upstream_no_cache(r, u) != NGX_OK) { ngx_http_upstream_finalize_request(r, u, NGX_ERROR); - return; - - case NGX_DECLINED: - u->cacheable = 0; - break; - - default: /* NGX_OK */ - - if (u->cache_status == NGX_HTTP_CACHE_BYPASS) { - - /* create cache if previously bypassed */ - - if (ngx_http_file_cache_create(r) != NGX_OK) { - ngx_http_upstream_finalize_request(r, u, NGX_ERROR); - return; - } - } - - break; } if (u->cacheable) { @@ -3372,6 +3364,50 @@ ngx_http_upstream_send_response(ngx_http } +#if (NGX_HTTP_CACHE) + +static ngx_int_t +ngx_http_upstream_no_cache(ngx_http_request_t *r, ngx_http_upstream_t *u) +{ + ngx_int_t rc; + + if (!u->cacheable) { + return NGX_OK; + } + + if (u->headers_in.no_cache || u->headers_in.expired) { + u->cacheable = 0; + return NGX_OK; + } + + rc = ngx_http_test_predicates(r, u->conf->no_cache); + + if (rc == NGX_ERROR) { + return NGX_ERROR; + } + + if (rc == NGX_DECLINED) { + u->cacheable = 0; + return NGX_OK; + } + + /* rc == NGX_OK */ + + if (u->cache_status == NGX_HTTP_CACHE_BYPASS) { + + /* create cache if previously bypassed */ + + if (ngx_http_file_cache_create(r) != NGX_OK) { + return NGX_ERROR; + } + } + + return NGX_OK; +} + +#endif + + static void ngx_http_upstream_upgrade(ngx_http_request_t *r, ngx_http_upstream_t *u) { @@ -4619,9 +4655,15 @@ ngx_http_upstream_finalize_request(ngx_h if (r->cache) { - if (u->cacheable) { - - if (rc == NGX_HTTP_BAD_GATEWAY || rc == NGX_HTTP_GATEWAY_TIME_OUT) { + if (rc == NGX_HTTP_BAD_GATEWAY || rc == NGX_HTTP_GATEWAY_TIME_OUT) { + + if (!u->header_sent) { + if (ngx_http_upstream_no_cache(r, u) != NGX_OK) { + u->cacheable = 0; + } + } + + if (u->cacheable) { time_t valid; valid = ngx_http_file_cache_valid(u->conf->cache_valid, rc);