Object.getOwnPropertyDescriptor and array length property

[drsm]

the length property in an empty array is handled incorrectly, and for non empty array an invalid result is returned:

>> JSON.stringify(Object.getOwnPropertyDescriptor([], 'length'))
"undefined"
>> JSON.stringify(Object.getOwnPropertyDescriptor([1,2,3], 'length'))
{"value":1,"configurable":"true","enumerable":"true","writable":"true"}

[xeioex]

please, try the patch below

# HG changeset patch
# User Dmitry Volyntsev <xeioex@nginx.com>
# Date 1524245451 -10800
#      Fri Apr 20 20:30:51 2018 +0300
# Node ID e344ea53e91c384daf6aa4d5c7da8fe4e96e28ac
# Parent  b3f737b90fdd511ceaa5f87fa9c886c60e72e4de
Fixed handling of special props in Object.getOwnPropertyDescriptor().

This fixes #6 issue on GitHub.

diff --git a/njs/njs_object.c b/njs/njs_object.c
--- a/njs/njs_object.c
+++ b/njs/njs_object.c
@@ -27,6 +27,8 @@
 
 
 static nxt_int_t njs_object_hash_test(nxt_lvlhsh_query_t *lhq, void *data);
+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,
     njs_value_t *name, njs_object_t *descriptor);
 
