# HG changeset patch # User Maxim Dounin # Date 1719341090 -10800 # Node ID c5623963c29e2c8a79920b259d6884e024414f9d # Parent ea0eef2dd12c2d41349d63c532e942cf95fc4d7b 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 diff -r ea0eef2dd12c -r c5623963c29e src/http/ngx_http_upstream.c --- a/src/http/ngx_http_upstream.c Sun Jun 16 04:17:27 2024 +0300 +++ b/src/http/ngx_http_upstream.c Tue Jun 25 21:44:50 2024 +0300 @@ -21,6 +21,8 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ } +#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 @@ 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);