Fixed Array.prototype.slice() 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 (9578cc729205) commit.

This closes #445 issue on Github.

diff --git a/src/njs_array.c b/src/njs_array.c
index 54b7fe04656538a31f6e796df695c367e01ca980..46ca9ad04cb731fd231fc7a1063339023ca5d52c 100644 (file)
--- a/src/njs_array.c
+++ b/src/njs_array.c
@@ -729,7 +729,7 @@ njs_array_prototype_slice_copy(njs_vm_t *vm, njs_value_t *this,
     uint32_t           n;
     njs_int_t          ret;
     njs_array_t        *array, *keys;
-    njs_value_t        *value, retval, self;
+    njs_value_t        *value, *last, retval, self;
     const u_char       *src, *end;
     njs_slice_prop_t   string_slice;
     njs_string_prop_t  string;
@@ -748,34 +748,7 @@ njs_array_prototype_slice_copy(njs_vm_t *vm, njs_value_t *this,
     n = 0;
 
     if (njs_fast_path(array->object.fast_array)) {
-        if (njs_fast_path(njs_is_fast_array(this))) {
-            value = njs_array_start(this);
-
-            do {
-                if (njs_fast_path(njs_is_valid(&value[start]))) {
-                    array->start[n++] = value[start++];
-
-                } else {
-
-                    /* src value may be in Array.prototype object. */
-
-                    ret = njs_value_property_i64(vm, this, start++,
-                                                 &array->start[n]);
-                    if (njs_slow_path(ret == NJS_ERROR)) {
-                        return NJS_ERROR;
-                    }
-
-                    if (ret != NJS_OK) {
-                        njs_set_invalid(&array->start[n]);
-                    }
-
-                    n++;
-                }
-
-                length--;
-            } while (length != 0);
-
-        } else if (njs_is_string(this) || njs_is_object_string(this)) {
+        if (njs_is_string(this) || njs_is_object_string(this)) {
 
             if (njs_is_object_string(this)) {
                 this = njs_object_value(this);
@@ -816,16 +789,18 @@ njs_array_prototype_slice_copy(njs_vm_t *vm, njs_value_t *this,
 
         } else if (njs_is_object(this)) {
 
-            do {
-                value = &array->start[n++];
-                ret = njs_value_property_i64(vm, this, start++, value);
+            last = &array->start[length];
+
+            for (value = array->start; value < last; value++, start++) {
+                ret = njs_value_property_i64(vm, this, start, value);
+                if (njs_slow_path(ret != NJS_OK)) {
+                    if (ret == NJS_ERROR) {
+                        return NJS_ERROR;
+                    }
 
-                if (ret != NJS_OK) {
                     njs_set_invalid(value);
                 }
-
-                length--;
-            } while (length != 0);
+            }
 
         } else {
 

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index ae932f0c9e3c0eafcceb5b438e28392c1eb0f2a4..c1a5f8413311f2fb42b36f6e5c9b78e99c007f9a 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -4525,6 +4525,16 @@ static njs_unit_test_t  njs_test[] =
                  "Array.prototype.slice.call(1, 0, 2)"),
       njs_str(",") },
 
+    { njs_str("var a = [1, /**/, 3, 4];"
+              "Object.defineProperty(a.__proto__, 1, {"
+              "    get: () => {"
+              "        a.length = 10**6;"
+              "        return 2;"
+              "    }"
+              "});"
+              "a.slice(1)"),
+      njs_str("2,3,4") },
+
     { njs_str("Array.prototype.pop()"),
       njs_str("undefined") },