details: https://hg.nginx.org/njs/rev/3ba1433803ee branches: changeset: 2358:3ba1433803ee user: Dmitry Volyntsev <xei...@nginx.com> date: Thu May 30 22:22:48 2024 -0700 description: HTTP: fixed r.subrequest() error handling.
Previously, when at least 2 subrequests were scheduled they both succeed, but the callback for the second threw an exception heap-use-after-free happened: a nested chain of ngx_http_run_posted_requests() calls and terminating request in the inner call left outer calls with already freed request pointer. The issue was introduced in 0.8.1 (4cb8e873e8c6). diffstat: nginx/ngx_http_js_module.c | 31 ++++++++++++++++++++----------- nginx/t/js_subrequests.t | 19 ++++++++++++++++++- 2 files changed, 38 insertions(+), 12 deletions(-) diffs (129 lines): diff -r 95bb22b30a0d -r 3ba1433803ee nginx/ngx_http_js_module.c --- a/nginx/ngx_http_js_module.c Mon Jun 10 23:06:26 2024 -0700 +++ b/nginx/ngx_http_js_module.c Thu May 30 22:22:48 2024 -0700 @@ -276,8 +276,8 @@ static void ngx_http_js_event_finalize(n static ngx_js_ctx_t *ngx_http_js_ctx(njs_vm_t *vm, ngx_http_request_t *r); static void ngx_http_js_periodic_handler(ngx_event_t *ev); -static void ngx_http_js_periodic_write_event_handler(ngx_http_request_t *r); static void ngx_http_js_periodic_shutdown_handler(ngx_event_t *ev); +static void ngx_http_js_periodic_write_handler(ngx_event_t *ev); static void ngx_http_js_periodic_finalize(ngx_http_request_t *r, ngx_int_t rc); static void ngx_http_js_periodic_destroy(ngx_http_request_t *r, ngx_js_periodic_t *periodic); @@ -4220,7 +4220,10 @@ ngx_http_js_periodic_handler(ngx_event_t c->data = r; c->destroyed = 0; c->pool = r->pool; + c->read->log = &periodic->log; c->read->handler = ngx_http_js_periodic_shutdown_handler; + c->write->log = &periodic->log; + c->write->handler = ngx_http_js_periodic_write_handler; periodic->connection = c; periodic->log_ctx.request = r; @@ -4234,7 +4237,6 @@ ngx_http_js_periodic_handler(ngx_event_t r->valid_unparsed_uri = 1; r->health_check = 1; - r->write_event_handler = ngx_http_js_periodic_write_event_handler; rc = ngx_http_js_init_vm(r, ngx_http_js_periodic_session_proto_id); @@ -4263,12 +4265,17 @@ ngx_http_js_periodic_handler(ngx_event_t static void -ngx_http_js_periodic_write_event_handler(ngx_http_request_t *r) +ngx_http_js_periodic_write_handler(ngx_event_t *ev) { - ngx_http_js_ctx_t *ctx; - - ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, - "http js periodic write event handler"); + ngx_connection_t *c; + ngx_http_js_ctx_t *ctx; + ngx_http_request_t *r; + + c = ev->data; + r = c->data; + + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, + "http js periodic write handler"); ctx = ngx_http_get_module_ctx(r, ngx_http_js_module); @@ -4340,6 +4347,10 @@ ngx_http_js_periodic_destroy(ngx_http_re c->fd = (ngx_socket_t) -1; c->pool = NULL; c->destroyed = 1; + + if (c->write->posted) { + ngx_delete_posted_event(c->write); + } } @@ -4451,10 +4462,8 @@ ngx_http_js_event_finalize(ngx_http_requ } if (rc == NGX_OK) { - ngx_http_post_request(r, NULL); - } - - ngx_http_run_posted_requests(r->connection); + ngx_post_event(r->connection->write, &ngx_posted_events); + } } diff -r 95bb22b30a0d -r 3ba1433803ee nginx/t/js_subrequests.t --- a/nginx/t/js_subrequests.t Mon Jun 10 23:06:26 2024 -0700 +++ b/nginx/t/js_subrequests.t Thu May 30 22:22:48 2024 -0700 @@ -180,6 +180,10 @@ http { js_content test.sr_in_sr_callback; } + location /sr_error_in_callback { + js_content test.sr_error_in_callback; + } + location /sr_uri_except { js_content test.sr_uri_except; } @@ -417,6 +421,12 @@ EOF .then(body_fwd_cb); } + function sr_error_in_callback(r) { + r.subrequest("/sub1", () => {}); + r.subrequest("/sub1", () => { throw "Oops!"; }); + r.return(200); + } + function sr_in_sr_callback(r) { r.subrequest('/return', function (reply) { try { @@ -508,7 +518,7 @@ EOF sr_js_in_subrequest, sr_js_in_subrequest_pr, js_sub, sr_in_sr_callback, sr_out_of_order, sr_except_not_a_func, sr_uri_except, sr_except_failed_to_convert_options_arg, - sr_unsafe}; + sr_unsafe, sr_error_in_callback}; EOF @@ -575,6 +585,13 @@ like(http_get('/sr_unsafe'), qr/500/s, ' } +TODO: { +local $TODO = 'not yet' unless has_version('0.8.5'); + +http_get('/sr_error_in_callback'); + +} + $t->stop(); ok(index($t->read_file('error.log'), 'callback is not a function') > 0, _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel