[njs] Fixed Object.defineProperty() for existing properties.

Dmitry Volyntsev xeioex at nginx.com
Mon Aug 27 13:55:46 UTC 2018


details:   http://hg.nginx.org/njs/rev/c572768e287b
branches:  
changeset: 586:c572768e287b
user:      Dmitry Volyntsev <xeioex at nginx.com>
date:      Mon Aug 27 16:23:08 2018 +0300
description:
Fixed Object.defineProperty() for existing properties.

This fixes #46 issues on Github.

diffstat:

 njs/njs_object.c         |  163 ++++++++++++++++++++++++++++++++++------------
 njs/njs_object.h         |   19 ++++-
 njs/test/njs_unit_test.c |   61 +++++++++++++++++
 3 files changed, 194 insertions(+), 49 deletions(-)

diffs (306 lines):

diff -r 7ce43874f706 -r c572768e287b njs/njs_object.c
--- a/njs/njs_object.c	Mon Aug 27 16:06:33 2018 +0300
+++ b/njs/njs_object.c	Mon Aug 27 16:23:08 2018 +0300
@@ -828,13 +828,72 @@ njs_object_define_properties(njs_vm_t *v
 }
 
 
+static uint8_t
+njs_descriptor_attribute(njs_vm_t *vm, const njs_object_t *descriptor,
+    nxt_lvlhsh_query_t *pq, nxt_bool_t unset)
+{
+    njs_object_prop_t  *prop;
+
+    prop = njs_object_property(vm, descriptor, pq);
+    if (prop != NULL) {
+        return prop->value.data.truth;
+    }
+
+    return unset ? NJS_ATTRIBUTE_UNSET : 0;
+}
+
+
+static njs_object_prop_t *
+njs_descriptor_prop(njs_vm_t *vm, const njs_value_t *name,
+    const njs_object_t *descriptor, nxt_bool_t unset)
+{
+    const njs_value_t   *value;
+    njs_object_prop_t   *prop, *pr;
+    nxt_lvlhsh_query_t  pq;
+
+    value = unset ? &njs_value_invalid : &njs_value_void;
+    prop = njs_object_prop_alloc(vm, name, value, 0);
+    if (nxt_slow_path(prop == NULL)) {
+        return NULL;
+    }
+
+    pq.key = nxt_string_value("configurable");
+    pq.key_hash = NJS_CONFIGURABLE_HASH;
+    prop->configurable = njs_descriptor_attribute(vm, descriptor, &pq, unset);
+
+    pq.key = nxt_string_value("enumerable");
+    pq.key_hash = NJS_ENUMERABLE_HASH;
+    prop->enumerable = njs_descriptor_attribute(vm, descriptor, &pq, unset);
+
+    pq.key = nxt_string_value("writable");
+    pq.key_hash = NJS_WRITABABLE_HASH;
+    prop->writable = njs_descriptor_attribute(vm, descriptor, &pq, unset);
+
+    pq.key = nxt_string_value("value");
+    pq.key_hash = NJS_VALUE_HASH;
+    pq.proto = &njs_object_hash_proto;
+
+    pr = njs_object_property(vm, descriptor, &pq);
+    if (pr != NULL) {
+        prop->value = pr->value;
+    }
+
+    return prop;
+}
+
+
+/*
+ * ES5.1, 8.12.9: [[DefineOwnProperty]]
+ *      Only data descriptors are suppored.
+ */
 static njs_ret_t
 njs_define_property(njs_vm_t *vm, njs_object_t *object, const njs_value_t *name,
     const njs_object_t *descriptor)
 {
     nxt_int_t           ret;
-    njs_object_prop_t   *prop, *pr;
-    nxt_lvlhsh_query_t  lhq, pq;
+    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);
@@ -842,65 +901,79 @@ njs_define_property(njs_vm_t *vm, njs_ob
 
     ret = nxt_lvlhsh_find(&object->hash, &lhq);
 
-    if (ret != NXT_OK) {
-        prop = njs_object_prop_alloc(vm, name, &njs_value_void, 0);
+    unset = (ret == NXT_OK);
+    desc = njs_descriptor_prop(vm, name, descriptor, unset);
+    if (nxt_slow_path(desc == NULL)) {
+        return NXT_ERROR;
+    }
 
-        if (nxt_slow_path(prop == NULL)) {
+    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, NULL);
             return NXT_ERROR;
         }
 
-        lhq.value = prop;
-
-    } else {
-        prop = lhq.value;
-    }
-
-    pq.key = nxt_string_value("value");
-    pq.key_hash = NJS_VALUE_HASH;
-    pq.proto = &njs_object_hash_proto;
-
-    pr = njs_object_property(vm, descriptor, &pq);
-
-    if (pr != NULL) {
-        prop->value = pr->value;
-    }
-
-    pq.key = nxt_string_value("configurable");
-    pq.key_hash = NJS_CONFIGURABLE_HASH;
-
-    pr = njs_object_property(vm, descriptor, &pq);
-
-    if (pr != NULL) {
-        prop->configurable = pr->value.data.truth;
+        return NXT_OK;
     }
 
