From 607de2bd454b6f6d6ed12601aeca9f26b6f35ee0 Mon Sep 17 00:00:00 2001 From: Dmitry Volyntsev Date: Thu, 10 Nov 2022 17:53:36 -0800 Subject: [PATCH] Fixed Array.prototype.splice(s,d) when d resizes "this" during eval. This closes #592 and #595 issues on Github. --- src/njs_array.c | 108 +++++++++++++++------------------------ src/test/njs_unit_test.c | 21 ++++++++ 2 files changed, 63 insertions(+), 66 deletions(-) diff --git a/src/njs_array.c b/src/njs_array.c index 4f6bbd6e..ec21e60c 100644 --- a/src/njs_array.c +++ b/src/njs_array.c @@ -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); + len = arr->length; - memmove(&arr->start[to_pos], &arr->start[from_pos], - count * sizeof(njs_value_t)); + 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; + return NJS_OK; + } } if (!forward) { @@ -1240,15 +1246,19 @@ njs_array_prototype_splice(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs, 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, njs_value_t *args, njs_uint_t nargs, } 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, njs_value_t *args, njs_uint_t nargs, } } } - - 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; - } - - /* Copy new items. */ + ret = njs_object_length_set(vm, &del_object, delete); + if (njs_slow_path(ret != NJS_OK)) { + return NJS_ERROR; + } - if (items > 0) { - memcpy(&array->start[start], &args[3], - items * sizeof(njs_value_t)); + 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; } - } 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 --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c index 56849fd5..f875da5b 100644 --- a/src/test/njs_unit_test.c +++ b/src/test/njs_unit_test.c @@ -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("[]") }, + + { 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',]") }, + +#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 {};" -- 2.47.3