From: Dmitry Volyntsev Date: Thu, 14 May 2026 02:06:24 +0000 (-0700) Subject: HTTP: fixed r.return() with body in js_access. X-Git-Tag: 0.9.9~5 X-Git-Url: http://git.kaiwu.me/postgresql/log/contrib/postgres_fdw/stylesheets/print.css?a=commitdiff_plain;h=26a79e5307a07a2c4ec92acbd9d978f1a7b991cc;p=njs.git HTTP: fixed r.return() with body in js_access. Previously, when a js_access handler called r.return(status, body), ngx_http_send_response() sent the response but returned NGX_OK. The access handler then returned NGX_OK to the phase engine, which treated the access check as allowed and continued to the content phase. As a result, a denied request could still reach proxy_pass or another content handler after the response had already been sent. Now the access handler finalizes the request if a response was already sent during js_access execution. This keeps r.return() behavior in js_content unchanged, while making r.return(status, body) terminal in js_access. --- diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c index 29333fc1..b91c700a 100644 --- a/nginx/ngx_http_js_module.c +++ b/nginx/ngx_http_js_module.c @@ -174,6 +174,8 @@ typedef struct { static ngx_int_t ngx_http_js_access_handler(ngx_http_request_t *r); +static ngx_int_t ngx_http_js_access_finalize(ngx_http_request_t *r, + ngx_http_js_ctx_t *ctx); static void ngx_http_js_access_write_event_handler(ngx_http_request_t *r); static ngx_int_t ngx_http_js_content_handler(ngx_http_request_t *r); static void ngx_http_js_content_event_handler(ngx_http_request_t *r); @@ -1540,7 +1542,7 @@ ngx_http_js_access_handler(ngx_http_request_t *r) return NGX_HTTP_INTERNAL_SERVER_ERROR; } - return ctx->status; + return ngx_http_js_access_finalize(r, ctx); } ctx->status = NGX_OK; @@ -1594,6 +1596,18 @@ done: return NGX_AGAIN; } + return ngx_http_js_access_finalize(r, ctx); +} + + +static ngx_int_t +ngx_http_js_access_finalize(ngx_http_request_t *r, ngx_http_js_ctx_t *ctx) +{ + if (r->header_sent) { + ngx_http_finalize_request(r, NGX_OK); + return NGX_DONE; + } + return ctx->status; } @@ -1601,6 +1615,7 @@ done: static void ngx_http_js_access_write_event_handler(ngx_http_request_t *r) { + ngx_int_t rc; ngx_http_js_ctx_t *ctx; ctx = ngx_http_get_module_ctx(r, ngx_http_js_module); @@ -1608,10 +1623,17 @@ ngx_http_js_access_write_event_handler(ngx_http_request_t *r) ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "http js access write event handler"); - if (!ngx_js_ctx_pending(ctx)) { - ngx_http_core_run_phases(r); + if (ngx_js_ctx_pending(ctx)) { return; } + + rc = ngx_http_js_access_finalize(r, ctx); + + if (rc == NGX_DONE) { + return; + } + + ngx_http_core_run_phases(r); } @@ -3697,6 +3719,10 @@ ngx_http_js_access_body_finalize(ngx_http_request_t *r, ngx_http_js_ctx_t *ctx, return; } + if (ngx_http_js_access_finalize(r, ctx) == NGX_DONE) { + return; + } + r->write_event_handler = ngx_http_core_run_phases; ngx_http_core_run_phases(r); break; diff --git a/nginx/t/js_access.t b/nginx/t/js_access.t index 29ac026e..bf328f0d 100644 --- a/nginx/t/js_access.t +++ b/nginx/t/js_access.t @@ -89,6 +89,31 @@ http { js_content test.content; } + location /deny_body_proxy { + js_access test.deny_body; + proxy_pass http://127.0.0.1:8083; + } + + location /async_deny_body_proxy { + js_access test.async_deny_body; + proxy_pass http://127.0.0.1:8083; + } + + location /access_return_200 { + js_access test.access_return_200; + js_content test.content; + } + + location /deny_error_page { + error_page 403 /custom_403; + js_access test.deny; + js_content test.content; + } + + location /custom_403 { + return 200 "custom403"; + } + location /async_exception { js_access test.async_exception; js_content test.content; @@ -161,6 +186,15 @@ http { return 200 "backend2"; } } + + server { + listen 127.0.0.1:8083; + access_log %%TESTDIR%%/backend_access.log combined; + + location / { + return 200 "backend_access"; + } + } } EOF @@ -178,6 +212,10 @@ $t->write_file('test.js', <write_file('test.js', < setTimeout(resolve, 5)); + r.return(403, 'denied_async'); + } + async function async_exception(r) { await new Promise(resolve => setTimeout(resolve, 5)); throw new Error("async_access_error"); @@ -257,6 +300,10 @@ $t->write_file('test.js', <write_file('test.js', <write_file('backend_access.log', ''); + $t->write_file_expand('duplicate.conf', bad_conf( http => 'js_import test.js;', location => 'js_access test.noop; js_access test.noop;')); @@ -280,7 +330,7 @@ $t->write_file_expand('duplicate.conf', bad_conf( $t->write_file_expand('no_import.conf', bad_conf( location => 'js_access test.noop;')); -$t->try_run('no js_access')->plan(26); +$t->try_run('no js_access')->plan(33); ############################################################################### @@ -306,6 +356,18 @@ like(http_get('/async_timeout'), qr/var:timeout_ok/, 'async js_access with setTimeout'); like(http_get('/async_deny'), qr/403 Forbidden/, 'async js_access r.return(403) rejects'); +like(http_post('/deny_body_proxy'), qr/403 Forbidden.*denied/s, + 'js_access r.return(403, body) rejects'); +is($t->read_file('backend_access.log'), '', + 'js_access r.return(403, body) skips proxy_pass'); +like(http_post('/async_deny_body_proxy'), qr/403 Forbidden.*denied_async/s, + 'async js_access r.return(403, body) rejects'); +is($t->read_file('backend_access.log'), '', + 'async js_access r.return(403, body) skips proxy_pass'); +like(http_get('/access_return_200'), qr/200 OK.*access_ok/s, + 'js_access r.return(200, body) finalizes request'); +like(http_get('/deny_error_page'), qr/custom403/, + 'js_access r.return(403) preserves error_page'); like(http_get('/async_exception'), qr/500 Internal Server Error/, 'async js_access exception returns 500'); like(http_get('/sr_skip'), qr/var:/, @@ -343,6 +405,9 @@ isnt($rc, 0, 'js_access without js_import fails'); like($out, qr/no imports defined for "js_access" "test\.noop"/, 'js_access without js_import error'); +unlike($t->read_file('error.log'), qr/header already sent/, + 'js_access body return does not double-send headers'); + ############################################################################### sub http_post {