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

[Dmitry Volyntsev]

details:   https://hg.nginx.org/njs/rev/2c1382bab643
branches:  
changeset: 1804:2c1382bab643
user:      Dmitry Volyntsev <xeioex at 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") },