Fixed Object.defineProperty() with fast arrays.

Previously, Object.defineProperty() failed to properly set attributes for properties of fast array.

diff --git a/src/njs_array.c b/src/njs_array.c
index a973f30ab9b54cae646b44f5c0dcbf82735a480d..c72a4630266e66dd8a277cda890f728ed642514f 100644 (file)
--- a/src/njs_array.c
+++ b/src/njs_array.c
@@ -100,7 +100,7 @@ njs_array_alloc(njs_vm_t *vm, njs_bool_t flat, uint64_t length, uint32_t spare)
         array->length = 0;
 
         njs_set_array(&value, array);
-        ret = njs_array_length_redefine(vm, &value, length);
+        ret = njs_array_length_redefine(vm, &value, length, 1);
         if (njs_slow_path(ret != NJS_OK)) {
             return NULL;
         }
@@ -173,7 +173,7 @@ njs_array_convert_to_slow_array(njs_vm_t *vm, njs_array_t *array)
 
 
 njs_int_t
-njs_array_length_redefine(njs_vm_t *vm, njs_value_t *value, uint32_t length)
+njs_array_length_redefine(njs_vm_t *vm, njs_value_t *value, uint32_t length, int writable)
 {
     njs_object_prop_t  *prop;
 
@@ -192,6 +192,7 @@ njs_array_length_redefine(njs_vm_t *vm, njs_value_t *value, uint32_t length)
         return NJS_ERROR;
     }
 
+    prop->writable = writable;
     prop->enumerable = 0;
     prop->configurable = 0;
 
