[njs] Fixed working with array-like object in Promise.all() and friends.

Dmitry Volyntsev xeioex at nginx.com
Fri Jun 17 00:36:43 UTC 2022


details:   https://hg.nginx.org/njs/rev/42a2406590c0
branches:  
changeset: 1891:42a2406590c0
user:      Dmitry Volyntsev <xeioex at nginx.com>
date:      Thu Jun 16 17:33:49 2022 -0700
description:
Fixed working with array-like object in Promise.all() and friends.

Prevously, the code while iterating over an array-like object did not
take into account objects with absent elements.  As a result, the
resulting array object was returning with elements containing garbage
values.

The fix is to allocate and fill the resulting array object on the fly.

This closes #538 issue on Github.

diffstat:

 src/njs_promise.c        |  88 +++++++++++++++++++++++++++++++++--------------
 src/test/njs_unit_test.c |  10 +++++
 2 files changed, 71 insertions(+), 27 deletions(-)

diffs (235 lines):

diff -r be631766cc1f -r 42a2406590c0 src/njs_promise.c
--- a/src/njs_promise.c	Wed Jun 15 17:10:39 2022 -0700
+++ b/src/njs_promise.c	Thu Jun 16 17:33:49 2022 -0700
@@ -1308,7 +1308,7 @@ njs_promise_perform_all(njs_vm_t *vm, nj
         return ret;
     }
 
-    pargs->args.data = njs_array_alloc(vm, 1, length, 0);
+    pargs->args.data = njs_array_alloc(vm, 1, 0, NJS_ARRAY_SPARE);
     if (njs_slow_path(pargs->args.data == NULL)) {
         return NJS_ERROR;
     }
