Added a patch to fix njs promise subrequest with error_page redirect
diff --git a/alpine/Makefile.module-njs b/alpine/Makefile.module-njs index 466f67d..2ae0c01 100644 --- a/alpine/Makefile.module-njs +++ b/alpine/Makefile.module-njs
@@ -11,6 +11,7 @@ MODULE_VERSION_PREFIX_njs=$(MODULE_TARGET_PREFIX) MODULE_BUILD_DEPENDS_njs=libedit-dev +MODULE_PATCHES_njs=extra-patch-1568-fix-subrequest-error-page define MODULE_ADD_CONTROL_TAGS_njs replaces="nginx-mod-http-js"
diff --git a/alpine/src/extra-patch-1568-fix-subrequest-error-page b/alpine/src/extra-patch-1568-fix-subrequest-error-page new file mode 100644 index 0000000..a56bdf1 --- /dev/null +++ b/alpine/src/extra-patch-1568-fix-subrequest-error-page
@@ -0,0 +1,151 @@ +# HG changeset patch +# User Dmitry Volyntsev <xeioex@nginx.com> +# Date 1605722951 0 +# Wed Nov 18 18:09:11 2020 +0000 +# Node ID c947a300b96c87c1d77cbb1a1fbc149ea8c6a1d0 +# Parent e97f76121196bda29a450388f82458f575665d88 +HTTP: fixed promise subrequest() with error_page redirect. + +Previously, promise callbacks for a subrequest were stored in a +subrequest context. This is incorrect because a subrequest content may +be destroyed (for example when error_page with redirect is enabled for a +subrequest location). + +The fix is to store callbacks in the parent context. + +The issue was introduced in 6fccbc9f1288 (0.3.8). + +diff -r e97f76121196 -r c947a300b96c nginx/ngx_http_js_module.c +--- a/njs-0.4.4/nginx/ngx_http_js_module.c Tue Nov 17 13:22:34 2020 +0000 ++++ b/njs-0.4.4/nginx/ngx_http_js_module.c Wed Nov 18 18:09:11 2020 +0000 +@@ -44,12 +44,18 @@ typedef struct { + njs_opaque_value_t request; + njs_opaque_value_t request_body; + ngx_str_t redirect_uri; +- njs_opaque_value_t promise_callbacks[2]; ++ ngx_array_t promise_callbacks; + } ngx_http_js_ctx_t; + + + typedef struct { + ngx_http_request_t *request; ++ njs_opaque_value_t callbacks[2]; ++} ngx_http_js_cb_t; ++ ++ ++typedef struct { ++ ngx_http_request_t *request; + njs_vm_event_t vm_event; + void *unused; + ngx_int_t ident; +@@ -2276,19 +2282,52 @@ static njs_int_t + ngx_http_js_promise_trampoline(njs_vm_t *vm, njs_value_t *args, + njs_uint_t nargs, njs_index_t unused) + { ++ ngx_uint_t i; + njs_function_t *callback; ++ ngx_http_js_cb_t *cb, *cbs; + ngx_http_js_ctx_t *ctx; + ngx_http_request_t *r; + + r = njs_vm_external(vm, njs_argument(args, 1)); +- ctx = ngx_http_get_module_ctx(r, ngx_http_js_module); ++ ctx = ngx_http_get_module_ctx(r->parent, ngx_http_js_module); + + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, +- "js subrequest promise trampoline ctx: %p", ctx); +- +- callback = njs_value_function(njs_value_arg(&ctx->promise_callbacks[0])); ++ "js subrequest promise trampoline parent ctx: %p", ctx); ++ ++ if (ctx == NULL) { ++ njs_vm_error(vm, "js subrequest: failed to get the parent context"); ++ return NJS_ERROR; ++ } ++ ++ cbs = ctx->promise_callbacks.elts; ++ ++ if (cbs == NULL) { ++ goto fail; ++ } ++ ++ cb = NULL; ++ ++ for (i = 0; i < ctx->promise_callbacks.nelts; i++) { ++ if (cbs[i].request == r) { ++ cb = &cbs[i]; ++ cb->request = NULL; ++ break; ++ } ++ } ++ ++ if (cb == NULL) { ++ goto fail; ++ } ++ ++ callback = njs_value_function(njs_value_arg(&cb->callbacks[0])); + + return njs_vm_call(vm, callback, njs_argument(args, 1), 1); ++ ++fail: ++ ++ njs_vm_error(vm, "js subrequest: promise callback not found"); ++ ++ return NJS_ERROR; + } + + +@@ -2298,9 +2337,10 @@ ngx_http_js_ext_subrequest(njs_vm_t *vm, + { + ngx_int_t rc, promise; + njs_str_t uri_arg, args_arg, method_name, body_arg; +- ngx_uint_t method, methods_max, has_body, detached; ++ ngx_uint_t i, method, methods_max, has_body, detached; + njs_value_t *value, *arg, *options; + njs_function_t *callback; ++ ngx_http_js_cb_t *cb, *cbs; + ngx_http_js_ctx_t *ctx; + njs_opaque_value_t lvalue; + ngx_http_request_t *r, *sr; +@@ -2507,15 +2547,36 @@ ngx_http_js_ext_subrequest(njs_vm_t *vm, + } + + if (promise) { +- ctx = ngx_pcalloc(sr->pool, sizeof(ngx_http_js_ctx_t)); +- if (ctx == NULL) { +- return NGX_ERROR; ++ cbs = ctx->promise_callbacks.elts; ++ ++ if (cbs == NULL) { ++ if (ngx_array_init(&ctx->promise_callbacks, r->pool, 4, ++ sizeof(ngx_http_js_cb_t)) != NGX_OK) ++ { ++ goto memory_error; ++ } + } + +- ngx_http_set_ctx(sr, ctx, ngx_http_js_module); ++ cb = NULL; ++ ++ for (i = 0; i < ctx->promise_callbacks.nelts; i++) { ++ if (cbs[i].request == NULL) { ++ cb = &cbs[i]; ++ break; ++ } ++ } ++ ++ if (i == ctx->promise_callbacks.nelts) { ++ cb = ngx_array_push(&ctx->promise_callbacks); ++ if (cb == NULL) { ++ goto memory_error; ++ } ++ } ++ ++ cb->request = sr; + + return njs_vm_promise_create(vm, njs_vm_retval(vm), +- njs_value_arg(&ctx->promise_callbacks)); ++ njs_value_arg(&cb->callbacks)); + } + + njs_value_undefined_set(njs_vm_retval(vm));
diff --git a/debian/Makefile.module-njs b/debian/Makefile.module-njs index b477557..492e394 100644 --- a/debian/Makefile.module-njs +++ b/debian/Makefile.module-njs
@@ -5,6 +5,7 @@ MODULE_VERSION_njs= 0.4.4 MODULE_RELEASE_njs= 1 +MODULE_PATCHES_njs= 1568-fix-subrequest-error-page.patch MODULE_SOURCES_njs= njs-$(MODULE_VERSION_njs).tar.gz MODULE_CONFARGS_njs= --add-dynamic-module=$(MODSRC_PREFIX)njs-$(MODULE_VERSION_njs)/nginx
diff --git a/debian/extra/1568-fix-subrequest-error-page.patch b/debian/extra/1568-fix-subrequest-error-page.patch new file mode 100644 index 0000000..b756910 --- /dev/null +++ b/debian/extra/1568-fix-subrequest-error-page.patch
@@ -0,0 +1,151 @@ +# HG changeset patch +# User Dmitry Volyntsev <xeioex@nginx.com> +# Date 1605722951 0 +# Wed Nov 18 18:09:11 2020 +0000 +# Node ID c947a300b96c87c1d77cbb1a1fbc149ea8c6a1d0 +# Parent e97f76121196bda29a450388f82458f575665d88 +HTTP: fixed promise subrequest() with error_page redirect. + +Previously, promise callbacks for a subrequest were stored in a +subrequest context. This is incorrect because a subrequest content may +be destroyed (for example when error_page with redirect is enabled for a +subrequest location). + +The fix is to store callbacks in the parent context. + +The issue was introduced in 6fccbc9f1288 (0.3.8). + +diff -r e97f76121196 -r c947a300b96c nginx/ngx_http_js_module.c +--- c/njs-0.4.4/nginx/ngx_http_js_module.c Tue Nov 17 13:22:34 2020 +0000 ++++ d/njs-0.4.4/nginx/ngx_http_js_module.c Wed Nov 18 18:09:11 2020 +0000 +@@ -44,12 +44,18 @@ typedef struct { + njs_opaque_value_t request; + njs_opaque_value_t request_body; + ngx_str_t redirect_uri; +- njs_opaque_value_t promise_callbacks[2]; ++ ngx_array_t promise_callbacks; + } ngx_http_js_ctx_t; + + + typedef struct { + ngx_http_request_t *request; ++ njs_opaque_value_t callbacks[2]; ++} ngx_http_js_cb_t; ++ ++ ++typedef struct { ++ ngx_http_request_t *request; + njs_vm_event_t vm_event; + void *unused; + ngx_int_t ident; +@@ -2276,19 +2282,52 @@ static njs_int_t + ngx_http_js_promise_trampoline(njs_vm_t *vm, njs_value_t *args, + njs_uint_t nargs, njs_index_t unused) + { ++ ngx_uint_t i; + njs_function_t *callback; ++ ngx_http_js_cb_t *cb, *cbs; + ngx_http_js_ctx_t *ctx; + ngx_http_request_t *r; + + r = njs_vm_external(vm, njs_argument(args, 1)); +- ctx = ngx_http_get_module_ctx(r, ngx_http_js_module); ++ ctx = ngx_http_get_module_ctx(r->parent, ngx_http_js_module); + + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, +- "js subrequest promise trampoline ctx: %p", ctx); +- +- callback = njs_value_function(njs_value_arg(&ctx->promise_callbacks[0])); ++ "js subrequest promise trampoline parent ctx: %p", ctx); ++ ++ if (ctx == NULL) { ++ njs_vm_error(vm, "js subrequest: failed to get the parent context"); ++ return NJS_ERROR; ++ } ++ ++ cbs = ctx->promise_callbacks.elts; ++ ++ if (cbs == NULL) { ++ goto fail; ++ } ++ ++ cb = NULL; ++ ++ for (i = 0; i < ctx->promise_callbacks.nelts; i++) { ++ if (cbs[i].request == r) { ++ cb = &cbs[i]; ++ cb->request = NULL; ++ break; ++ } ++ } ++ ++ if (cb == NULL) { ++ goto fail; ++ } ++ ++ callback = njs_value_function(njs_value_arg(&cb->callbacks[0])); + + return njs_vm_call(vm, callback, njs_argument(args, 1), 1); ++ ++fail: ++ ++ njs_vm_error(vm, "js subrequest: promise callback not found"); ++ ++ return NJS_ERROR; + } + + +@@ -2298,9 +2337,10 @@ ngx_http_js_ext_subrequest(njs_vm_t *vm, + { + ngx_int_t rc, promise; + njs_str_t uri_arg, args_arg, method_name, body_arg; +- ngx_uint_t method, methods_max, has_body, detached; ++ ngx_uint_t i, method, methods_max, has_body, detached; + njs_value_t *value, *arg, *options; + njs_function_t *callback; ++ ngx_http_js_cb_t *cb, *cbs; + ngx_http_js_ctx_t *ctx; + njs_opaque_value_t lvalue; + ngx_http_request_t *r, *sr; +@@ -2507,15 +2547,36 @@ ngx_http_js_ext_subrequest(njs_vm_t *vm, + } + + if (promise) { +- ctx = ngx_pcalloc(sr->pool, sizeof(ngx_http_js_ctx_t)); +- if (ctx == NULL) { +- return NGX_ERROR; ++ cbs = ctx->promise_callbacks.elts; ++ ++ if (cbs == NULL) { ++ if (ngx_array_init(&ctx->promise_callbacks, r->pool, 4, ++ sizeof(ngx_http_js_cb_t)) != NGX_OK) ++ { ++ goto memory_error; ++ } + } + +- ngx_http_set_ctx(sr, ctx, ngx_http_js_module); ++ cb = NULL; ++ ++ for (i = 0; i < ctx->promise_callbacks.nelts; i++) { ++ if (cbs[i].request == NULL) { ++ cb = &cbs[i]; ++ break; ++ } ++ } ++ ++ if (i == ctx->promise_callbacks.nelts) { ++ cb = ngx_array_push(&ctx->promise_callbacks); ++ if (cb == NULL) { ++ goto memory_error; ++ } ++ } ++ ++ cb->request = sr; + + return njs_vm_promise_create(vm, njs_vm_retval(vm), +- njs_value_arg(&ctx->promise_callbacks)); ++ njs_value_arg(&cb->callbacks)); + } + + njs_value_undefined_set(njs_vm_retval(vm));
diff --git a/rpm/SOURCES/1568-fix-subrequest-error-page.patch b/rpm/SOURCES/1568-fix-subrequest-error-page.patch new file mode 100644 index 0000000..a56bdf1 --- /dev/null +++ b/rpm/SOURCES/1568-fix-subrequest-error-page.patch
@@ -0,0 +1,151 @@ +# HG changeset patch +# User Dmitry Volyntsev <xeioex@nginx.com> +# Date 1605722951 0 +# Wed Nov 18 18:09:11 2020 +0000 +# Node ID c947a300b96c87c1d77cbb1a1fbc149ea8c6a1d0 +# Parent e97f76121196bda29a450388f82458f575665d88 +HTTP: fixed promise subrequest() with error_page redirect. + +Previously, promise callbacks for a subrequest were stored in a +subrequest context. This is incorrect because a subrequest content may +be destroyed (for example when error_page with redirect is enabled for a +subrequest location). + +The fix is to store callbacks in the parent context. + +The issue was introduced in 6fccbc9f1288 (0.3.8). + +diff -r e97f76121196 -r c947a300b96c nginx/ngx_http_js_module.c +--- a/njs-0.4.4/nginx/ngx_http_js_module.c Tue Nov 17 13:22:34 2020 +0000 ++++ b/njs-0.4.4/nginx/ngx_http_js_module.c Wed Nov 18 18:09:11 2020 +0000 +@@ -44,12 +44,18 @@ typedef struct { + njs_opaque_value_t request; + njs_opaque_value_t request_body; + ngx_str_t redirect_uri; +- njs_opaque_value_t promise_callbacks[2]; ++ ngx_array_t promise_callbacks; + } ngx_http_js_ctx_t; + + + typedef struct { + ngx_http_request_t *request; ++ njs_opaque_value_t callbacks[2]; ++} ngx_http_js_cb_t; ++ ++ ++typedef struct { ++ ngx_http_request_t *request; + njs_vm_event_t vm_event; + void *unused; + ngx_int_t ident; +@@ -2276,19 +2282,52 @@ static njs_int_t + ngx_http_js_promise_trampoline(njs_vm_t *vm, njs_value_t *args, + njs_uint_t nargs, njs_index_t unused) + { ++ ngx_uint_t i; + njs_function_t *callback; ++ ngx_http_js_cb_t *cb, *cbs; + ngx_http_js_ctx_t *ctx; + ngx_http_request_t *r; + + r = njs_vm_external(vm, njs_argument(args, 1)); +- ctx = ngx_http_get_module_ctx(r, ngx_http_js_module); ++ ctx = ngx_http_get_module_ctx(r->parent, ngx_http_js_module); + + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, +- "js subrequest promise trampoline ctx: %p", ctx); +- +- callback = njs_value_function(njs_value_arg(&ctx->promise_callbacks[0])); ++ "js subrequest promise trampoline parent ctx: %p", ctx); ++ ++ if (ctx == NULL) { ++ njs_vm_error(vm, "js subrequest: failed to get the parent context"); ++ return NJS_ERROR; ++ } ++ ++ cbs = ctx->promise_callbacks.elts; ++ ++ if (cbs == NULL) { ++ goto fail; ++ } ++ ++ cb = NULL; ++ ++ for (i = 0; i < ctx->promise_callbacks.nelts; i++) { ++ if (cbs[i].request == r) { ++ cb = &cbs[i]; ++ cb->request = NULL; ++ break; ++ } ++ } ++ ++ if (cb == NULL) { ++ goto fail; ++ } ++ ++ callback = njs_value_function(njs_value_arg(&cb->callbacks[0])); + + return njs_vm_call(vm, callback, njs_argument(args, 1), 1); ++ ++fail: ++ ++ njs_vm_error(vm, "js subrequest: promise callback not found"); ++ ++ return NJS_ERROR; + } + + +@@ -2298,9 +2337,10 @@ ngx_http_js_ext_subrequest(njs_vm_t *vm, + { + ngx_int_t rc, promise; + njs_str_t uri_arg, args_arg, method_name, body_arg; +- ngx_uint_t method, methods_max, has_body, detached; ++ ngx_uint_t i, method, methods_max, has_body, detached; + njs_value_t *value, *arg, *options; + njs_function_t *callback; ++ ngx_http_js_cb_t *cb, *cbs; + ngx_http_js_ctx_t *ctx; + njs_opaque_value_t lvalue; + ngx_http_request_t *r, *sr; +@@ -2507,15 +2547,36 @@ ngx_http_js_ext_subrequest(njs_vm_t *vm, + } + + if (promise) { +- ctx = ngx_pcalloc(sr->pool, sizeof(ngx_http_js_ctx_t)); +- if (ctx == NULL) { +- return NGX_ERROR; ++ cbs = ctx->promise_callbacks.elts; ++ ++ if (cbs == NULL) { ++ if (ngx_array_init(&ctx->promise_callbacks, r->pool, 4, ++ sizeof(ngx_http_js_cb_t)) != NGX_OK) ++ { ++ goto memory_error; ++ } + } + +- ngx_http_set_ctx(sr, ctx, ngx_http_js_module); ++ cb = NULL; ++ ++ for (i = 0; i < ctx->promise_callbacks.nelts; i++) { ++ if (cbs[i].request == NULL) { ++ cb = &cbs[i]; ++ break; ++ } ++ } ++ ++ if (i == ctx->promise_callbacks.nelts) { ++ cb = ngx_array_push(&ctx->promise_callbacks); ++ if (cb == NULL) { ++ goto memory_error; ++ } ++ } ++ ++ cb->request = sr; + + return njs_vm_promise_create(vm, njs_vm_retval(vm), +- njs_value_arg(&ctx->promise_callbacks)); ++ njs_value_arg(&cb->callbacks)); + } + + njs_value_undefined_set(njs_vm_retval(vm));
diff --git a/rpm/SPECS/Makefile.module-njs b/rpm/SPECS/Makefile.module-njs index d80c047..47731ec 100644 --- a/rpm/SPECS/Makefile.module-njs +++ b/rpm/SPECS/Makefile.module-njs
@@ -7,6 +7,7 @@ MODULE_VERSION_PREFIX_njs=$(MODULE_TARGET_PREFIX) +MODULE_PATCHES_njs=1568-fix-subrequest-error-page.patch MODULE_SOURCES_njs= njs-$(MODULE_VERSION_njs).tar.gz MODULE_CONFARGS_njs= --add-dynamic-module=njs-$(MODULE_VERSION_njs)/nginx