Mercurial > hg > nginx

changeset 9295:c5623963c29e

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
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);