details: https://hg.nginx.org/njs/rev/23caaca15f08
branches:
changeset: 1955:23caaca15f08
user: Dmitry Volyntsev <xeioex@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 = &prop;
- }
+ 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()"),
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org