@@ -209,12 +210,7 @@ njs_array_length_set(njs_vm_t *vm, njs_value_t *value,
     int64_t       prev_length;
     uint32_t      i, length;
     njs_int_t     ret;
-    njs_array_t   *array, *keys;
-
-    array = njs_object_proto_lookup(njs_object(value), NJS_ARRAY, njs_array_t);
-    if (njs_slow_path(array == NULL)) {
-        return NJS_DECLINED;
-    }
+    njs_array_t   *keys;
 
     ret = njs_value_to_number(vm, setval, &num);
     if (njs_slow_path(ret != NJS_OK)) {
@@ -256,7 +252,7 @@ njs_array_length_set(njs_vm_t *vm, njs_value_t *value,
         }
     }
 
-    ret = njs_array_length_redefine(vm, value, length);
+    ret = njs_array_length_redefine(vm, value, length, prev->writable);
     if (njs_slow_path(ret != NJS_OK)) {
         return ret;
     }
@@ -1604,7 +1600,7 @@ static int
 njs_array_indices_handler(const void *first, const void *second, void *ctx)
 {
     double             num1, num2;
-    int64_t            diff;
+    int64_t            diff, cmp_res;
     njs_str_t          str1, str2;
     const njs_value_t  *val1, *val2;
 
@@ -1635,8 +1631,19 @@ njs_array_indices_handler(const void *first, const void *second, void *ctx)
     njs_string_get(val1, &str1);
     njs_string_get(val2, &str2);
 
-    return strncmp((const char *) str1.start, (const char *) str2.start,
+    cmp_res =  strncmp((const char *) str1.start, (const char *) str2.start,
                    njs_min(str1.length, str2.length));
+    if (cmp_res == 0) {
+        if (str1.length < str2.length) {
+            return -1;
+        } else if (str1.length > str2.length) {
+            return 1;
+        } else {
+            return 0;
+        }
+    }
+
+    return cmp_res;
 }
 
 

diff --git a/src/njs_array.h b/src/njs_array.h
index 5dca7ed012a143731f57c8d9e3244aea468c6b11..828b5de2d548b731060724910ee1f4945e2f9860 100644 (file)
--- a/src/njs_array.h
+++ b/src/njs_array.h
@@ -25,7 +25,7 @@ void njs_array_destroy(njs_vm_t *vm, njs_array_t *array);
 njs_int_t njs_array_add(njs_vm_t *vm, njs_array_t *array, njs_value_t *value);
 njs_int_t njs_array_convert_to_slow_array(njs_vm_t *vm, njs_array_t *array);
 njs_int_t njs_array_length_redefine(njs_vm_t *vm, njs_value_t *value,
-    uint32_t length);
+    uint32_t length, int writable);
 njs_int_t njs_array_length_set(njs_vm_t *vm, njs_value_t *value,
     njs_object_prop_t *prev, njs_value_t *setval);
 njs_array_t *njs_array_keys(njs_vm_t *vm, njs_value_t *array, njs_bool_t all);

diff --git a/src/njs_json.c b/src/njs_json.c
index 9b74365ead96eda62539b94b7136f9e36e468dbb..803d3155299d22c7e567b77ed3f8c8056d941dfd 100644 (file)
--- a/src/njs_json.c
+++ b/src/njs_json.c
@@ -989,8 +989,8 @@ njs_json_push_stringify_state(njs_vm_t *vm, njs_json_stringify_t *stringify,
     if (njs_is_array(&stringify->replacer)) {
         state->keys = njs_array(&stringify->replacer);
 
-    } else if (state->array && !state->fast_array) {
-        state->keys = njs_array_keys(vm, value, 0);
+    } else if (state->array) {
+        state->keys = njs_array_keys(vm, value, 1);
         if (njs_slow_path(state->keys == NULL)) {
             return NULL;
         }
@@ -1727,7 +1727,7 @@ njs_dump_terminal(njs_json_stringify_t *stringify, njs_chb_t *chain,
         break;
 
     case NJS_INVALID:
-        njs_chb_append_literal(chain, "<empty>");
+        /* Fall through. */
         break;
 
     case NJS_OBJECT_VALUE:
@@ -1916,7 +1916,7 @@ njs_dump_empty(njs_json_stringify_t *stringify, njs_json_state_t *state,
     double   key, prev;
     int64_t  diff;
 
-    if (!state->array || state->fast_array) {
+    if (!state->array) {
         return 0;
     }
 
@@ -1926,7 +1926,12 @@ njs_dump_empty(njs_json_stringify_t *stringify, njs_json_state_t *state,
 
     } else {
         key = state->length;
-        prev = (state->index > 0) ? njs_key_to_index(state->key) : -1;
+        if (state->key == NULL) {
+            prev = -1;
+
+        } else {
+            prev = (state->index > 0) ? njs_key_to_index(state->key) : -1;
+        }
     }
 
     if (isnan(key)) {
@@ -1937,11 +1942,19 @@ njs_dump_empty(njs_json_stringify_t *stringify, njs_json_state_t *state,
 
     if (diff > 1) {
         if (sep_position == 0 && state->keys->length) {
-            njs_chb_append_literal(chain, ",");
-            njs_json_stringify_indent(stringify, chain, 1);
+            if (prev != -1) {
+                njs_chb_append_literal(chain, ",");
+                njs_json_stringify_indent(stringify, chain, 1);
+            }
+        }
+
+        if (diff - 1 == 1) {
+            njs_chb_sprintf(chain, 64, "<empty>");
+
+        } else {
+            njs_chb_sprintf(chain, 64, "<%L empty items>", diff - 1);
         }
 
-        njs_chb_sprintf(chain, 64, "<%L empty items>", diff - 1);
         state->written = 1;
 
         if (sep_position == 1 && state->keys->length) {
@@ -2019,7 +2032,7 @@ njs_vm_value_dump(njs_vm_t *vm, njs_str_t *retval, njs_value_t *value,
             njs_json_stringify_indent(stringify, &chain, 1);
         }
 
-        if (njs_json_stringify_done(state, state->fast_array)) {
+        if (njs_json_stringify_done(state, 0)) {
             njs_dump_empty(stringify, state, &chain, 0);
 
             njs_json_stringify_indent(stringify, &chain, 0);
@@ -2033,57 +2046,59 @@ njs_vm_value_dump(njs_vm_t *vm, njs_str_t *retval, njs_value_t *value,
             continue;
         }
 
-        if (state->fast_array) {
-            if (state->written) {
-                njs_chb_append_literal(&chain, ",");
-                njs_json_stringify_indent(stringify, &chain, 1);
-            }
+        njs_property_query_init(&pq, NJS_PROPERTY_QUERY_GET, 0);
 
-            state->written = 1;
+        key = &state->keys->start[state->index++];
 
-            val = &njs_array_start(&state->value)[state->index++];
-
-        } else {
-            njs_property_query_init(&pq, NJS_PROPERTY_QUERY_GET, 0);
-
-            key = &state->keys->start[state->index++];
-            state->key = key;
-
-            ret = njs_property_query(vm, &pq, &state->value, key);
-            if (njs_slow_path(ret != NJS_OK)) {
-                if (ret == NJS_DECLINED) {
+        if(state->array) {
+            if (key->type == NJS_STRING) {
+                njs_string_get(key, &str);
+                if (str.length == 6 && memcmp(str.start, "length", 6) == 0) {
                     continue;
                 }
-
-                goto exception;
             }
+        }
 
-            prop = pq.lhq.value;
+        state->key = key;
 
-            if (prop->type == NJS_WHITEOUT || !prop->enumerable) {
+        ret = njs_property_query(vm, &pq, &state->value, key);
+        if (njs_slow_path(ret != NJS_OK)) {
+            if (ret == NJS_DECLINED) {
                 continue;
             }
 
-            if (state->written) {
-                njs_chb_append_literal(&chain, ",");
-                njs_json_stringify_indent(stringify, &chain, 1);
+            goto exception;
+        }
+
+        prop = pq.lhq.value;
+
+        if (prop->type == NJS_WHITEOUT || !prop->enumerable) {
+            if (!state->array) {
+                continue;
             }
+        }
 
-            state->written = 1;
+        if (state->written) {
+            njs_chb_append_literal(&chain, ",");
+            njs_json_stringify_indent(stringify, &chain, 1);
+        }
 
-            njs_dump_empty(stringify, state, &chain, 1);
+        state->written = 1;
 
-            if (!state->array || isnan(njs_key_to_index(key))) {
-                njs_key_string_get(vm, key, &pq.lhq.key);
-                njs_chb_append(&chain, pq.lhq.key.start, pq.lhq.key.length);
-                njs_chb_append_literal(&chain, ":");
-                if (stringify->space.length != 0) {
-                    njs_chb_append_literal(&chain, " ");
-                }
+        njs_dump_empty(stringify, state, &chain, 1);
+
+        if (!state->array || isnan(njs_key_to_index(key))) {
+            njs_key_string_get(vm, key, &pq.lhq.key);
+            njs_chb_append(&chain, pq.lhq.key.start, pq.lhq.key.length);
+            njs_chb_append_literal(&chain, ":");
+            if (stringify->space.length != 0) {
+                njs_chb_append_literal(&chain, " ");
             }
+        }
 
-            val = &prop->value;
+        val = &prop->value;
 
+        if (!state->fast_array) {
             if (prop->type == NJS_PROPERTY_HANDLER) {
                 pq.scratch = *prop;
                 prop = &pq.scratch;

diff --git a/src/njs_object.c b/src/njs_object.c
index 4c01f0ba7c529b9742ed76f93b1b689778657821..31bd2b1967a0675ae27704a442faccd60dd908ee 100644 (file)
--- a/src/njs_object.c
+++ b/src/njs_object.c
@@ -1586,11 +1586,10 @@ njs_object_set_integrity_level(njs_vm_t *vm, njs_value_t *args,
             return ret;
         }
 
-        ret = njs_array_length_redefine(vm, value, length);
+        ret = njs_array_length_redefine(vm, value, length, 1);
         if (njs_slow_path(ret != NJS_OK)) {
             return ret;
         }
-
     }
 
     object = njs_object(value);

diff --git a/src/njs_object_prop.c b/src/njs_object_prop.c
index 74e63fc111d41ea61a6cef1ac06703831c728eae..8e3cdf59e06e45047718a9f334f784e019bb59a9 100644 (file)
--- a/src/njs_object_prop.c
+++ b/src/njs_object_prop.c
@@ -153,6 +153,21 @@ njs_object_prop_define(njs_vm_t *vm, njs_value_t *object,
         }
     }
 
+    if (njs_slow_path(njs_is_fast_array(object))) {
+        array = njs_array(object);
+        length = array->length;
+
+        ret = njs_array_convert_to_slow_array(vm, array);
+        if (ret != NJS_OK) {
+            return NJS_ERROR;
+        }
+
+        ret = njs_array_length_redefine(vm, object, length, 1);
+        if (njs_slow_path(ret != NJS_OK)) {
+            return ret;
+        }
+    }
+
 again:
 
     njs_property_query_init(&pq, NJS_PROPERTY_QUERY_SET, 1);
@@ -290,7 +305,7 @@ set_prop:
                 return ret;
             }
 
-            ret = njs_array_length_redefine(vm, object, length);
+            ret = njs_array_length_redefine(vm, object, length, 1);
             if (njs_slow_path(ret != NJS_OK)) {
                 return ret;
             }
@@ -440,11 +455,24 @@ done:
             }
 
         } else {
-            if (njs_slow_path(pq.lhq.key_hash == NJS_LENGTH_HASH)) {
-                if (njs_strstr_eq(&pq.lhq.key, &length_key)) {
-                    ret = njs_array_length_set(vm, object, prev, &prop->value);
-                    if (ret != NJS_DECLINED) {
-                        return ret;
+            if (njs_is_array(object)) {
+                if (njs_slow_path(pq.lhq.key_hash == NJS_LENGTH_HASH)) {
+
+                    if (njs_strstr_eq(&pq.lhq.key, &length_key)) {
+
+                        if (prev->configurable != 1 &&
+                            prev->writable != 1 &&
+                            !njs_values_strict_equal(&prev->value, &prop->value))
+                        {
+                            njs_type_error(vm, "Cannot redefine property: \"length\"");
+                            return NJS_ERROR;
+                        }
+
+                        if (prop->writable != NJS_ATTRIBUTE_UNSET) {
+                            prev->writable = prop->writable;
+                        }
+
+                        return njs_array_length_set(vm, object, prev, &prop->value);
                     }
                 }
             }

diff --git a/src/njs_value.c b/src/njs_value.c
index f0f3651e3963ef9d4f8126149daa408c0b0a30fe..1c583aff8a0ebeda65d5ee21f3d03ee1415a78cc 100644 (file)
--- a/src/njs_value.c
+++ b/src/njs_value.c
@@ -790,7 +790,7 @@ njs_array_property_query(njs_vm_t *vm, njs_property_query_t *pq,
         }
 
         if ((index + 1) > length) {
-            ret = njs_array_length_redefine(vm, &value, index + 1);
+            ret = njs_array_length_redefine(vm, &value, index + 1, 1);
             if (njs_slow_path(ret != NJS_OK)) {
                 return ret;
             }
@@ -1223,11 +1223,10 @@ slow_path:
         if (pq.own) {
             switch (prop->type) {
             case NJS_PROPERTY:
-                if (njs_slow_path(pq.lhq.key_hash == NJS_LENGTH_HASH)) {
-                    if (njs_strstr_eq(&pq.lhq.key, &length_key)) {
-                        ret = njs_array_length_set(vm, value, prop, setval);
-                        if (ret != NJS_DECLINED) {
-                            return ret;
+                if (njs_is_array(value)) {
+                    if (njs_slow_path(pq.lhq.key_hash == NJS_LENGTH_HASH)) {
+                        if (njs_strstr_eq(&pq.lhq.key, &length_key)) {
+                            return njs_array_length_set(vm, value, prop, setval);
                         }
                     }
                 }

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index ee7bb2e140f5e7483257bb0b7d486825303e4d59..085f96b5fc60ea3178e61fe8429f5a668708f48d 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -4455,6 +4455,18 @@ static njs_unit_test_t  njs_test[] =
               "Object.defineProperty(a, 'length', {writable:true})"),
       njs_str("TypeError: Cannot redefine property: \"length\"") },
 
+    { njs_str ("var a =[0,1,2]; Object.defineProperty(a, 100, {value:100});"
+              "njs.dump(a);"),
+      njs_str("[0,1,2,<97 empty items>,100]") },
+
+    { njs_str("var a =[0,1,2]; Object.defineProperty(a, 3, {value:30});"
+              "njs.dump(Object.getOwnPropertyDescriptor(a,3));"),
+      njs_str("{value:30,writable:false,enumerable:false,configurable:false}") },
+
+    { njs_str("var a =[0,1,2]; Object.defineProperty(a, 3, {value:30});"
+              "a[3]=33;"),
+      njs_str("TypeError: Cannot assign to read-only property \"3\" of array") },
+
     { njs_str("[1, 2, 3, 4, 5].copyWithin(0, 3)"),
       njs_str("4,5,3,4,5") },
 
@@ -7032,11 +7044,11 @@ static njs_unit_test_t  njs_test[] =
 
     { njs_str("var a = [,,undefined,undefined,,undefined];"
               "a.sort(function(x, y) { return x - y }); njs.dump(a)"),
-      njs_str("[undefined,undefined,undefined,<empty>,<empty>,<empty>]") },
+      njs_str("[undefined,undefined,undefined,<3 empty items>]") },
 
     { njs_str("var a = [1,,undefined,8,undefined,,undefined,,2];"
               "a.sort(function(x, y) { return x - y }); njs.dump(a)"),
-      njs_str("[1,2,8,undefined,undefined,undefined,<empty>,<empty>,<empty>]") },
+      njs_str("[1,2,8,undefined,undefined,undefined,<3 empty items>]") },
 
     { njs_str("var a = [1,,];"
               "a.sort(function(x, y) { return x - y })"),
@@ -12473,8 +12485,8 @@ static njs_unit_test_t  njs_test[] =
       njs_str("0") },
 
     { njs_str("var x = [0, 1, 2]; x[4294967294] = 4294967294; x.length = 2;"
-              "njs.dump([x,x.length,Array.prototype,Array.prototype.length])"),
-      njs_str("[[0,1],2,[],0]") },
+              "njs.dump([x,x.length,Array.prototype.length])"),
+      njs_str("[[0,1],2,0]") },
 
 #if 0 /* TODO: length 2**53-1. */
     { njs_str("var x = Array(2**20), y = Array(2**12).fill(x);"
@@ -14713,7 +14725,7 @@ static njs_unit_test_t  njs_test[] =
       njs_str("a,b") },
 
     { njs_str("Object.getOwnPropertyNames(Object.defineProperty([], 'b', {}))"),
-      njs_str("b,length") },
+      njs_str("length,b") },
 
     { njs_str("Object.getOwnPropertyNames(Object.defineProperty(new String(), 'b', {}))"),
       njs_str("b,length") },
@@ -18014,11 +18026,11 @@ static njs_unit_test_t  njs_test[] =
 
     { njs_str("var a = ['a',,'c']; a.length = 2**31;"
               "njs.dump(a)"),
-      njs_str("['a',<1 empty items>,'c',<2147483645 empty items>]") },
+      njs_str("['a',<empty>,'c',<2147483645 empty items>]") },
 
     { njs_str("var a = [,'b','c']; a.length = 2**31;"
               "njs.dump(a)"),
-      njs_str("[<1 empty items>,'b','c',<2147483645 empty items>]") },
+      njs_str("[<empty>,'b','c',<2147483645 empty items>]") },
 
 #if (!NJS_HAVE_MEMORY_SANITIZER) /* False-positive in MSAN? */
     { njs_str("var a = []; a[2**31] = 'Z'; a[0] = 'A'; njs.dump(a)"),
@@ -18035,7 +18047,7 @@ static njs_unit_test_t  njs_test[] =
       njs_str("[1,'[Getter]',3]") },
 
     { njs_str("var a = [1,2,3];Object.defineProperty(a, '1', {enumerable:false});njs.dump(a)"),
-      njs_str("[1,<1 empty items>,3]") },
+      njs_str("[1,2,3]") },
 
     { njs_str("njs.dump(-0)"),
       njs_str("-0") },