From 6d624bbbef5c68ee9c28a9976bc8dd99b2fc11cf Mon Sep 17 00:00:00 2001 From: Dmitry Volyntsev Date: Wed, 22 May 2024 23:08:15 -0700 Subject: [PATCH] Fixed retval handling after an exception. Previously, some functions set a retval too early. If this happened before an exception a partially created object in inconsistent state may be visible outside the affected functions. The following functions were fixed: Object.prototype.valueOf() Array.prototype.toSpliced() Array.prototype.toReversed() Array.prototype.toSorted() This fixes #713 issue on Github. --- src/njs_array.c | 39 ++++++++++++++++++++++----------------- src/njs_object.c | 12 +++++++++--- src/test/njs_unit_test.c | 12 ++++++++++++ 3 files changed, 43 insertions(+), 20 deletions(-) diff --git a/src/njs_array.c b/src/njs_array.c index 5a04cde3..15a6b6d3 100644 --- a/src/njs_array.c +++ b/src/njs_array.c @@ -591,14 +591,14 @@ njs_array_of(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs, return NJS_ERROR; } - njs_set_array(retval, array); - if (array->object.fast_array) { for (i = 0; i < length; i++) { array->start[i] = args[i + 1]; } } + njs_set_array(retval, array); + return NJS_OK; } @@ -1388,7 +1388,7 @@ njs_array_prototype_to_spliced(njs_vm_t *vm, njs_value_t *args, { int64_t i, n, r, start, length, to_insert, to_skip, new_length; njs_int_t ret; - njs_value_t *this, value; + njs_value_t *this, a, value; njs_array_t *array; this = njs_argument(args, 0); @@ -1439,7 +1439,7 @@ njs_array_prototype_to_spliced(njs_vm_t *vm, njs_value_t *args, return NJS_ERROR; } - njs_set_array(retval, array); + njs_set_array(&a, array); for (i = 0; i < start; i++) { ret = njs_value_property_i64(vm, this, i, &value); @@ -1447,14 +1447,14 @@ njs_array_prototype_to_spliced(njs_vm_t *vm, njs_value_t *args, return NJS_ERROR; } - ret = njs_value_create_data_prop_i64(vm, retval, i, &value, 0); + ret = njs_value_create_data_prop_i64(vm, &a, i, &value, 0); if (njs_slow_path(ret != NJS_OK)) { return ret; } } for (n = 3; to_insert-- > 0; i++, n++) { - ret = njs_value_create_data_prop_i64(vm, retval, i, &args[n], 0); + ret = njs_value_create_data_prop_i64(vm, &a, i, &args[n], 0); if (njs_slow_path(ret != NJS_OK)) { return ret; } @@ -1468,7 +1468,7 @@ njs_array_prototype_to_spliced(njs_vm_t *vm, njs_value_t *args, return NJS_ERROR; } - ret = njs_value_create_data_prop_i64(vm, retval, i, &value, 0); + ret = njs_value_create_data_prop_i64(vm, &a, i, &value, 0); if (njs_slow_path(ret != NJS_OK)) { return ret; } @@ -1477,6 +1477,8 @@ njs_array_prototype_to_spliced(njs_vm_t *vm, njs_value_t *args, i++; } + njs_set_array(retval, array); + return NJS_OK; } @@ -1562,7 +1564,7 @@ njs_array_prototype_to_reversed(njs_vm_t *vm, njs_value_t *args, int64_t length, i; njs_int_t ret; njs_array_t *array; - njs_value_t *this, value; + njs_value_t *this, a, value; this = njs_argument(args, 0); @@ -1581,7 +1583,7 @@ njs_array_prototype_to_reversed(njs_vm_t *vm, njs_value_t *args, return NJS_ERROR; } - njs_set_array(retval, array); + njs_set_array(&a, array); for (i = 0; i < length; i++) { ret = njs_value_property_i64(vm, this, length - i - 1, &value); @@ -1589,12 +1591,14 @@ njs_array_prototype_to_reversed(njs_vm_t *vm, njs_value_t *args, return NJS_ERROR; } - ret = njs_value_create_data_prop_i64(vm, retval, i, &value, 0); + ret = njs_value_create_data_prop_i64(vm, &a, i, &value, 0); if (njs_slow_path(ret != NJS_OK)) { return ret; } } + njs_set_array(retval, array); + return NJS_OK; } @@ -3018,7 +3022,7 @@ njs_array_prototype_to_sorted(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs, int64_t i, nslots, nunds, length; njs_int_t ret; njs_array_t *array; - njs_value_t *this, *comparefn; + njs_value_t *this, *comparefn, a; njs_function_t *compare; njs_array_sort_slot_t *slots; @@ -3070,24 +3074,25 @@ njs_array_prototype_to_sorted(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs, njs_assert(length == (nslots + nunds)); - njs_set_array(retval, array); + njs_set_array(&a, array); for (i = 0; i < nslots; i++) { - ret = njs_value_property_i64_set(vm, retval, i, &slots[i].value); - if (njs_slow_path(ret == NJS_ERROR)) { + ret = njs_value_create_data_prop_i64(vm, &a, i, &slots[i].value, 0); + if (njs_slow_path(ret != NJS_OK)) { goto exception; } } for (; i < length; i++) { - ret = njs_value_property_i64_set(vm, retval, i, - njs_value_arg(&njs_value_undefined)); - if (njs_slow_path(ret == NJS_ERROR)) { + ret = njs_value_create_data_prop_i64(vm, &a, i, + njs_value_arg(&njs_value_undefined), 0); + if (njs_slow_path(ret != NJS_OK)) { goto exception; } } ret = NJS_OK; + njs_set_array(retval, array); exception: diff --git a/src/njs_object.c b/src/njs_object.c index 1ecf5f07..1b92437c 100644 --- a/src/njs_object.c +++ b/src/njs_object.c @@ -2285,12 +2285,18 @@ static njs_int_t njs_object_prototype_value_of(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs, njs_index_t unused, njs_value_t *retval) { - njs_value_assign(retval, njs_argument(args, 0)); + njs_value_t *value; + + value = njs_argument(args, 0); - if (!njs_is_object(retval)) { - return njs_value_to_object(vm, retval); + if (!njs_is_object(value)) { + if (njs_value_to_object(vm, value) != NJS_OK) { + return NJS_ERROR; + } } + njs_value_assign(retval, value); + return NJS_OK; } diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c index 981404ee..3edf60c3 100644 --- a/src/test/njs_unit_test.c +++ b/src/test/njs_unit_test.c @@ -5181,6 +5181,12 @@ static njs_unit_test_t njs_test[] = { njs_str("'/A/B/C/D/'.split('/').toSpliced(1,1).join('/')"), njs_str("/B/C/D/") }, + { njs_str("let r, arr = new Array(4);" + "Object.defineProperty(arr, 0, { get: () => { throw 'Oops'; } });" + "try { r = arr.toSpliced(0, 0); } catch (e) { }" + "r.toString()"), + njs_str("TypeError: cannot get property \"toString\" of undefined") }, + { njs_str("var a = []; a.reverse()"), njs_str("") }, @@ -5237,6 +5243,12 @@ static njs_unit_test_t njs_test[] = { njs_str("Array.prototype[0] = 0; var x = [,1]; x.reverse(); x"), njs_str("1,0") }, + { njs_str("let r, arr = new Array(4);" + "Object.defineProperty(arr, 0, { get: () => { throw 'Oops'; } });" + "try { r = arr.toReversed(0, 0); } catch (e) { }" + "r.toString()"), + njs_str("TypeError: cannot get property \"toString\" of undefined") }, + { njs_str("var a = [,3,2,1]; njs.dump([a.toReversed(),a])"), njs_str("[[1,2,3,undefined],[,3,2,1]]") }, -- 2.47.3