-    pq.key = nxt_string_value("enumerable");
-    pq.key_hash = NJS_ENUMERABLE_HASH;
+    /* Updating existing prop. */
+
+    current = lhq.value;
+
+    if (!current->configurable) {
+        if (desc->configurable == NJS_ATTRIBUTE_TRUE) {
+            goto exception;
+        }
 
-    pr = njs_object_property(vm, descriptor, &pq);
+        if (desc->enumerable != NJS_ATTRIBUTE_UNSET
+            && current->enumerable != desc->enumerable)
+        {
+            goto exception;
+        }
 
-    if (pr != NULL) {
-        prop->enumerable = pr->value.data.truth;
+        if (desc->writable == NJS_ATTRIBUTE_TRUE
+            && current->writable == NJS_ATTRIBUTE_FALSE)
+        {
+            goto exception;
+        }
+
+        if (njs_is_valid(&desc->value)
+            && current->writable == NJS_ATTRIBUTE_FALSE
+            && !njs_values_strict_equal(&desc->value, &current->value))
+        {
+            goto exception;
+        }
     }
 
-    pq.key = nxt_string_value("writable");
-    pq.key_hash = NJS_WRITABABLE_HASH;
-
-    pr = njs_object_property(vm, descriptor, &pq);
-
-    if (pr != NULL) {
-        prop->writable = pr->value.data.truth;
+    if (desc->configurable != NJS_ATTRIBUTE_UNSET) {
+        current->configurable = desc->configurable;
     }
 
-    lhq.replace = 0;
-    lhq.pool = vm->mem_cache_pool;
+    if (desc->enumerable != NJS_ATTRIBUTE_UNSET) {
+        current->enumerable = desc->enumerable;
+    }
 
-    ret = nxt_lvlhsh_insert(&object->hash, &lhq);
-    if (nxt_slow_path(ret != NXT_OK)) {
-        return NXT_ERROR;
+    if (desc->writable != NJS_ATTRIBUTE_UNSET) {
+        current->writable = desc->writable;
+    }
+
+    if (njs_is_valid(&desc->value)) {
+        current->value = desc->value;
     }
 
     return NXT_OK;
+
+exception:
+
+    njs_type_error(vm, "Cannot redefine property: '%.*s'",
+                   (int) lhq.key.length, lhq.key.start);
+
+    return NXT_ERROR;
 }
 
 
diff -r 7ce43874f706 -r c572768e287b njs/njs_object.h
--- a/njs/njs_object.h	Mon Aug 27 16:06:33 2018 +0300
+++ b/njs/njs_object.h	Mon Aug 27 16:23:08 2018 +0300
@@ -18,15 +18,26 @@ typedef enum {
 } njs_object_property_type_t;
 
 
+/*
+ * Attributes are generally used as Boolean values.
+ * The UNSET value is used internally only by njs_define_property().
+ */
+typedef enum {
+    NJS_ATTRIBUTE_FALSE = 0,
+    NJS_ATTRIBUTE_TRUE = 1,
+    NJS_ATTRIBUTE_UNSET,
+} njs_object_attribute_t;
+
 typedef struct {
     /* Must be aligned to njs_value_t. */
     njs_value_t                 value;
     njs_value_t                 name;
 
-    njs_object_property_type_t  type:8;        /* 3 bits */
-    uint8_t                     enumerable;    /* 1 bit  */
-    uint8_t                     writable;      /* 1 bit  */
-    uint8_t                     configurable;  /* 1 bit  */
+    njs_object_property_type_t  type:8;          /* 3 bits */
+
+    njs_object_attribute_t      enumerable:8;    /* 2 bits */
+    njs_object_attribute_t      writable:8;      /* 2 bits */
+    njs_object_attribute_t      configurable:8;  /* 2 bits */
 } njs_object_prop_t;
 
 
diff -r 7ce43874f706 -r c572768e287b njs/test/njs_unit_test.c
--- a/njs/test/njs_unit_test.c	Mon Aug 27 16:06:33 2018 +0300
+++ b/njs/test/njs_unit_test.c	Mon Aug 27 16:23:08 2018 +0300
@@ -6717,6 +6717,67 @@ static njs_unit_test_t  njs_test[] =
                  "Object.defineProperty(o, 'a', Object.create({value:2})); o.a"),
       nxt_string("2") },
 
