Mercurial > hg > nginx

changeset 9217:6765e5f6d991

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Upstream: fixed X-Accel-Redirect handling from cache files. The X-Accel-Redirect header might appear in cache files if its handling is ignored with the "proxy_ignore_headers" directive. If the cache file is later served with different settings, ngx_http_upstream_process_headers() used to call ngx_http_upstream_finalize_request(NGX_DECLINED), which is not expected to happen before the cleanup handler is installed and resulted in ngx_http_finalize_request(NGX_DONE) (after 5994:5abf5af257a7, nginx 1.7.11), leading to unexpected request counter decrement, "request count is zero" alerts, and segmentation faults. Similarly, errors in ngx_http_upstream_process_headers() resulted in ngx_http_upstream_finalize_request(NGX_HTTP_INTERNAL_SERVER_ERROR) being called. This is also not expected to happen before the cleanup handler is installed, and resulted in ngx_http_finalize_request(NGX_DONE) without proper request finalization. Fix is to avoid calling ngx_http_upstream_finalize_request() from ngx_http_upstream_process_headers(), notably when the cleanup handler is not yet installed. Errors are now simply return NGX_ERROR, so the caller is responsible for proper finalization by calling either ngx_http_finalize_request() or ngx_http_upstream_finalize_request(). And X-Accel-Redirect handling now does not call ngx_http_upstream_finalize_request(NGX_DECLINED) if no cleanup handler is installed. Reported by Jiří Setnička (https://mailman.nginx.org/pipermail/nginx-devel/2024-February/HWLYHOO3DDB3XTFT6X3GRMXIEJ3SJRUA.html).
author Maxim Dounin <mdounin@mdounin.ru>
date Tue, 20 Feb 2024 01:23:43 +0300
parents de20142f5046
children de4208483315
files src/http/ngx_http_upstream.c
diffstat 1 files changed, 17 insertions(+), 10 deletions(-) [+]
line wrap: on
line diff
--- a/src/http/ngx_http_upstream.c
+++ b/src/http/ngx_http_upstream.c
@@ -1087,8 +1087,10 @@ ngx_http_upstream_cache_send(ngx_http_re
 
     if (rc == NGX_OK) {
 
-        if (ngx_http_upstream_process_headers(r, u) != NGX_OK) {
-            return NGX_DONE;
+        rc = ngx_http_upstream_process_headers(r, u);
+
+        if (rc != NGX_OK) {
+            return rc;
         }
 
         return ngx_http_cache_send(r);
@@ -2516,7 +2518,14 @@ ngx_http_upstream_process_header(ngx_htt
         }
     }
 
-    if (ngx_http_upstream_process_headers(r, u) != NGX_OK) {
+    rc = ngx_http_upstream_process_headers(r, u);
+
+    if (rc == NGX_DONE) {
+        return;
+    }
+
+    if (rc == NGX_ERROR) {
+        ngx_http_upstream_finalize_request(r, u, NGX_ERROR);
         return;
     }
 
@@ -2829,7 +2838,9 @@ ngx_http_upstream_process_headers(ngx_ht
     if (u->headers_in.x_accel_redirect
         && !(u->conf->ignore_headers & NGX_HTTP_UPSTREAM_IGN_XA_REDIRECT))
     {
-        ngx_http_upstream_finalize_request(r, u, NGX_DECLINED);
+        if (u->cleanup) {
+            ngx_http_upstream_finalize_request(r, u, NGX_DECLINED);
+        }
 
         part = &u->headers_in.headers.part;
         h = part->elts;
@@ -2918,18 +2929,14 @@ ngx_http_upstream_process_headers(ngx_ht
 
         if (hh) {
             if (hh->copy_handler(r, &h[i], hh->conf) != NGX_OK) {
-                ngx_http_upstream_finalize_request(r, u,
-                                               NGX_HTTP_INTERNAL_SERVER_ERROR);
-                return NGX_DONE;
+                return NGX_ERROR;
             }
 
             continue;
         }
 
         if (ngx_http_upstream_copy_header_line(r, &h[i], 0) != NGX_OK) {
-            ngx_http_upstream_finalize_request(r, u,
-                                               NGX_HTTP_INTERNAL_SERVER_ERROR);
-            return NGX_DONE;
+            return NGX_ERROR;
         }
     }