Welcome! Log In Create A New Profile

Advanced

[njs] HTTP: fixed r.subrequest() error handling.

Anonymous User
June 12, 2024 06:12PM
details: https://hg.nginx.org/njs/rev/3ba1433803ee
branches:
changeset: 2358:3ba1433803ee
user: Dmitry Volyntsev <xeioex@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
Subject Author Views Posted

[njs] HTTP: fixed r.subrequest() error handling.

Anonymous User 314 June 12, 2024 06:12PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 180
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 500 on July 15, 2024
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready