[njs] Fixed retval handling after an exception.

Dmitry Volyntsev xeioex at nginx.com
Thu May 23 16:59:59 UTC 2024


details:   https://hg.nginx.org/njs/rev/585756e8cafe
branches:  
changeset: 2337:585756e8cafe
user:      Dmitry Volyntsev <xeioex at nginx.com>
date:      Wed May 22 23:08:15 2024 -0700
description:
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.

diffstat:

 src/njs_array.c          |  39 ++++++++++++++++++++++-----------------
 src/njs_object.c         |  14 ++++++++++----
 src/test/njs_unit_test.c |  12 ++++++++++++
 3 files changed, 44 insertions(+), 21 deletions(-)

diffs (203 lines):

diff -r 077a5b2f30d8 -r 585756e8cafe src/njs_array.c
--- a/src/njs_array.c	Wed May 22 17:26:16 2024 -0700
+++ b/src/njs_array.c	Wed May 22 23:08:15 2024 -0700
@@ -591,14 +591,14 @@ njs_array_of(njs_vm_t *vm, njs_value_t *
         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 
 {
     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 
         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 
             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 
             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 
         i++;
     }
 
+    njs_set_array(retval, array);
+
     return NJS_OK;
 }
 
@@ -1562,7 +1564,7 @@ njs_array_prototype_to_reversed(njs_vm_t
     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
         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
             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 *
     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 *
 
     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 -r 077a5b2f30d8 -r 585756e8cafe src/njs_object.c
--- a/src/njs_object.c	Wed May 22 17:26:16 2024 -0700
+++ b/src/njs_object.c	Wed May 22 23:08:15 2024 -0700
@@ -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));
-
-    if (!njs_is_object(retval)) {
-        return njs_value_to_object(vm, retval);
+    njs_value_t  *value;
+
+    value = njs_argument(args, 0);
+
+    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 -r 077a5b2f30d8 -r 585756e8cafe src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c	Wed May 22 17:26:16 2024 -0700
+++ b/src/test/njs_unit_test.c	Wed May 22 23:08:15 2024 -0700
@@ -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],[<empty>,3,2,1]]") },
 


More information about the nginx-devel mailing list