[njs] Fixed property set instruction when key modifies base binding.

Dmitry Volyntsev xeioex at nginx.com
Fri Sep 16 06:46:15 UTC 2022


details:   https://hg.nginx.org/njs/rev/23caaca15f08
branches:  
changeset: 1955:23caaca15f08
user:      Dmitry Volyntsev <xeioex at nginx.com>
date:      Thu Sep 15 20:20:10 2022 -0700
description:
Fixed property set instruction when key modifies base binding.

Previously, when obj[prop] expression was evaluated, and prop was an
object with custom "toString" method, which modifies obj binding as its
side-effect, the binding update was visible to property set instruction
which is not correct.

This closes #550 issue on Github.

diffstat:

 src/njs_object.c         |  22 ++++++++++--
 src/njs_value.c          |  25 +++++++++------
 src/njs_value.h          |   4 ++
 src/njs_vmcode.c         |  80 ++++++++++++++++++++++++++++++++++++++++++++++-
 src/test/njs_unit_test.c |  16 +++++++++-
 5 files changed, 129 insertions(+), 18 deletions(-)

diffs (294 lines):

diff -r 75922905bd9c -r 23caaca15f08 src/njs_object.c
--- a/src/njs_object.c	Thu Sep 15 17:56:35 2022 -0700
+++ b/src/njs_object.c	Thu Sep 15 20:20:10 2022 -0700
@@ -2450,7 +2450,7 @@ njs_object_prototype_has_own_property(nj
     njs_uint_t nargs, njs_index_t unused)
 {
     njs_int_t             ret;
-    njs_value_t           *value, *property;
+    njs_value_t           *value, *property, lvalue;
     njs_property_query_t  pq;
 
     value = njs_argument(args, 0);
@@ -2461,7 +2461,14 @@ njs_object_prototype_has_own_property(nj
         return NJS_ERROR;
     }
 
-    property = njs_arg(args, nargs, 1);
+    property = njs_lvalue_arg(&lvalue, args, nargs, 1);
+
+    if (njs_slow_path(!njs_is_key(property))) {
+        ret = njs_value_to_key(vm, property, property);
+        if (njs_slow_path(ret != NJS_OK)) {
+            return NJS_ERROR;
+        }
+    }
 
     njs_property_query_init(&pq, NJS_PROPERTY_QUERY_GET, 1);
 
@@ -2488,7 +2495,7 @@ njs_object_prototype_prop_is_enumerable(
     njs_uint_t nargs, njs_index_t unused)
 {
     njs_int_t             ret;
-    njs_value_t           *value, *property;
+    njs_value_t           *value, *property, lvalue;
     const njs_value_t     *retval;
     njs_object_prop_t     *prop;
     njs_property_query_t  pq;
@@ -2501,7 +2508,14 @@ njs_object_prototype_prop_is_enumerable(
         return NJS_ERROR;
     }
 
-    property = njs_arg(args, nargs, 1);
+    property = njs_lvalue_arg(&lvalue, args, nargs, 1);
+
+    if (njs_slow_path(!njs_is_key(property))) {
+        ret = njs_value_to_key(vm, property, property);
+        if (njs_slow_path(ret != NJS_OK)) {
+            return NJS_ERROR;
+        }
+    }
 
     njs_property_query_init(&pq, NJS_PROPERTY_QUERY_GET, 1);
 
diff -r 75922905bd9c -r 23caaca15f08 src/njs_value.c
--- a/src/njs_value.c	Thu Sep 15 17:56:35 2022 -0700
+++ b/src/njs_value.c	Thu Sep 15 20:20:10 2022 -0700
@@ -535,20 +535,11 @@ njs_property_query(njs_vm_t *vm, njs_pro
     uint32_t        index;
     njs_int_t       ret;
     njs_object_t    *obj;
-    njs_value_t     prop;
     njs_function_t  *function;
 
-    if (njs_slow_path(!njs_is_primitive(key))) {
-        ret = njs_value_to_string(vm, &prop, key);
-        if (ret != NJS_OK) {
-            return ret;
-        }
-
-        key = ∝
-    }
+    njs_assert(njs_is_index_or_key(key));
 
     switch (value->type) {
-
     case NJS_BOOLEAN:
     case NJS_NUMBER:
     case NJS_SYMBOL:
@@ -1004,6 +995,8 @@ njs_value_property(njs_vm_t *vm, njs_val
     njs_typed_array_t     *tarray;
     njs_property_query_t  pq;
 
+    njs_assert(njs_is_index_or_key(key));
+
     if (njs_fast_path(njs_is_number(key))) {
         num = njs_number(key);
 
@@ -1135,6 +1128,8 @@ njs_value_property_set(njs_vm_t *vm, njs
 
     static const njs_str_t  length_key = njs_str("length");
 
+    njs_assert(njs_is_index_or_key(key));
+
     if (njs_fast_path(njs_is_number(key))) {
         num = njs_number(key);
 
@@ -1330,9 +1325,19 @@ njs_value_property_delete(njs_vm_t *vm, 
     njs_value_t *removed, njs_bool_t thrw)
 {
     njs_int_t             ret;
+    njs_value_t           primitive;
     njs_object_prop_t     *prop;
     njs_property_query_t  pq;
 
+    if (njs_slow_path(!njs_is_key(key))) {
+        ret = njs_value_to_key(vm, &primitive, key);
+        if (njs_slow_path(ret != NJS_OK)) {
+            return NJS_ERROR;
+        }
+
+        key = &primitive;
+    }
+
     njs_property_query_init(&pq, NJS_PROPERTY_QUERY_DELETE, 1);
 
     ret = njs_property_query(vm, &pq, value, key);
diff -r 75922905bd9c -r 23caaca15f08 src/njs_value.h
--- a/src/njs_value.h	Thu Sep 15 17:56:35 2022 -0700
+++ b/src/njs_value.h	Thu Sep 15 20:20:10 2022 -0700
@@ -532,6 +532,10 @@ typedef struct {
     (njs_is_string(value) || njs_is_symbol(value))
 
 
+#define njs_is_index_or_key(value)                                            \
+    (njs_is_number(value) || njs_is_key(value))
+
+
 /*
  * The truth field coincides with short_string.size and short_string.length
  * so when string size and length are zero the string's value is false and
diff -r 75922905bd9c -r 23caaca15f08 src/njs_vmcode.c
--- a/src/njs_vmcode.c	Thu Sep 15 17:56:35 2022 -0700
+++ b/src/njs_vmcode.c	Thu Sep 15 20:20:10 2022 -0700
@@ -58,6 +58,8 @@ static njs_jump_off_t njs_vmcode_try_end
 static njs_jump_off_t njs_vmcode_finally(njs_vm_t *vm, njs_value_t *invld,
     njs_value_t *retval, u_char *pc);
 static void njs_vmcode_error(njs_vm_t *vm, u_char *pc);
+static njs_int_t njs_throw_cannot_property(njs_vm_t *vm, njs_value_t *object,
+    njs_value_t *key, const char *what);
 
 static njs_jump_off_t njs_string_concat(njs_vm_t *vm, njs_value_t *val1,
     njs_value_t *val2);
@@ -185,6 +187,21 @@ next:
                 get = (njs_vmcode_prop_get_t *) pc;
                 njs_vmcode_operand(vm, get->value, retval);
 
+                if (njs_slow_path(!njs_is_index_or_key(value2))) {
+                    if (njs_slow_path(njs_is_null_or_undefined(value1))) {
+                        (void) njs_throw_cannot_property(vm, value1, value2,
+                                                         "get");
+                        goto error;
+                    }
+
+                    ret = njs_value_to_key(vm, &primitive1, value2);
+                    if (njs_slow_path(ret != NJS_OK)) {
+                        goto error;
+                    }
+
+                    value2 = &primitive1;
+                }
+
                 ret = njs_value_property(vm, value1, value2, retval);
                 if (njs_slow_path(ret == NJS_ERROR)) {
                     goto error;
@@ -670,6 +687,23 @@ next:
                 set = (njs_vmcode_prop_set_t *) pc;
                 njs_vmcode_operand(vm, set->value, retval);
 
+                if (njs_slow_path(!njs_is_index_or_key(value2))) {
+                    if (njs_slow_path(njs_is_null_or_undefined(value1))) {
+                        (void) njs_throw_cannot_property(vm, value1, value2,
+                                                         "set");
+                        goto error;
+                    }
+
+                    njs_value_assign(&primitive1, value1);
+                    ret = njs_value_to_key(vm, &primitive2, value2);
+                    if (njs_slow_path(ret != NJS_OK)) {
+                        goto error;
+                    }
+
+                    value1 = &primitive1;
+                    value2 = &primitive2;
+                }
+
                 ret = njs_value_property_set(vm, value1, value2, retval);
                 if (njs_slow_path(ret == NJS_ERROR)) {
                     goto error;
@@ -768,6 +802,21 @@ next:
             case NJS_VMCODE_METHOD_FRAME:
                 method_frame = (njs_vmcode_method_frame_t *) pc;
 
+                if (njs_slow_path(!njs_is_key(value2))) {
+                    if (njs_slow_path(njs_is_null_or_undefined(value1))) {
+                        (void) njs_throw_cannot_property(vm, value1, value2,
+                                                         "get");
+                        goto error;
+                    }
+
+                    ret = njs_value_to_key(vm, &primitive1, value2);
+                    if (njs_slow_path(ret != NJS_OK)) {
+                        goto error;
+                    }
+
+                    value2 = &primitive1;
+                }
+
                 ret = njs_value_property(vm, value1, value2, &dst);
                 if (njs_slow_path(ret == NJS_ERROR)) {
                     goto error;
@@ -1417,6 +1466,7 @@ static njs_jump_off_t
 njs_vmcode_property_in(njs_vm_t *vm, njs_value_t *value, njs_value_t *key)
 {
     njs_int_t             ret;
+    njs_value_t           primitive;
     njs_property_query_t  pq;
 
     if (njs_slow_path(njs_is_primitive(value))) {
@@ -1425,11 +1475,13 @@ njs_vmcode_property_in(njs_vm_t *vm, njs
         return NJS_ERROR;
     }
 
-    if (njs_slow_path(!njs_is_key(key))) {
-        ret = njs_value_to_key(vm, key, key);
+    if (njs_slow_path(!njs_is_index_or_key(key))) {
+        ret = njs_value_to_key(vm, &primitive, key);
         if (njs_slow_path(ret != NJS_OK)) {
-            return ret;
+            return NJS_ERROR;
         }
+
+        key = &primitive;
     }
 
     njs_property_query_init(&pq, NJS_PROPERTY_QUERY_GET, 0);
@@ -2186,3 +2238,25 @@ njs_vmcode_error(njs_vm_t *vm, u_char *p
         njs_error_fmt_new(vm, &vm->retval, err->type, "%V", &err->u.message);
     }
 }
+
+
+static njs_int_t
+njs_throw_cannot_property(njs_vm_t *vm, njs_value_t *object, njs_value_t *key,
+    const char *what)
+{
+    njs_int_t    ret;
+    njs_str_t    string;
+    njs_value_t  dst;
+
+    ret = njs_value_to_key2(vm, &dst, key, 0);
+    if (njs_slow_path(ret != NJS_OK)) {
+        return NJS_ERROR;
+    }
+
+    njs_key_string_get(vm, &dst, &string);
+
+    njs_type_error(vm, "cannot %s property \"%V\" of %s", what,
+                   &string, njs_is_null(object) ? "null" : "undefined");
+
+    return NJS_OK;
+}
diff -r 75922905bd9c -r 23caaca15f08 src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c	Thu Sep 15 17:56:35 2022 -0700
+++ b/src/test/njs_unit_test.c	Thu Sep 15 20:20:10 2022 -0700
@@ -3462,7 +3462,7 @@ static njs_unit_test_t  njs_test[] =
     { njs_str("function f() { Object.prototype.toString = 1; };"
               "Object.prototype.toString = f;"
               "(function () { try { 's'[{}](); } catch (e) { throw e; } })()"),
-      njs_str("TypeError: Cannot convert object to primitive value") },
+      njs_str("TypeError: (intermediate value)[\"undefined\"] is not a function") },
 
     { njs_str("var i; for (i = 0; i < 10; i++) { i += 1 } i"),
       njs_str("10") },
@@ -4409,6 +4409,20 @@ static njs_unit_test_t  njs_test[] =
                  "var a = [1,2]; a[1.5] = 5; '' + (n in a) + (delete a[n])"),
       njs_str("truetrue") },
 
+    { njs_str("var o = {},  v = o;"
+              "v[{toString: () => { v = 'V'; return 'a';}}] = 1;"
+              "[v, o.a]"),
+      njs_str("V,1") },
+
+    { njs_str("var o = null; o[{toString:()=>{throw 'OOps'}}]"),
+      njs_str("TypeError: cannot get property \"[object Object]\" of null") },
+
+    { njs_str("var o = null; o[{toString:()=>{throw 'OOps'}}]()"),
+      njs_str("TypeError: cannot get property \"[object Object]\" of null") },
+
+    { njs_str("var o = null; o[{toString:()=>{throw 'OOps'}}] = 1"),
+      njs_str("TypeError: cannot set property \"[object Object]\" of null") },
+
     /**/
 
     { njs_str("Array.isArray()"),



More information about the nginx-devel mailing list