From e008f7ae22834ff1173b7a0067b14c821102018d Mon Sep 17 00:00:00 2001 From: Dmitry Volyntsev Date: Thu, 16 Jun 2022 17:33:49 -0700 Subject: [PATCH] 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. --- src/njs_promise.c | 88 ++++++++++++++++++++++++++++------------ src/test/njs_unit_test.c | 10 +++++ 2 files changed, 71 insertions(+), 27 deletions(-) diff --git a/src/njs_promise.c b/src/njs_promise.c index fca8e6d6..8ed3cc20 100644 --- a/src/njs_promise.c +++ b/src/njs_promise.c @@ -1308,7 +1308,7 @@ njs_promise_perform_all(njs_vm_t *vm, njs_value_t *iterator, 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 *vm, njs_iterator_args_t *args, { 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 *vm, njs_iterator_args_t *args, 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_functions(njs_vm_t *vm, njs_value_t *args, } 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_vm_t *vm, njs_iterator_args_t *args, { 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(njs_vm_t *vm, njs_iterator_args_t *args, 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_functions(njs_vm_t *vm, 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_functions(njs_vm_t *vm, 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_functions(njs_vm_t *vm, 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 *vm, njs_iterator_args_t *args, { 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 *vm, njs_iterator_args_t *args, 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(njs_vm_t *vm, njs_value_t *args, } 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 --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c index 1841de1e..c24a0e8c 100644 --- a/src/test/njs_unit_test.c +++ b/src/test/njs_unit_test.c @@ -21222,6 +21222,16 @@ static njs_unit_test_t njs_externals_test[] = /* 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);" -- 2.47.3