[njs] Fixed Object.defineProperty() with fast arrays.
Vadim Zhestikov
v.zhestikov at f5.com
Wed Sep 7 02:17:39 UTC 2022
details: https://hg.nginx.org/njs/rev/a2db32044812
branches:
changeset: 1949:a2db32044812
user: Vadim Zhestikov <v.zhestikov at f5.com>
date: Tue Sep 06 18:44:47 2022 -0700
description:
Fixed Object.defineProperty() with fast arrays.
Previously, Object.defineProperty() failed to properly set attributes for properties of fast array.
diffstat:
src/njs_array.c | 29 ++++++----
src/njs_array.h | 2 +-
src/njs_json.c | 123 ++++++++++++++++++++++++++--------------------
src/njs_object.c | 3 +-
src/njs_object_prop.c | 40 ++++++++++++--
src/njs_value.c | 11 +--
src/test/njs_unit_test.c | 28 +++++++---
7 files changed, 148 insertions(+), 88 deletions(-)
diffs (464 lines):
diff -r d40481363bdf -r a2db32044812 src/njs_array.c
--- a/src/njs_array.c Tue Sep 06 11:13:31 2022 -0700
+++ b/src/njs_array.c Tue Sep 06 18:44:47 2022 -0700
@@ -100,7 +100,7 @@ njs_array_alloc(njs_vm_t *vm, njs_bool_t
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
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,
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_v
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_v
}
}
- 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 *fi
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 -r d40481363bdf -r a2db32044812 src/njs_array.h
--- a/src/njs_array.h Tue Sep 06 11:13:31 2022 -0700
+++ b/src/njs_array.h Tue Sep 06 18:44:47 2022 -0700
@@ -25,7 +25,7 @@ void njs_array_destroy(njs_vm_t *vm, njs
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 -r d40481363bdf -r a2db32044812 src/njs_json.c
--- a/src/njs_json.c Tue Sep 06 11:13:31 2022 -0700
+++ b/src/njs_json.c Tue Sep 06 18:44:47 2022 -0700
@@ -989,8 +989,8 @@ njs_json_push_stringify_state(njs_vm_t *
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 *
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 *str
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 *str
} 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 *str
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);
+ }
}
- njs_chb_sprintf(chain, 64, "<%L empty items>", diff - 1);
+ if (diff - 1 == 1) {
+ njs_chb_sprintf(chain, 64, "<empty>");
+
+ } else {
+ 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_
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_
continue;
}
- if (state->fast_array) {
- if (state->written) {
- njs_chb_append_literal(&chain, ",");
- njs_json_stringify_indent(stringify, &chain, 1);
- }
-
- state->written = 1;
-
- 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) {
+ njs_property_query_init(&pq, NJS_PROPERTY_QUERY_GET, 0);
+
+ key = &state->keys->start[state->index++];
+
+ 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;
-
- if (prop->type == NJS_WHITEOUT || !prop->enumerable) {
+ }
+
+ state->key = key;
+
+ ret = njs_property_query(vm, &pq, &state->value, key);
+ if (njs_slow_path(ret != NJS_OK)) {
+ if (ret == NJS_DECLINED) {
+ continue;
+ }
+
+ goto exception;
+ }
+
+ prop = pq.lhq.value;
+
+ if (prop->type == NJS_WHITEOUT || !prop->enumerable) {
+ if (!state->array) {
continue;
}
-
- if (state->written) {
- njs_chb_append_literal(&chain, ",");
- njs_json_stringify_indent(stringify, &chain, 1);
+ }
+
+ if (state->written) {
+ njs_chb_append_literal(&chain, ",");
+ njs_json_stringify_indent(stringify, &chain, 1);
+ }
+
+ state->written = 1;
+
+ 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, " ");
}
-
- state->written = 1;
-
- 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 -r d40481363bdf -r a2db32044812 src/njs_object.c
--- a/src/njs_object.c Tue Sep 06 11:13:31 2022 -0700
+++ b/src/njs_object.c Tue Sep 06 18:44:47 2022 -0700
@@ -1586,11 +1586,10 @@ njs_object_set_integrity_level(njs_vm_t
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 -r d40481363bdf -r a2db32044812 src/njs_object_prop.c
--- a/src/njs_object_prop.c Tue Sep 06 11:13:31 2022 -0700
+++ b/src/njs_object_prop.c Tue Sep 06 18:44:47 2022 -0700
@@ -153,6 +153,21 @@ njs_object_prop_define(njs_vm_t *vm, njs
}
}
+ 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 -r d40481363bdf -r a2db32044812 src/njs_value.c
--- a/src/njs_value.c Tue Sep 06 11:13:31 2022 -0700
+++ b/src/njs_value.c Tue Sep 06 18:44:47 2022 -0700
@@ -790,7 +790,7 @@ njs_array_property_query(njs_vm_t *vm, n
}
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 -r d40481363bdf -r a2db32044812 src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c Tue Sep 06 11:13:31 2022 -0700
+++ b/src/test/njs_unit_test.c Tue Sep 06 18:44:47 2022 -0700
@@ -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") },
More information about the nginx-devel
mailing list