Welcome! Log In Create A New Profile

Advanced

[njs] Fixed Array.prototype.concat() when array is changed while iterating.

Dmitry Volyntsev
January 14, 2022 10:02AM
details: https://hg.nginx.org/njs/rev/2c1382bab643
branches:
changeset: 1804:2c1382bab643
user: Dmitry Volyntsev <xeioex@nginx.com>
date: Thu Jan 13 16:20:58 2022 +0000
description:
Fixed Array.prototype.concat() when array is changed while iterating.

Previously, the flat array may be converted to a slow one as a
side-effect of a custom getter invocation for a proto array object.
The function erroneously assumed that the this array remains flat
while iterating.

The fix is to eliminate the micro-optimization which uses direct
pointers.

The problem is similar to the previous commits.

diffstat:

src/njs_array.c | 63 +++++++++--------------------------------------
src/test/njs_unit_test.c | 11 ++++++++
2 files changed, 24 insertions(+), 50 deletions(-)

diffs (128 lines):

diff -r d940c6aaec5d -r 2c1382bab643 src/njs_array.c
--- a/src/njs_array.c Thu Jan 13 15:59:08 2022 +0000
+++ b/src/njs_array.c Thu Jan 13 16:20:58 2022 +0000
@@ -1780,7 +1780,7 @@ njs_array_prototype_concat(njs_vm_t *vm,
int64_t k, len, length;
njs_int_t ret;
njs_uint_t i;
- njs_value_t this, retval, *value, *e;
+ njs_value_t this, retval, *e;
njs_array_t *array, *keys;

ret = njs_value_to_object(vm, &args[0]);
@@ -1819,26 +1819,18 @@ njs_array_prototype_concat(njs_vm_t *vm,
return NJS_ERROR;
}

- if (njs_is_fast_array(&this) && njs_is_fast_array(e)
- && (length + len) <= NJS_ARRAY_LARGE_OBJECT_LENGTH)
- {
+ if (njs_is_fast_array(e)) {
for (k = 0; k < len; k++, length++) {
- value = &njs_array_start(e)[k];
-
- if (njs_slow_path(!njs_is_valid(value))) {
- ret = njs_value_property_i64(vm, e, k, &retval);
- if (njs_slow_path(ret == NJS_ERROR)) {
- return ret;
+ ret = njs_value_property_i64(vm, e, k, &retval);
+ if (njs_slow_path(ret != NJS_OK)) {
+ if (ret == NJS_ERROR) {
+ return NJS_ERROR;
}

- if (ret == NJS_DECLINED) {
- njs_set_invalid(&retval);
- }
-
- value = &retval;
+ njs_set_invalid(&retval);
}

- ret = njs_array_add(vm, array, value);
+ ret = njs_array_add(vm, array, &retval);
if (njs_slow_path(ret != NJS_OK)) {
return NJS_ERROR;
}
@@ -1847,27 +1839,6 @@ njs_array_prototype_concat(njs_vm_t *vm,
continue;
}

- if (njs_fast_object(len)) {
- for (k = 0; k < len; k++, length++) {
- ret = njs_value_property_i64(vm, e, k, &retval);
- if (njs_slow_path(ret == NJS_ERROR)) {
- return ret;
- }
-
- if (ret != NJS_OK) {
- continue;
- }
-
- ret = njs_value_property_i64_set(vm, &this, length,
- &retval);
- if (njs_slow_path(ret == NJS_ERROR)) {
- return ret;
- }
- }
-
- continue;
- }
-
keys = njs_array_indices(vm, e);
if (njs_slow_path(keys == NULL)) {
return NJS_ERROR;
@@ -1879,9 +1850,9 @@ njs_array_prototype_concat(njs_vm_t *vm,
return ret;
}

- idx = njs_string_to_index(&keys->start[k]) + length;
-
if (ret == NJS_OK) {
+ idx = njs_string_to_index(&keys->start[k]) + length;
+
ret = njs_value_property_i64_set(vm, &this, idx, &retval);
if (njs_slow_path(ret == NJS_ERROR)) {
njs_array_destroy(vm, keys);
@@ -1902,17 +1873,9 @@ njs_array_prototype_concat(njs_vm_t *vm,
return NJS_ERROR;
}

- if (njs_is_fast_array(&this)) {
- ret = njs_array_add(vm, array, e);
- if (njs_slow_path(ret != NJS_OK)) {
- return ret;
- }
-
- } else {
- ret = njs_value_property_i64_set(vm, &this, length, e);
- if (njs_slow_path(ret == NJS_ERROR)) {
- return ret;
- }
+ ret = njs_value_property_i64_set(vm, &this, length, e);
+ if (njs_slow_path(ret == NJS_ERROR)) {
+ return ret;
}

length++;
diff -r d940c6aaec5d -r 2c1382bab643 src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c Thu Jan 13 15:59:08 2022 +0000
+++ b/src/test/njs_unit_test.c Thu Jan 13 16:20:58 2022 +0000
@@ -12834,6 +12834,17 @@ static njs_unit_test_t njs_test[] =
"Object.getOwnPropertyDescriptor(o, Symbol.isConcatSpreadable).value"),
njs_str("true") },

+ { njs_str("var a = [1];"
+ "var b = [2, /**/, 4, 5];"
+ "Object.defineProperty(b.__proto__, 1, {"
+ " get: () => {"
+ " b.length = 10**6;"
+ " return 3;"
+ " }"
+ "});"
+ "a.concat(b)"),
+ njs_str("1,2,3,4,5") },
+
{ njs_str("var o = {}, n = 5381 /* NJS_DJB_HASH_INIT */;"
"while(n--) o[Symbol()] = 'test'; o[''];"),
njs_str("undefined") },
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org
Subject Author Views Posted

[njs] Fixed Array.prototype.concat() when array is changed while iterating.

Dmitry Volyntsev 72 January 14, 2022 10:02AM



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

Online Users

Guests: 70
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready