Fixed Object.defineProperty() for existing properties.

This fixes #46 issues on Github.

diff --git a/njs/njs_object.c b/njs/njs_object.c
index 5cfbae731f4eafdaaa583db7a543c0fefd0e99d1..12cc6c17d8f649865f91d4c868d4b425b665eae8 100644 (file)
--- a/njs/njs_object.c
+++ b/njs/njs_object.c
@@ -828,79 +828,152 @@ njs_object_define_properties(njs_vm_t *vm, njs_value_t *args, nxt_uint_t nargs,
 }
 
 
-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)
+static uint8_t
+njs_descriptor_attribute(njs_vm_t *vm, const njs_object_t *descriptor,
+    nxt_lvlhsh_query_t *pq, nxt_bool_t unset)
 {
-    nxt_int_t           ret;
-    njs_object_prop_t   *prop, *pr;
-    nxt_lvlhsh_query_t  lhq, pq;
-
-    njs_string_get(name, &lhq.key);
-    lhq.key_hash = nxt_djb_hash(lhq.key.start, lhq.key.length);
-    lhq.proto = &njs_object_hash_proto;
+    njs_object_prop_t  *prop;
 
-    ret = nxt_lvlhsh_find(&object->hash, &lhq);
+    prop = njs_object_property(vm, descriptor, pq);
+    if (prop != NULL) {
+        return prop->value.data.truth;
+    }
 
-    if (ret != NXT_OK) {
-        prop = njs_object_prop_alloc(vm, name, &njs_value_void, 0);
+    return unset ? NJS_ATTRIBUTE_UNSET : 0;
+}
 
-        if (nxt_slow_path(prop == NULL)) {
-            return NXT_ERROR;
-        }
 
-        lhq.value = prop;
+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;
 
-    } else {
-        prop = lhq.value;
+    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;
     }
 
-    pq.key = nxt_string_value("configurable");
-    pq.key_hash = NJS_CONFIGURABLE_HASH;
+    return prop;
+}
 
-    pr = njs_object_property(vm, descriptor, &pq);
 
-    if (pr != NULL) {
-        prop->configurable = pr->value.data.truth;
+/*
+ * 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;
+    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);
+
+    unset = (ret == NXT_OK);
+    desc = njs_descriptor_prop(vm, name, descriptor, unset);
+    if (nxt_slow_path(desc == NULL)) {
+        return NXT_ERROR;
     }
 
-    pq.key = nxt_string_value("enumerable");
-    pq.key_hash = NJS_ENUMERABLE_HASH;
+    if (nxt_fast_path(ret == NXT_DECLINED)) {
+        lhq.value = desc;
+        lhq.replace = 0;
+        lhq.pool = vm->mem_cache_pool;
 
-    pr = njs_object_property(vm, descriptor, &pq);
+        ret = nxt_lvlhsh_insert(&object->hash, &lhq);
+        if (nxt_slow_path(ret != NXT_OK)) {
+            njs_internal_error(vm, NULL);
+            return NXT_ERROR;
+        }
 
-    if (pr != NULL) {
-        prop->enumerable = pr->value.data.truth;
+        return NXT_OK;
     }
 
-    pq.key = nxt_string_value("writable");
-    pq.key_hash = NJS_WRITABABLE_HASH;
+    /* Updating existing prop. */
 
-    pr = njs_object_property(vm, descriptor, &pq);
+    current = lhq.value;
 
-    if (pr != NULL) {
-        prop->writable = pr->value.data.truth;
+    if (!current->configurable) {
+        if (desc->configurable == NJS_ATTRIBUTE_TRUE) {
+            goto exception;
+        }
+
+        if (desc->enumerable != NJS_ATTRIBUTE_UNSET
+            && current->enumerable != desc->enumerable)
+        {
+            goto exception;
+        }
+
+        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;
+        }
     }
 
-    lhq.replace = 0;
-    lhq.pool = vm->mem_cache_pool;
+    if (desc->configurable != NJS_ATTRIBUTE_UNSET) {
+        current->configurable = desc->configurable;
+    }
 
-    ret = nxt_lvlhsh_insert(&object->hash, &lhq);
-    if (nxt_slow_path(ret != NXT_OK)) {
-        return NXT_ERROR;
+    if (desc->enumerable != NJS_ATTRIBUTE_UNSET) {
+        current->enumerable = desc->enumerable;
+    }
+
+    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 --git a/njs/njs_object.h b/njs/njs_object.h
index 29c39ac1cd715e54cd7b2f956b8c3304872f9bc9..0026263237a1cac5c7edf7e194eb14a91f446b20 100644 (file)
--- a/njs/njs_object.h
+++ b/njs/njs_object.h
@@ -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 --git a/njs/test/njs_unit_test.c b/njs/test/njs_unit_test.c
index a42f0d755182f2e6d70cb08908fb7bed37068fd2..3e2a65bf72bc99be2b099973ba83f8b5e57e1b25 100644 (file)
--- a/njs/test/njs_unit_test.c
+++ b/njs/test/njs_unit_test.c
@@ -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") },