nginx: src/http/ngx_http_upstream.c comparison

comparison src/http/ngx_http_upstream.c @ 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	5e7588d2d9cc
children	8cdab3d89c44

comparison

equal deleted inserted replaced

-:ea0eef2dd12c
+:c5623963c29e
 static ngx_int_t ngx_http_upstream_cache_send(ngx_http_request_t *r,
 ngx_http_upstream_t *u);
 static ngx_int_t ngx_http_upstream_cache_background_update(
 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,
 ngx_http_variable_value_t *v, uintptr_t data);
 valid = r->cache->valid_sec;
 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) {
 ngx_http_upstream_finalize_request(r, u, rc);
 return NGX_OK;
 if (rc == NGX_HTTP_UPSTREAM_INVALID_HEADER) {
 rc = NGX_HTTP_INTERNAL_SERVER_ERROR;
 }
-if (valid == 0) {
+if (u->cacheable) {
-valid = r->cache->valid_sec;
-updating = r->cache->updating_sec;
+if (valid == 0) {
-error = r->cache->error_sec;
+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 == 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;
-if (valid) {
+r->cache->date = now;
-r->cache->valid_sec = valid;
-r->cache->updating_sec = updating;
+ngx_http_file_cache_update_header(r);
-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;
 }
 #if (NGX_HTTP_CACHE)
 if (r->cache) {
-if (u->headers_in.no_cache || u->headers_in.expired) {
+if (ngx_http_upstream_no_cache(r, u) != NGX_OK) {
-u->cacheable = 0;
+ngx_http_upstream_finalize_request(r, u,
+NGX_HTTP_INTERNAL_SERVER_ERROR);
+return NGX_OK;
 }
 if (u->cacheable) {
 time_t  valid;
 if (r->cache && r->cache->file.fd != NGX_INVALID_FILE) {
 ngx_pool_run_cleanup_file(r->pool, r->cache->file.fd);
 r->cache->file.fd = NGX_INVALID_FILE;
 }
-switch (ngx_http_test_predicates(r, u->conf->no_cache)) {
+if (ngx_http_upstream_no_cache(r, u) != NGX_OK) {
-case NGX_ERROR:
 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) {
 time_t  now, valid;
 ngx_http_upstream_process_upstream(r, u);
 }
+#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)
 {
 ngx_connection_t          *c;
 ngx_http_core_loc_conf_t  *clcf;
 #if (NGX_HTTP_CACHE)
 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);
 if (valid) {

Mercurial > hg > nginx

comparison src/http/ngx_http_upstream.c @ 9295:c5623963c29e