Skip to content

Commit

Permalink
Fixed retval handling after an exception.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@xeioex
xeioex committed May 23, 2024
1 parent 09d04c3 commit 6d624bb
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 20 deletions.
39 changes: 22 additions & 17 deletions src/njs_array.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1439,22 +1439,22 @@ 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);
if (njs_slow_path(ret == NJS_ERROR)) {
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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}

Expand Down Expand Up @@ -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);

Expand All @@ -1581,20 +1583,22 @@ 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);
if (njs_slow_path(ret == NJS_ERROR)) {
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;
}

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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:

Expand Down
12 changes: 9 additions & 3 deletions src/njs_object.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
12 changes: 12 additions & 0 deletions src/test/njs_unit_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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("") },

Expand Down Expand Up @@ -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],[<empty>,3,2,1]]") },

Expand Down

0 comments on commit 6d624bb

Please sign in to comment.