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