@@ -1363,7 +1363,7 @@ njs_promise_perform_all_handler(njs_vm_t
 {
     njs_int_t                    ret;
     njs_array_t                  *array;
-    njs_value_t                  arguments[2], next;
+    njs_value_t                  arguments[2], next, arr_value;
     njs_function_t               *on_fulfilled;
     njs_promise_capability_t     *capability;
     njs_promise_all_context_t    *context;
@@ -1378,7 +1378,13 @@ njs_promise_perform_all_handler(njs_vm_t
     capability = pargs->capability;
 
     array = args->data;
-    njs_set_undefined(&array->start[index]);
+    njs_set_array(&arr_value, array);
+
+    ret = njs_value_property_i64_set(vm, &arr_value, index,
+                                     njs_value_arg(&njs_value_undefined));
+    if (njs_slow_path(ret == NJS_ERROR)) {
+        return ret;
+    }
 
     ret = njs_function_call(vm, pargs->function, pargs->constructor, value,
                             1, &next);
@@ -1421,7 +1427,8 @@ static njs_int_t
 njs_promise_all_resolve_element_functions(njs_vm_t *vm, njs_value_t *args,
     njs_uint_t nargs, njs_index_t unused)
 {
-    njs_value_t                arguments;
+    njs_int_t                  ret;
+    njs_value_t                arr_value;
     njs_promise_all_context_t  *context;
 
     context = vm->top_frame->function->context;
@@ -1432,16 +1439,21 @@ njs_promise_all_resolve_element_function
     }
 
     context->already_called = 1;
-    context->values->start[context->index] = *njs_arg(args, nargs, 1);
+
+    njs_set_array(&arr_value, context->values);
+
+    ret = njs_value_property_i64_set(vm, &arr_value, context->index,
+                                     njs_arg(args, nargs, 1));
+    if (njs_slow_path(ret == NJS_ERROR)) {
+        return ret;
+    }
 
     if (--(*context->remaining_elements) == 0) {
         njs_mp_free(vm->mem_pool, context->remaining_elements);
 
-        njs_set_array(&arguments, context->values);
-
         return njs_function_call(vm,
                                  njs_function(&context->capability->resolve),
-                                 &njs_value_undefined, &arguments, 1,
+                                 &njs_value_undefined, &arr_value, 1,
                                  &vm->retval);
     }
 
@@ -1457,7 +1469,7 @@ njs_promise_perform_all_settled_handler(
 {
     njs_int_t                    ret;
     njs_array_t                  *array;
-    njs_value_t                  arguments[2], next;
+    njs_value_t                  arguments[2], next, arr_value;
     njs_function_t               *on_fulfilled, *on_rejected;
     njs_promise_capability_t     *capability;
     njs_promise_all_context_t    *context;
@@ -1472,7 +1484,13 @@ njs_promise_perform_all_settled_handler(
     capability = pargs->capability;
 
     array = args->data;
-    njs_set_undefined(&array->start[index]);
+    njs_set_array(&arr_value, array);
+
+    ret = njs_value_property_i64_set(vm, &arr_value, index,
+                                     njs_value_arg(&njs_value_undefined));
+    if (njs_slow_path(ret == NJS_ERROR)) {
+        return ret;
+    }
 
     ret = njs_function_call(vm, pargs->function, pargs->constructor, value,
                             1, &next);
@@ -1527,7 +1545,7 @@ njs_promise_all_settled_element_function
     njs_value_t *args, njs_uint_t nargs, njs_index_t rejected)
 {
     njs_int_t                  ret;
-    njs_value_t                arguments, *value;
+    njs_value_t                obj_value, arr_value;
     njs_object_t               *obj;
     const njs_value_t          *status, *set;
     njs_promise_all_context_t  *context;
@@ -1552,9 +1570,7 @@ njs_promise_all_settled_element_function
         return NJS_ERROR;
     }
 
-    value = &context->values->start[context->index];
-
-    njs_set_object(value, obj);
+    njs_set_object(&obj_value, obj);
 
     if (rejected) {
         status = &string_rejected;
@@ -1565,26 +1581,32 @@ njs_promise_all_settled_element_function
         set = &string_value;
     }
 
-    ret = njs_value_property_set(vm, value, njs_value_arg(&string_status),
+    ret = njs_value_property_set(vm, &obj_value, njs_value_arg(&string_status),
                                  njs_value_arg(status));
     if (njs_slow_path(ret == NJS_ERROR)) {
         return ret;
     }
 
-    ret = njs_value_property_set(vm, value, njs_value_arg(set),
+    ret = njs_value_property_set(vm, &obj_value, njs_value_arg(set),
                                  njs_arg(args, nargs, 1));
     if (njs_slow_path(ret == NJS_ERROR)) {
         return ret;
     }
 
+    njs_set_array(&arr_value, context->values);
+
+    ret = njs_value_property_i64_set(vm, &arr_value, context->index,
+                                     &obj_value);
+    if (njs_slow_path(ret == NJS_ERROR)) {
+        return ret;
+    }
+
     if (--(*context->remaining_elements) == 0) {
         njs_mp_free(vm->mem_pool, context->remaining_elements);
 
-        njs_set_array(&arguments, context->values);
-
         return njs_function_call(vm,
                                  njs_function(&context->capability->resolve),
-                                 &njs_value_undefined, &arguments, 1,
+                                 &njs_value_undefined, &arr_value, 1,
                                  &vm->retval);
     }
 
@@ -1600,7 +1622,7 @@ njs_promise_perform_any_handler(njs_vm_t
 {
     njs_int_t                    ret;
     njs_array_t                  *array;
-    njs_value_t                  arguments[2], next;
+    njs_value_t                  arguments[2], next, arr_value;
     njs_function_t               *on_rejected;
     njs_promise_capability_t     *capability;
     njs_promise_all_context_t    *context;
@@ -1614,8 +1636,14 @@ njs_promise_perform_any_handler(njs_vm_t
 
     capability = pargs->capability;
 
-    array = pargs->args.data;
-    njs_set_undefined(&array->start[index]);
+    array = args->data;
+    njs_set_array(&arr_value, array);
+
+    ret = njs_value_property_i64_set(vm, &arr_value, index,
+                                     njs_value_arg(&njs_value_undefined));
+    if (njs_slow_path(ret == NJS_ERROR)) {
+        return ret;
+    }
 
     ret = njs_function_call(vm, pargs->function, pargs->constructor, value, 1,
                             &next);
@@ -1658,7 +1686,8 @@ static njs_int_t
 njs_promise_any_reject_element_functions(njs_vm_t *vm, njs_value_t *args,
     njs_uint_t nargs, njs_index_t unused)
 {
-    njs_value_t                argument;
+    njs_int_t                  ret;
+    njs_value_t                argument, arr_value;
     njs_object_t               *error;
     njs_promise_all_context_t  *context;
 
@@ -1670,15 +1699,20 @@ njs_promise_any_reject_element_functions
     }
 
     context->already_called = 1;
-    context->values->start[context->index] = *njs_arg(args, nargs, 1);
+
+    njs_set_array(&arr_value, context->values);
+
+    ret = njs_value_property_i64_set(vm, &arr_value, context->index,
+                                     njs_arg(args, nargs, 1));
+    if (njs_slow_path(ret == NJS_ERROR)) {
+        return ret;
+    }
 
     if (--(*context->remaining_elements) == 0) {
         njs_mp_free(vm->mem_pool, context->remaining_elements);
 
-        njs_set_array(&argument, context->values);
-
         error = njs_error_alloc(vm, NJS_OBJ_TYPE_AGGREGATE_ERROR,
-                                NULL, &string_any_rejected, &argument);
+                                NULL, &string_any_rejected, &arr_value);
         if (njs_slow_path(error == NULL)) {
             return NJS_ERROR;
         }
diff -r be631766cc1f -r 42a2406590c0 src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c	Wed Jun 15 17:10:39 2022 -0700
+++ b/src/test/njs_unit_test.c	Thu Jun 16 17:33:49 2022 -0700
@@ -21222,6 +21222,16 @@ static njs_unit_test_t  njs_externals_te
               /* TODO: RejectAbrupt() exception should not percolate */
       njs_str("TypeError: resolve is not callable") },
 
+    { njs_str("Promise.all({length: 1025}) "
+              ".then(v => $r.retval(v[0]))"),
+              /* TODO: TypeError: object is not iterable */
+      njs_str("undefined") },
+
+    { njs_str("Promise.allSettled({length: 1025}) "
+              ".then(v => $r.retval(v[0]))"),
+              /* TODO: TypeError: object is not iterable */
+      njs_str("undefined") },
+
     { njs_str("var r = [1].map(v => {"
               "    function C(a) {"
               "        a(a, parseInt);"



More information about the nginx-devel mailing list