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 2b032e8df,
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).
This commit is contained in:
Maxim Dounin authored and Roman Arutyunyan committed 2026-10-01 16:34:43 +04:00
1 parent 2b5c2b605b
commit adf76b2fea
1 file changed
+17 -10
+17 -10
View File
@@ -1136,8 +1136,10 @@ ngx_http_upstream_cache_send(ngx_http_request_t *r, ngx_http_upstream_t *u)
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);
@@ -2642,7 +2644,14 @@ done:
u->peer.notify(&u->peer, u->peer.data, NGX_HTTP_UPSTREAM_NOTIFY_HEADER);
}
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;
}
@@ -3097,7 +3106,9 @@ ngx_http_upstream_process_headers(ngx_http_request_t *r, ngx_http_upstream_t *u)
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;
@@ -3186,18 +3197,14 @@ ngx_http_upstream_process_headers(ngx_http_request_t *r, ngx_http_upstream_t *u)
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;
}
}