+    { nxt_string("var o = {};"
+                 "Object.defineProperty(o, 'a', {configurable:false});"
+                 "Object.defineProperty(o, 'a', {configurable:true})"),
+      nxt_string("TypeError: Cannot redefine property: 'a'") },
+
+    { nxt_string("var o = {};"
+                 "Object.defineProperty(o, 'a', {configurable:false});"
+                 "Object.defineProperty(o, 'a', {enumerable:true})"),
+      nxt_string("TypeError: Cannot redefine property: 'a'") },
+
+    { nxt_string("var o = {};"
+                 "Object.defineProperty(o, 'a', {configurable:false});"
+                 "Object.defineProperty(o, 'a', {writable:true})"),
+      nxt_string("TypeError: Cannot redefine property: 'a'") },
+
+    { nxt_string("var o = {};"
+                 "Object.defineProperty(o, 'a', {configurable:false});"
+                 "Object.defineProperty(o, 'a', {enumerable:false}).a"),
+      nxt_string("undefined") },
+
+    { nxt_string("var o = {};"
+                 "Object.defineProperty(o, 'a', {configurable:false});"
+                 "Object.defineProperty(o, 'a', {}).a"),
+      nxt_string("undefined") },
+
+    { nxt_string("var o = {};"
+                 "Object.defineProperty(o, 'a', {configurable:false, writable:true});"
+                 "Object.defineProperty(o, 'a', {writable:false}).a"),
+      nxt_string("undefined") },
+
+    { nxt_string("var o = {};"
+                 "Object.defineProperty(o, 'a', {configurable:true, writable:false});"
+                 "Object.defineProperty(o, 'a', {writable:true}).a"),
+      nxt_string("undefined") },
+
+    { nxt_string("var o = {};"
+                 "Object.defineProperty(o, 'a', {});"
+                 "Object.defineProperty(o, 'a', {}).a"),
+      nxt_string("undefined") },
+
+    { nxt_string("var o = {};"
+                 "Object.defineProperty(o, 'a', {value:1});"
+                 "Object.defineProperty(o, 'a', {value:1}).a"),
+      nxt_string("1") },
+
+    { nxt_string("var o = {};"
+                 "Object.defineProperty(o, 'a', {value:1});"
+                 "Object.defineProperty(o, 'a', {value:2}).a"),
+      nxt_string("TypeError: Cannot redefine property: 'a'") },
+
+    { nxt_string("var o = {};"
+                 "Object.defineProperty(o, 'a', {configurable:true});"
+                 "Object.defineProperty(o, 'a', {value:1}).a"),
+      nxt_string("1") },
+
+    { nxt_string("var o = {};"
+                 "Object.defineProperty(o, 'a', { configurable: true, value: 0 });"
+                 "Object.defineProperty(o, 'a', { value: 1 });"
+                 "Object.defineProperty(o, 'a', { configurable: false, value: 2 }).a"),
+      nxt_string("2") },
+
     { nxt_string("var o = {}; Object.defineProperty()"),
       nxt_string("TypeError: cannot convert void argument to object") },
 


More information about the nginx-devel mailing list