[njs] Handling non-object values in Object.keys().

Dmitry Volyntsev xeioex at nginx.com
Thu Nov 15 17:32:03 UTC 2018


details:   http://hg.nginx.org/njs/rev/76e139b439ad
branches:  
changeset: 658:76e139b439ad
user:      Dmitry Volyntsev <xeioex at nginx.com>
date:      Thu Nov 15 20:31:35 2018 +0300
description:
Handling non-object values in Object.keys().

This fixes #54 issue on Github.

diffstat:

 njs/njs_object.c         |  133 +++++++++++++++++++++++++++++++---------------
 njs/njs_object.h         |    2 +-
 njs/test/njs_unit_test.c |   21 ++++++-
 3 files changed, 108 insertions(+), 48 deletions(-)

diffs (226 lines):

diff -r 5f0090c02589 -r 76e139b439ad 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
@@ -865,7 +865,7 @@ njs_object_keys(njs_vm_t *vm, njs_value_
 
     value = njs_arg(args, nargs, 1);
 
-    if (!njs_is_object(value)) {
+    if (njs_is_null_or_void(value)) {
         njs_type_error(vm, "cannot convert %s argument to object",
                        njs_type_string(value->type));
 
@@ -874,7 +874,6 @@ njs_object_keys(njs_vm_t *vm, njs_value_
 
     keys = njs_object_keys_array(vm, value);
     if (keys == NULL) {
-        njs_memory_error(vm);
         return NXT_ERROR;
     }
 
@@ -887,43 +886,69 @@ njs_object_keys(njs_vm_t *vm, njs_value_
 
 
 njs_array_t *
-njs_object_keys_array(njs_vm_t *vm, const njs_value_t *object)
+njs_object_keys_array(njs_vm_t *vm, const njs_value_t *value)
 {
-    uint32_t           i, n, keys_length, array_length;
-    njs_value_t        *value;
+    uint32_t           i, n, length, keys_length;
+    njs_value_t        *string;
     njs_array_t        *keys, *array;
     nxt_lvlhsh_t       *hash;
     njs_object_prop_t  *prop;
+    njs_string_prop_t  string_prop;
     nxt_lvlhsh_each_t  lhe;
 
     array = NULL;
+    length = 0;
     keys_length = 0;
-    array_length = 0;
-
-    if (njs_is_array(object)) {
-        array = object->data.u.array;
-        array_length = array->length;
-
-        for (i = 0; i < array_length; i++) {
+
+    switch (value->type) {
+    case NJS_ARRAY:
+        array = value->data.u.array;
+        length = array->length;
+
+        for (i = 0; i < length; i++) {
             if (njs_is_valid(&array->start[i])) {
                 keys_length++;
             }
         }
+
+        break;
+
+    case NJS_STRING:
+    case NJS_OBJECT_STRING:
+        if (value->type == NJS_OBJECT_STRING) {
+            string = &value->data.u.object_value->value;
+
+        } else {
+            string = (njs_value_t *) value;
+        }
+
+        length = njs_string_prop(&string_prop, string);
+        keys_length += length;
+        break;
+
+    default:
+        break;
     }
 
-    nxt_lvlhsh_each_init(&lhe, &njs_object_hash_proto);
-
-    hash = &object->data.u.object->hash;
-
-    for ( ;; ) {
-        prop = nxt_lvlhsh_each(hash, &lhe);
-
-        if (prop == NULL) {
-            break;
-        }
-
-        if (prop->type != NJS_WHITEOUT && prop->enumerable) {
-            keys_length++;
+    if (nxt_fast_path(njs_is_object(value))) {
+        nxt_lvlhsh_each_init(&lhe, &njs_object_hash_proto);
+        hash = &value->data.u.object->hash;
+
+    } else {
+        hash = NULL;
+    }
+
+    if (nxt_fast_path(hash != NULL)) {
+        for ( ;; ) {
+            prop = nxt_lvlhsh_each(hash, &lhe);
+
+            if (prop == NULL) {
+                break;
+            }
+
+            if (prop->type != NJS_WHITEOUT && prop->enumerable) {
+                keys_length++;
+            }
         }
     }
 
@@ -934,28 +959,48 @@ njs_object_keys_array(njs_vm_t *vm, cons
 
     n = 0;
 
-    for (i = 0; i < array_length; i++) {
-        if (njs_is_valid(&array->start[i])) {
-            value = &keys->start[n++];
-            /*
-             * The maximum array index is 4294967294, so
-             * it can be stored as a short string inside value.
-             */
-            njs_uint32_to_string(value, i);
+    switch (value->type) {
+    case NJS_ARRAY:
+        for (i = 0; i < length; i++) {
+            if (njs_is_valid(&array->start[i])) {
+                njs_uint32_to_string(&keys->start[n++], i);
+            }
         }
+
+        break;
+
+    case NJS_STRING:
+    case NJS_OBJECT_STRING:
+        if (value->type == NJS_OBJECT_STRING) {
+            string = &value->data.u.object_value->value;
+
+        } else {
+            string = (njs_value_t *) value;
+        }
+
+        for (i = 0; i < length; i++) {
+            njs_uint32_to_string(&keys->start[n++], i);
+        }
+
+        break;
+
+    default:
+        break;
     }
 
-    nxt_lvlhsh_each_init(&lhe, &njs_object_hash_proto);
-
-    for ( ;; ) {
-        prop = nxt_lvlhsh_each(hash, &lhe);
-
-        if (prop == NULL) {
-            break;
-        }
-
-        if (prop->type != NJS_WHITEOUT && prop->enumerable) {
-            njs_string_copy(&keys->start[n++], &prop->name);
+    if (nxt_fast_path(hash != NULL)) {
+        nxt_lvlhsh_each_init(&lhe, &njs_object_hash_proto);
+
+        for ( ;; ) {
+            prop = nxt_lvlhsh_each(hash, &lhe);
+
+            if (prop == NULL) {
+                break;
+            }
+
+            if (prop->type != NJS_WHITEOUT && prop->enumerable) {
+                njs_string_copy(&keys->start[n++], &prop->name);
+            }
         }
     }
 
diff -r 5f0090c02589 -r 76e139b439ad njs/njs_object.h
--- a/njs/njs_object.h	Thu Nov 15 20:31:35 2018 +0300
+++ b/njs/njs_object.h	Thu Nov 15 20:31:35 2018 +0300
@@ -79,7 +79,7 @@ njs_object_t *njs_object_alloc(njs_vm_t 
 njs_object_t *njs_object_value_copy(njs_vm_t *vm, njs_value_t *value);
 njs_object_t *njs_object_value_alloc(njs_vm_t *vm, const njs_value_t *value,
     nxt_uint_t type);
-njs_array_t *njs_object_keys_array(njs_vm_t *vm, const njs_value_t *object);
+njs_array_t *njs_object_keys_array(njs_vm_t *vm, const njs_value_t *value);
 njs_ret_t njs_value_property(njs_vm_t *vm, njs_value_t *value,
     const njs_value_t *property, njs_value_t *retval);
 njs_object_prop_t *njs_object_property(njs_vm_t *vm, const njs_object_t *obj,
diff -r 5f0090c02589 -r 76e139b439ad 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
@@ -7236,11 +7236,26 @@ static njs_unit_test_t  njs_test[] =
     { nxt_string("Object.keys()"),
       nxt_string("TypeError: cannot convert undefined argument to object") },
 
-    { nxt_string("Object.keys('a')"),
-      nxt_string("TypeError: cannot convert string argument to object") },
+    { nxt_string("Object.keys('αβZ')"),
+      nxt_string("0,1,2") },
+
+    { nxt_string("Object.keys(new String('αβZ'))"),
+      nxt_string("0,1,2") },
+
+    { nxt_string("var s = new String('αβZ'); s.a = 1; Object.keys(s)"),
+      nxt_string("0,1,2,a") },
+
+    { nxt_string("var r = new RegExp('αbc'); r.a = 1; Object.keys(r)"),
+      nxt_string("a") },
+
+    { nxt_string("Object.keys(Object.create(new String('abc')))"),
+      nxt_string("") },
 
     { nxt_string("Object.keys(1)"),
-      nxt_string("TypeError: cannot convert number argument to object") },
+      nxt_string("") },
+
+    { nxt_string("Object.keys(true)"),
+      nxt_string("") },
 
     { nxt_string("var o = {}; Object.defineProperty(o, 'a', {}); o.a"),
       nxt_string("undefined") },


More information about the nginx-devel mailing list