[njs] Fixed Array.prototype.splice(s, d) when d resizes "this" during eval.

Dmitry Volyntsev xeioex at nginx.com
Fri Nov 11 01:55:09 UTC 2022


details:   https://hg.nginx.org/njs/rev/1d6cea817ef4
branches:  
changeset: 1996:1d6cea817ef4
user:      Dmitry Volyntsev <xeioex at nginx.com>
date:      Thu Nov 10 17:53:36 2022 -0800
description:
Fixed Array.prototype.splice(s,d) when d resizes "this" during eval.

This closes #592 and #595 issues on Github.

diffstat:

 src/njs_array.c          |  116 ++++++++++++++++++----------------------------
 src/test/njs_unit_test.c |   21 ++++++++
 2 files changed, 67 insertions(+), 70 deletions(-)

diffs (201 lines):

diff -r 5964ac864676 -r 1d6cea817ef4 src/njs_array.c
--- a/src/njs_array.c	Thu Nov 10 17:53:35 2022 -0800
+++ b/src/njs_array.c	Thu Nov 10 17:53:36 2022 -0800
@@ -274,18 +274,24 @@ static njs_int_t
 njs_array_copy_within(njs_vm_t *vm, njs_value_t *array, int64_t to_pos,
     int64_t from_pos, int64_t count, njs_bool_t forward)
 {
-    int64_t      i, from, to;
+    int64_t      i, from, to, len;
     njs_int_t    ret;
     njs_array_t  *arr;
     njs_value_t  value;
 
+    njs_assert(to_pos >= 0);
+    njs_assert(from_pos >= 0);
+
     if (njs_fast_path(njs_is_fast_array(array) && count > 0)) {
         arr = njs_array(array);
-
-        memmove(&arr->start[to_pos], &arr->start[from_pos],
-                count * sizeof(njs_value_t));
-
-        return NJS_OK;
+        len = arr->length;
+
+        if (to_pos + count < len && from_pos + count < len) {
+            memmove(&arr->start[to_pos], &arr->start[from_pos],
+                    count * sizeof(njs_value_t));
+
+            return NJS_OK;
+        }
     }
 
     if (!forward) {
@@ -1240,15 +1246,19 @@ njs_array_prototype_splice(njs_vm_t *vm,
         return NJS_ERROR;
     }
 
-    if (njs_fast_path(njs_is_fast_array(this) && deleted->object.fast_array)) {
+    njs_set_array(&del_object, deleted);
+
+    if (njs_fast_path(njs_is_fast_array(this)
+                      && deleted->object.fast_array
+                      && delete <= deleted->length
+                      && start + delete <= njs_array_len(this)))
+    {
         array = njs_array(this);
         for (i = 0, n = start; i < delete; i++, n++) {
-            deleted->start[i] = array->start[n];
+            njs_value_assign(&deleted->start[i], &array->start[n]);
         }
 
     } else {
-        njs_set_array(&del_object, deleted);
-
         for (i = 0, n = start; i < delete; i++, n++) {
             ret = njs_value_property_i64(vm, this, n, &value);
             if (njs_slow_path(ret == NJS_ERROR)) {
@@ -1256,8 +1266,8 @@ njs_array_prototype_splice(njs_vm_t *vm,
             }
 
             if (ret == NJS_OK) {
-                /* TODO:  CreateDataPropertyOrThrow(). */
-                ret = njs_value_property_i64_set(vm, &del_object, i, &value);
+                ret = njs_value_create_data_prop_i64(vm, &del_object, i, &value,
+                                                     0);
                 if (njs_slow_path(ret == NJS_ERROR)) {
                     return ret;
                 }
@@ -1268,76 +1278,42 @@ njs_array_prototype_splice(njs_vm_t *vm,
                 }
             }
         }
-
-        ret = njs_object_length_set(vm, &del_object, delete);
-        if (njs_slow_path(ret != NJS_OK)) {
-            return NJS_ERROR;
-        }
+    }
+
+    ret = njs_object_length_set(vm, &del_object, delete);
+    if (njs_slow_path(ret != NJS_OK)) {
+        return NJS_ERROR;
     }
 
-    if (njs_fast_path(njs_is_fast_array(this))) {
-        array = njs_array(this);
-
-        if (delta != 0) {
-            /*
-             * Relocate the rest of items.
-             * Index of the first item is in "n".
-             */
-            if (delta > 0) {
-                ret = njs_array_expand(vm, array, 0, delta);
-                if (njs_slow_path(ret != NJS_OK)) {
-                    return ret;
-                }
-            }
-
-            ret = njs_array_copy_within(vm, this, start + items, start + delete,
-                                        array->length - (start + delete), 0);
-            if (njs_slow_path(ret != NJS_OK)) {
-                return ret;
-            }
-
-            array->length += delta;
+   if (delta != 0) {
+       ret = njs_array_copy_within(vm, this, start + items, start + delete,
+                                   length - (start + delete), delta < 0);
+        if (njs_slow_path(ret != NJS_OK)) {
+            return ret;
         }
 
-        /* Copy new items. */
-
-        if (items > 0) {
-            memcpy(&array->start[start], &args[3],
-                   items * sizeof(njs_value_t));
-        }
-
-    } else {
-
-       if (delta != 0) {
-           ret = njs_array_copy_within(vm, this, start + items, start + delete,
-                                       length - (start + delete), delta < 0);
-            if (njs_slow_path(ret != NJS_OK)) {
-                return ret;
-            }
-
-            for (i = length - 1; i >= length + delta; i--) {
-                ret = njs_value_property_i64_delete(vm, this, i, NULL);
-                if (njs_slow_path(ret == NJS_ERROR)) {
-                    return NJS_ERROR;
-                }
-            }
-       }
-
-        /* Copy new items. */
-
-        for (i = 3, n = start; items-- > 0; i++, n++) {
-            ret = njs_value_property_i64_set(vm, this, n, &args[i]);
+        for (i = length - 1; i >= length + delta; i--) {
+            ret = njs_value_property_i64_delete(vm, this, i, NULL);
             if (njs_slow_path(ret == NJS_ERROR)) {
                 return NJS_ERROR;
             }
         }
-
-        ret = njs_object_length_set(vm, this, length + delta);
-        if (njs_slow_path(ret != NJS_OK)) {
+   }
+
+    /* Copy new items. */
+
+    for (i = 3, n = start; items-- > 0; i++, n++) {
+        ret = njs_value_property_i64_set(vm, this, n, &args[i]);
+        if (njs_slow_path(ret == NJS_ERROR)) {
             return NJS_ERROR;
         }
     }
 
+    ret = njs_object_length_set(vm, this, length + delta);
+    if (njs_slow_path(ret != NJS_OK)) {
+        return NJS_ERROR;
+    }
+
     njs_set_array(&vm->retval, deleted);
 
     return NJS_OK;
diff -r 5964ac864676 -r 1d6cea817ef4 src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c	Thu Nov 10 17:53:35 2022 -0800
+++ b/src/test/njs_unit_test.c	Thu Nov 10 17:53:36 2022 -0800
@@ -4980,6 +4980,27 @@ static njs_unit_test_t  njs_test[] =
               ".map(v=>v.join(''))"),
       njs_str(",1345,,1,13,13,13") },
 
+    { njs_str("var a = ['x'];"
+              "var d = a.splice(0, { valueOf() {  a.length = 0; return 10; } });"
+              "njs.dump(d)"),
+      njs_str("[<empty>]") },
+
+    { njs_str("var a = ['a', 'b', 'c'];"
+              "var d = a.splice(0, { valueOf() {  a.length = 2; return 3; } });"
+              "njs.dump(d)"),
+      njs_str("['a','b',<empty>]") },
+
+#if NJS_HAVE_LARGE_STACK
+    { njs_str("let arr = [ 'x' ];"
+              "let a = { toString() {"
+               "          new Float64Array(100).set(["
+               "            {toString() {Array.prototype.splice.call(arr, a)}}"
+               "          ])"
+               "        }};"
+               "a.toString()"),
+      njs_str("RangeError: Maximum call stack size exceeded") },
+#endif
+
     { njs_str("var o = { toString: () => {"
               "             for (var i = 0; i < 0x10; i++) {a.push(1)};"
               "             return {};"



More information about the nginx-devel mailing list