details: https://hg.nginx.org/njs/rev/a2db32044812 branches: changeset: 1949:a2db32044812 user: Vadim Zhestikov <v.zhesti...@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") }, _______________________________________________ nginx-devel mailing list -- nginx-devel@nginx.org To unsubscribe send an email to nginx-devel-le...@nginx.org