@@ -229,6 +231,118 @@ njs_object_property(njs_vm_t *vm, njs_ob
 
 
 njs_ret_t
+njs_object_property_query(njs_vm_t *vm, njs_property_query_t *pq,
+    njs_value_t *value, njs_object_t *object)
+{
+    njs_ret_t          ret;
+    njs_object_prop_t  *prop;
+
+    pq->lhq.proto = &njs_object_hash_proto;
+
+    if (pq->query == NJS_PROPERTY_QUERY_SET) {
+        ret = njs_object_query_prop_handler(pq, object);
+        if (ret == NXT_OK) {
+            return ret;
+        }
+    }
+
+    do {
+        pq->prototype = object;
+
+        ret = nxt_lvlhsh_find(&object->hash, &pq->lhq);
+
+        if (ret == NXT_OK) {
+            prop = pq->lhq.value;
+
+            if (prop->type != NJS_WHITEOUT) {
+                pq->shared = 0;
+
+                return ret;
+            }
+
+            goto next;
+        }
+
+        if (pq->query > NJS_PROPERTY_QUERY_IN) {
+            /* NXT_DECLINED */
+            return ret;
+        }
+
+        ret = nxt_lvlhsh_find(&object->shared_hash, &pq->lhq);
+
+        if (ret == NXT_OK) {
+            pq->shared = 1;
+
+            if (pq->query == NJS_PROPERTY_QUERY_GET) {
+                prop = pq->lhq.value;
+
+                if (prop->type == NJS_PROPERTY_HANDLER) {
+                    pq->scratch = *prop;
+                    prop = &pq->scratch;
+                    ret = prop->value.data.u.prop_handler(vm, value, NULL,
+                                                          &prop->value);
+
+                    if (nxt_fast_path(ret == NXT_OK)) {
+                        prop->type = NJS_PROPERTY;
+                        pq->lhq.value = prop;
+                    }
+                }
+            }
+
+            return ret;
+        }
+
+        if (pq->query > NJS_PROPERTY_QUERY_IN) {
+            /* NXT_DECLINED */
+            return ret;
+        }
+
+    next:
+
+        object = object->__proto__;
+
+    } while (object != NULL);
+
+    if (njs_is_string(value)) {
+        return NJS_STRING_VALUE;
+    }
+
+    /* NXT_DECLINED */
+
+    return ret;
+}
+
+
+static njs_ret_t
+njs_object_query_prop_handler(njs_property_query_t *pq, njs_object_t *object)
+{
+    njs_ret_t          ret;
+    njs_object_prop_t  *prop;
+
+    do {
+        pq->prototype = object;
+
+        ret = nxt_lvlhsh_find(&object->shared_hash, &pq->lhq);
+
+        if (ret == NXT_OK) {
+            prop = pq->lhq.value;
+
+            if (prop->type == NJS_PROPERTY_HANDLER) {
+                return NXT_OK;
+            }
+        }
+
+        object = object->__proto__;
+
+    } while (object != NULL);
+
+    return NXT_DECLINED;
+}
+
+
+
+
+njs_ret_t
 njs_object_constructor(njs_vm_t *vm, njs_value_t *args, nxt_uint_t nargs,
     njs_index_t unused)
 {
@@ -604,13 +718,15 @@ static njs_ret_t
 njs_object_get_own_property_descriptor(njs_vm_t *vm, njs_value_t *args,
     nxt_uint_t nargs, njs_index_t unused)
 {
-    uint32_t            index;
-    nxt_int_t           ret;
-    njs_array_t         *array;
-    njs_object_t        *descriptor;
-    njs_object_prop_t   *pr, *prop, array_prop;
-    const njs_value_t   *value;
-    nxt_lvlhsh_query_t  lhq;
+    double                num;
+    uint32_t              index;
+    nxt_int_t             ret;
+    njs_array_t           *array;
+    njs_object_t          *descriptor;
+    njs_object_prop_t     *pr, *prop, array_prop;
+    const njs_value_t     *value;
+    nxt_lvlhsh_query_t    lhq;
+    njs_property_query_t  pq;
 
     if (nargs < 2 || njs_is_null_or_void(&args[1])) {
         njs_type_error(vm, "cannot convert null argument to object", NULL);
@@ -626,9 +742,13 @@ njs_object_get_own_property_descriptor(n
 
     if (njs_is_array(&args[1])) {
         array = args[1].data.u.array;
-        index = njs_string_to_index(&args[2]);
+        num = njs_string_to_index(&args[2]);
+        index = num;
 
-        if (index < array->length && njs_is_valid(&array->start[index])) {
+        if ((double) index == num
+            && index < array->length
+            && njs_is_valid(&array->start[index]))
+        {
             prop = &array_prop;
 
             array_prop.name = args[2];
@@ -640,20 +760,21 @@ njs_object_get_own_property_descriptor(n
         }
     }
 
-    lhq.proto = &njs_object_hash_proto;
+    if (prop == NULL) {
+        pq.query = NJS_PROPERTY_QUERY_GET;
+        njs_string_get(&args[2], &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;
 
-    if (prop == NULL) {
-        njs_string_get(&args[2], &lhq.key);
-        lhq.key_hash = nxt_djb_hash(lhq.key.start, lhq.key.length);
-
-        ret = nxt_lvlhsh_find(&args[1].data.u.object->hash, &lhq);
+        ret = njs_object_property_query(vm, &pq, &args[1],
+                                        args[1].data.u.object);
 
         if (ret != NXT_OK) {
             vm->retval = njs_string_void;
             return NXT_OK;
         }
 
-        prop = lhq.value;
+        prop = pq.lhq.value;
     }
 
     descriptor = njs_object_alloc(vm);
@@ -663,6 +784,7 @@ njs_object_get_own_property_descriptor(n
 
     lhq.replace = 0;
     lhq.pool = vm->mem_cache_pool;
+    lhq.proto = &njs_object_hash_proto;
 
     lhq.key = nxt_string_value("value");
     lhq.key_hash = NJS_VALUE_HASH;
diff --git a/njs/njs_object.h b/njs/njs_object.h
--- a/njs/njs_object.h
+++ b/njs/njs_object.h
@@ -30,6 +30,20 @@ typedef struct {
 } njs_object_prop_t;
 
 
+typedef struct {
+    nxt_lvlhsh_query_t             lhq;
+
+    /* scratch is used to get the value of an NJS_PROPERTY_HANDLER property. */
+    njs_object_prop_t              scratch;
+
+    njs_value_t                    value;
+    njs_object_t                   *prototype;
+    uint8_t                        query;
+    uint8_t                        shared;
+} njs_property_query_t;
+
+
+
 struct njs_object_init_s {
     nxt_str_t                   name;
     const njs_object_prop_t     *properties;
@@ -44,6 +58,8 @@ njs_object_t *njs_object_value_alloc(njs
 njs_array_t *njs_object_keys_array(njs_vm_t *vm, njs_value_t *object);
 njs_object_prop_t *njs_object_property(njs_vm_t *vm, njs_object_t *obj,
     nxt_lvlhsh_query_t *lhq);
+njs_ret_t njs_object_property_query(njs_vm_t *vm, njs_property_query_t *pq,
+    njs_value_t *value, njs_object_t *object);
 nxt_int_t njs_object_hash_create(njs_vm_t *vm, nxt_lvlhsh_t *hash,
     const njs_object_prop_t *prop, nxt_uint_t n);
 njs_ret_t njs_object_constructor(njs_vm_t *vm, njs_value_t *args,
diff --git a/njs/njs_vm.c b/njs/njs_vm.c
--- a/njs/njs_vm.c
+++ b/njs/njs_vm.c
@@ -33,36 +33,6 @@
 #include <stdio.h>
 
 
-/* The values must be greater than NXT_OK. */
-#define NJS_PRIMITIVE_VALUE        1
-#define NJS_STRING_VALUE           2
-#define NJS_ARRAY_VALUE            3
-#define NJS_EXTERNAL_VALUE         4
-
-
-/*
- * NJS_PROPERTY_QUERY_GET must be less or equal to NJS_PROPERTY_QUERY_IN,
- * NJS_PROPERTY_QUERY_SET and NJS_PROPERTY_QUERY_DELETE must be greater
- * than NJS_PROPERTY_QUERY_IN.
- */
-#define NJS_PROPERTY_QUERY_GET     0
-#define NJS_PROPERTY_QUERY_IN      1
-#define NJS_PROPERTY_QUERY_SET     2
-#define NJS_PROPERTY_QUERY_DELETE  3
-
-
-typedef struct {
-    nxt_lvlhsh_query_t             lhq;
-
-    /* scratch is used to get the value of an NJS_PROPERTY_HANDLER property. */
-    njs_object_prop_t              scratch;
-
-    njs_value_t                    value;
-    njs_object_t                   *prototype;
-    uint8_t                        query;
-    uint8_t                        shared;
-} njs_property_query_t;
-
 
 struct njs_property_next_s {
     int32_t                        index;
@@ -80,10 +50,6 @@ static nxt_noinline njs_ret_t njs_proper
     njs_property_query_t *pq, njs_value_t *object, njs_value_t *property);
 static njs_ret_t njs_array_property_query(njs_vm_t *vm,
     njs_property_query_t *pq, njs_value_t *object, uint32_t index);
-static njs_ret_t njs_object_property_query(njs_vm_t *vm,
-    njs_property_query_t *pq, njs_value_t *value, njs_object_t *object);
-static njs_ret_t njs_object_query_prop_handler(njs_property_query_t *pq,
-    njs_object_t *object);
 static njs_ret_t njs_method_private_copy(njs_vm_t *vm,
     njs_property_query_t *pq);
 static nxt_noinline njs_ret_t njs_values_equal(const njs_value_t *val1,
@@ -1158,116 +1124,6 @@ njs_array_property_query(njs_vm_t *vm, n
 
 
 static njs_ret_t
-njs_object_property_query(njs_vm_t *vm, njs_property_query_t *pq,
-    njs_value_t *value, njs_object_t *object)
-{
-    njs_ret_t          ret;
-    njs_object_prop_t  *prop;
-
-    pq->lhq.proto = &njs_object_hash_proto;
-
-    if (pq->query == NJS_PROPERTY_QUERY_SET) {
-        ret = njs_object_query_prop_handler(pq, object);
-        if (ret == NXT_OK) {
-            return ret;
-        }
-    }
-
-    do {
-        pq->prototype = object;
-
-        ret = nxt_lvlhsh_find(&object->hash, &pq->lhq);
-
-        if (ret == NXT_OK) {
-            prop = pq->lhq.value;
-
-            if (prop->type != NJS_WHITEOUT) {
-                pq->shared = 0;
-
-                return ret;
-            }
-
-            goto next;
-        }
-
-        if (pq->query > NJS_PROPERTY_QUERY_IN) {
-            /* NXT_DECLINED */
-            return ret;
-        }
-
-        ret = nxt_lvlhsh_find(&object->shared_hash, &pq->lhq);
-
-        if (ret == NXT_OK) {
-            pq->shared = 1;
-
-            if (pq->query == NJS_PROPERTY_QUERY_GET) {
-                prop = pq->lhq.value;
-
-                if (prop->type == NJS_PROPERTY_HANDLER) {
-                    pq->scratch = *prop;
-                    prop = &pq->scratch;
-                    ret = prop->value.data.u.prop_handler(vm, value, NULL,
-                                                          &prop->value);
-
-                    if (nxt_fast_path(ret == NXT_OK)) {
-                        prop->type = NJS_PROPERTY;
-                        pq->lhq.value = prop;
-                    }
-                }
-            }
-
-            return ret;
-        }
-
-        if (pq->query > NJS_PROPERTY_QUERY_IN) {
-            /* NXT_DECLINED */
-            return ret;
-        }
-
-    next:
-
-        object = object->__proto__;
-
-    } while (object != NULL);
-
-    if (njs_is_string(value)) {
-        return NJS_STRING_VALUE;
-    }
-
-    /* NXT_DECLINED */
-
-    return ret;
-}
-
-
-static njs_ret_t
-njs_object_query_prop_handler(njs_property_query_t *pq, njs_object_t *object)
-{
-    njs_ret_t          ret;
-    njs_object_prop_t  *prop;
-
-    do {
-        pq->prototype = object;
-
-        ret = nxt_lvlhsh_find(&object->shared_hash, &pq->lhq);
-
-        if (ret == NXT_OK) {
-            prop = pq->lhq.value;
-
-            if (prop->type == NJS_PROPERTY_HANDLER) {
-                return NXT_OK;
-            }
-        }
-
-        object = object->__proto__;
-
-    } while (object != NULL);
-
-    return NXT_DECLINED;
-}
-
-
-static njs_ret_t
 njs_method_private_copy(njs_vm_t *vm, njs_property_query_t *pq)
 {
     njs_function_t     *function;
diff --git a/njs/njs_vm.h b/njs/njs_vm.h
--- a/njs/njs_vm.h
+++ b/njs/njs_vm.h
@@ -49,6 +49,24 @@
 #define NJS_APPLIED              NXT_DONE
 
 
+/* The values must be greater than NXT_OK. */
+#define NJS_PRIMITIVE_VALUE        1
+#define NJS_STRING_VALUE           2
+#define NJS_ARRAY_VALUE            3
+#define NJS_EXTERNAL_VALUE         4
+
+
+/*
+ * NJS_PROPERTY_QUERY_GET must be less or equal to NJS_PROPERTY_QUERY_IN,
+ * NJS_PROPERTY_QUERY_SET and NJS_PROPERTY_QUERY_DELETE must be greater
+ * than NJS_PROPERTY_QUERY_IN.
+ */
+#define NJS_PROPERTY_QUERY_GET     0
+#define NJS_PROPERTY_QUERY_IN      1
+#define NJS_PROPERTY_QUERY_SET     2
+#define NJS_PROPERTY_QUERY_DELETE  3
+
+
 /*
  * The order of the enum is used in njs_vmcode_typeof()
  * and njs_object_prototype_to_string().
diff --git a/njs/test/njs_unit_test.c b/njs/test/njs_unit_test.c
--- a/njs/test/njs_unit_test.c
+++ b/njs/test/njs_unit_test.c
@@ -6587,6 +6587,12 @@ static njs_unit_test_t  njs_test[] =
     { nxt_string("Object.getOwnPropertyDescriptor([3,4], 1).value"),
       nxt_string("4") },
 
+    { nxt_string("Object.getOwnPropertyDescriptor([], 'length').value"),
+      nxt_string("0") },
+
+    { nxt_string("Object.getOwnPropertyDescriptor([3,4], 'length').value"),
+      nxt_string("2") },
+
     { nxt_string("Object.getOwnPropertyDescriptor([3,4], '3')"),
       nxt_string("undefined") },
 

[drsm]

>> JSON.stringify(Object.getOwnPropertyDescriptor([], 'length'))
{"value":0,"configurable":"false","enumerable":"false","writable":"false"}
>> JSON.stringify(Object.getOwnPropertyDescriptor([1,2,3], 'length'))
{"value":3,"configurable":"false","enumerable":"false","writable":"false"}

I think there should be writable: true.