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.

diff --git a/src/njs_object.c b/src/njs_object.c
index ff74ffe43e2a871326b7c66df882254d86484a2d..580634f060926ef463e7b25c49bbce662d1a3b76 100644 (file)
--- a/src/njs_object.c
+++ b/src/njs_object.c
@@ -2450,7 +2450,7 @@ njs_object_prototype_has_own_property(njs_vm_t *vm, njs_value_t *args,
     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(njs_vm_t *vm, njs_value_t *args,
         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_vm_t *vm, njs_value_t *args,
     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(njs_vm_t *vm, njs_value_t *args,
         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 --git a/src/njs_value.c b/src/njs_value.c
index 1c583aff8a0ebeda65d5ee21f3d03ee1415a78cc..7e8597f307d6bfb75fc78a64375b9c24cd7909a8 100644 (file)
--- a/src/njs_value.c
+++ b/src/njs_value.c
@@ -535,20 +535,11 @@ njs_property_query(njs_vm_t *vm, njs_property_query_t *pq, njs_value_t *value,
     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_value_t *value, njs_value_t *key,
     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_value_t *value, njs_value_t *key,
 
     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 *value, njs_value_t *key,
     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 --git a/src/njs_value.h b/src/njs_value.h
index 6507c09803da2c4fff4a1e50ddeb584b451972e4..e42ddda98a3d3bfb18c34225c69a486002b67351 100644 (file)
--- a/src/njs_value.h
+++ b/src/njs_value.h
@@ -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 --git a/src/njs_vmcode.c b/src/njs_vmcode.c
index 3427570b3be1f72a1d6cf93673b787148992504f..e0785f0b68824cadf032a04f98e6f91ed52a407c 100644 (file)
--- a/src/njs_vmcode.c
+++ b/src/njs_vmcode.c
@@ -58,6 +58,8 @@ static njs_jump_off_t njs_vmcode_try_end(njs_vm_t *vm, njs_value_t *invld,
 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_value_t *value, njs_value_t *key)
         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 *pc)
         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 --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index 74a82d5003f18658b9a90a91569a7a540ed2897e..2f9e1b3cc40d24540310c03ea7d6ebc53c5d33f1 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -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()"),