Discussion:
[njs] Extended Object.defineProperty() spec conformance.
Dmitry Volyntsev
2018-11-15 17:32:03 UTC
Permalink
details: http://hg.nginx.org/njs/rev/5f0090c02589
branches:
changeset: 657:5f0090c02589
user: Dmitry Volyntsev <***@nginx.com>
date: Thu Nov 15 20:31:35 2018 +0300
description:
Extended Object.defineProperty() spec conformance.

1) non-primitive property names.
2) support of array index properties.
3) support of special properties with
NJS_PROPERTY_HANDLER, like length.

diffstat:

njs/njs_object.c | 128 ++++++++++++++++++++++++++++++++--------------
njs/test/njs_unit_test.c | 43 +++++++++++++++
2 files changed, 131 insertions(+), 40 deletions(-)

diffs (278 lines):

diff -r 93ef4b20c674 -r 5f0090c02589 njs/njs_object.c
--- a/njs/njs_object.c Thu Nov 15 20:31:35 2018 +0300
+++ b/njs/njs_object.c Thu Nov 15 20:31:35 2018 +0300
@@ -24,7 +24,7 @@ static njs_ret_t njs_external_property_d
njs_value_t *setval, njs_value_t *retval);
static njs_ret_t njs_object_query_prop_handler(njs_property_query_t *pq,
njs_object_t *object);
-static njs_ret_t njs_define_property(njs_vm_t *vm, njs_object_t *object,
+static njs_ret_t njs_define_property(njs_vm_t *vm, njs_value_t *object,
const njs_value_t *name, const njs_object_t *descriptor);


@@ -967,17 +967,18 @@ static njs_ret_t
njs_object_define_property(njs_vm_t *vm, njs_value_t *args, nxt_uint_t nargs,
njs_index_t unused)
{
- nxt_int_t ret;
- const njs_value_t *value, *name, *descriptor;
-
- value = njs_arg(args, nargs, 1);
-
- if (!njs_is_object(value)) {
+ nxt_int_t ret;
+ njs_value_t *value;
+ const njs_value_t *name, *descriptor;
+
+ if (!njs_is_object(njs_arg(args, nargs, 1))) {
njs_type_error(vm, "cannot convert %s argument to object",
- njs_type_string(value->type));
+ njs_type_string(njs_arg(args, nargs, 1)->type));
return NXT_ERROR;
}

+ value = &args[1];
+
if (!value->data.u.object->extensible) {
njs_type_error(vm, "object is not extensible");
return NXT_ERROR;
@@ -985,16 +986,14 @@ njs_object_define_property(njs_vm_t *vm,

descriptor = njs_arg(args, nargs, 3);

- if (!njs_is_object(descriptor)){
+ if (!njs_is_object(descriptor)) {
njs_type_error(vm, "descriptor is not an object");
return NXT_ERROR;
}

name = njs_arg(args, nargs, 2);

- ret = njs_define_property(vm, value->data.u.object, name,
- descriptor->data.u.object);
-
+ ret = njs_define_property(vm, value, name, descriptor->data.u.object);
if (nxt_slow_path(ret != NXT_OK)) {
return NXT_ERROR;
}
@@ -1010,13 +1009,13 @@ njs_object_define_properties(njs_vm_t *v
njs_index_t unused)
{
nxt_int_t ret;
+ njs_value_t *value;
nxt_lvlhsh_t *hash;
- njs_object_t *object;
nxt_lvlhsh_each_t lhe;
njs_object_prop_t *prop;
- const njs_value_t *value, *descriptor;
-
- value = njs_arg(args, nargs, 1);
+ const njs_value_t *descriptor;
+
+ value = &args[1];

if (!njs_is_object(value)) {
njs_type_error(vm, "cannot convert %s argument to object",
@@ -1039,7 +1038,6 @@ njs_object_define_properties(njs_vm_t *v

nxt_lvlhsh_each_init(&lhe, &njs_object_hash_proto);

- object = value->data.u.object;
hash = &descriptor->data.u.object->hash;

for ( ;; ) {
@@ -1050,7 +1048,7 @@ njs_object_define_properties(njs_vm_t *v
}

if (prop->enumerable && njs_is_object(&prop->value)) {
- ret = njs_define_property(vm, object, &prop->name,
+ ret = njs_define_property(vm, value, &prop->name,
prop->value.data.u.object);

if (nxt_slow_path(ret != NXT_OK)) {
@@ -1121,22 +1119,29 @@ njs_descriptor_prop(njs_vm_t *vm, const

/*
* ES5.1, 8.12.9: [[DefineOwnProperty]]
- * Only data descriptors are suppored.
+ * Limited support of special descriptors like length and array index
+ * (values can be set, but without property flags support).
*/
static njs_ret_t
-njs_define_property(njs_vm_t *vm, njs_object_t *object, const njs_value_t *name,
+njs_define_property(njs_vm_t *vm, njs_value_t *object, const njs_value_t *name,
const njs_object_t *descriptor)
{
- nxt_int_t ret;
- nxt_bool_t unset;
- njs_object_prop_t *desc, *current;
- nxt_lvlhsh_query_t lhq;
-
- njs_string_get(name, &lhq.key);
- lhq.key_hash = nxt_djb_hash(lhq.key.start, lhq.key.length);
- lhq.proto = &njs_object_hash_proto;
-
- ret = nxt_lvlhsh_find(&object->hash, &lhq);
+ nxt_int_t ret;
+ nxt_bool_t unset;
+ njs_object_prop_t *desc, *current;
+ njs_property_query_t pq;
+
+ njs_string_get(name, &pq.lhq.key);
+ pq.lhq.key_hash = nxt_djb_hash(pq.lhq.key.start, pq.lhq.key.length);
+ pq.lhq.proto = &njs_object_hash_proto;
+
+ njs_property_query_init(&pq, NJS_PROPERTY_QUERY_SET, 0);
+
+ ret = njs_property_query(vm, &pq, object, name);
+
+ if (ret != NXT_OK && ret != NXT_DECLINED) {
+ return ret;
+ }

unset = (ret == NXT_OK);
desc = njs_descriptor_prop(vm, name, descriptor, unset);
@@ -1145,14 +1150,24 @@ njs_define_property(njs_vm_t *vm, njs_ob
}

if (nxt_fast_path(ret == NXT_DECLINED)) {
- lhq.value = desc;
- lhq.replace = 0;
- lhq.pool = vm->mem_cache_pool;
-
- ret = nxt_lvlhsh_insert(&object->hash, &lhq);
- if (nxt_slow_path(ret != NXT_OK)) {
- njs_internal_error(vm, "lvlhsh insert failed");
- return NXT_ERROR;
+ if (nxt_slow_path(pq.lhq.value != NULL)) {
+ current = pq.lhq.value;
+
+ if (nxt_slow_path(current->type == NJS_WHITEOUT)) {
+ /* Previously deleted property. */
+ *current = *desc;
+ }
+
+ } else {
+ pq.lhq.value = desc;
+ pq.lhq.replace = 0;
+ pq.lhq.pool = vm->mem_cache_pool;
+
+ ret = nxt_lvlhsh_insert(&object->data.u.object->hash, &pq.lhq);
+ if (nxt_slow_path(ret != NXT_OK)) {
+ njs_internal_error(vm, "lvlhsh insert failed");
+ return NXT_ERROR;
+ }
}

return NXT_OK;
@@ -1160,7 +1175,40 @@ njs_define_property(njs_vm_t *vm, njs_ob

/* Updating existing prop. */

- current = lhq.value;
+ current = pq.lhq.value;
+
+ switch (current->type) {
+ case NJS_PROPERTY:
+ break;
+
+ case NJS_PROPERTY_REF:
+ if (njs_is_valid(&desc->value)) {
+ *current->value.data.u.value = desc->value;
+ } else {
+ *current->value.data.u.value = njs_value_void;
+ }
+
+ return NXT_OK;
+
+ case NJS_PROPERTY_HANDLER:
+ if (current->writable && njs_is_valid(&desc->value)) {
+ ret = current->value.data.u.prop_handler(vm, object, &desc->value,
+ &vm->retval);
+
+ if (nxt_slow_path(ret != NXT_OK)) {
+ return ret;
+ }
+ }
+
+ return NXT_OK;
+
+ default:
+ njs_internal_error(vm, "unexpected property type '%s' "
+ "while defining property",
+ njs_prop_type_string(current->type));
+
+ return NXT_ERROR;
+ }

if (!current->configurable) {
if (desc->configurable == NJS_ATTRIBUTE_TRUE) {
@@ -1208,7 +1256,7 @@ njs_define_property(njs_vm_t *vm, njs_ob
exception:

njs_type_error(vm, "Cannot redefine property: '%.*s'",
- (int) lhq.key.length, lhq.key.start);
+ (int) pq.lhq.key.length, pq.lhq.key.start);

return NXT_ERROR;
}
diff -r 93ef4b20c674 -r 5f0090c02589 njs/test/njs_unit_test.c
--- a/njs/test/njs_unit_test.c Thu Nov 15 20:31:35 2018 +0300
+++ b/njs/test/njs_unit_test.c Thu Nov 15 20:31:35 2018 +0300
@@ -2933,6 +2933,9 @@ static njs_unit_test_t njs_test[] =
{ nxt_string("[].length = 2**32 - 1"),
nxt_string("MemoryError") },

+ { nxt_string("Object.defineProperty([], 'length',{value: 2**32 - 1})"),
+ nxt_string("MemoryError") },
+
{ nxt_string("[].length = 2**32"),
nxt_string("RangeError: Invalid array length") },

@@ -7369,6 +7372,46 @@ static njs_unit_test_t njs_test[] =
"Object.keys(o)"),
nxt_string("b") },

+ { nxt_string("var o = {a:1}; delete o.a;"
+ "Object.defineProperty(o, 'a', { value: 1 }); o.a"),
+ nxt_string("1") },
+
+ { nxt_string("var o = {a:1}; delete o.a;"
+ "Object.defineProperty(o, 'a', { value: 1 }); o.a = 2; o.a"),
+ nxt_string("TypeError: Cannot assign to read-only property 'a' of object") },
+
+ { nxt_string("var o = {a:1}; delete o.a;"
+ "Object.defineProperty(o, 'a', { value: 1, writable:1 }); o.a = 2; o.a"),
+ nxt_string("2") },
+
+ { nxt_string("var o = {};"
+ "Object.defineProperty(o, new String('a'), { value: 1}); o.a"),
+ nxt_string("1") },
+
+ { nxt_string("var o = {};"
+ "Object.defineProperty(o, {toString:function(){return 'a'}}, { value: 1}); o.a"),
+ nxt_string("1") },
+
+ { nxt_string("var a = [1, 2];"
+ "Object.defineProperty(a, '1', {value: 5}); a[1]"),
+ nxt_string("5") },
+
+ { nxt_string("var a = [1, 2];"
+ "Object.defineProperty(a, '3', {}); njs.dump(a)"),
+ nxt_string("[1,2,<empty>,undefined]") },
+
+ { nxt_string("var a = [1, 2];"
+ "Object.defineProperty(a, 'length', {}); a"),
+ nxt_string("1,2") },
+
+ { nxt_string("var a = [1, 2];"
+ "Object.defineProperty(a, 'length', {value: 1}); a"),
+ nxt_string("1") },
+
+ { nxt_string("var a = [1, 2];"
+ "Object.defineProperty(a, 'length', {value: 5}); a"),
+ nxt_string("1,2,,,") },
+
{ nxt_string("var o = Object.defineProperties({a:1}, {}); o.a"),
nxt_string("1") },

Loading...