[nginx] Upstream: fixed proxy_no_cache when caching errors.
Maxim Dounin
mdounin at mdounin.ru
Tue Jun 25 19:03:03 UTC 2024
details: http://freenginx.org/hg/nginx/rev/c5623963c29e
branches:
changeset: 9295:c5623963c29e
user: Maxim Dounin <mdounin at mdounin.ru>
date: Tue Jun 25 21:44:50 2024 +0300
description:
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
diffstat:
src/http/ngx_http_upstream.c | 136 ++++++++++++++++++++++++++++--------------
1 files changed, 89 insertions(+), 47 deletions(-)
diffs (193 lines):
diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c
--- 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);
More information about the nginx-devel
mailing list