Welcome! Log In Create A New Profile

Advanced

[njs] Fixed njs_value_index().

Alexander Borisov
September 17, 2019 02:22AM
details: https://hg.nginx.org/njs/rev/d0d4fa8918ac
branches:
changeset: 1160:d0d4fa8918ac
user: Alexander Borisov <alexander.borisov@nginx.com>
date: Tue Sep 17 09:20:24 2019 +0300
description:
Fixed njs_value_index().

Previous, there were several issues with njs_values_hash_test().
1) The function assumed string types of left and right values are identical.
(If current value is a long string it assumed the second value is also
a long string). Long vs short strings collision.
2) The function assumed if hashes are identical value length are equal.
Long vs long string collision.

The fix is to always check value sizes.

In addition, for short strings njs_value_index() calculated hash value including
padding bytes (which values are undefined). It could result in identical
short strings (but with different padding bytes) treated as different strings.

diffstat:

src/njs_string.c | 48 +++++++++++++++++++++++++-----------------------
src/test/njs_unit_test.c | 16 ++++++++++++++++
2 files changed, 41 insertions(+), 23 deletions(-)

diffs (123 lines):

diff -r 50cc68d71326 -r d0d4fa8918ac src/njs_string.c
--- a/src/njs_string.c Mon Sep 16 17:21:36 2019 +0300
+++ b/src/njs_string.c Tue Sep 17 09:20:24 2019 +0300
@@ -4723,23 +4723,22 @@ uri_error:
static njs_int_t
njs_values_hash_test(njs_lvlhsh_query_t *lhq, void *data)
{
- u_char *start;
+ njs_str_t string;
njs_value_t *value;

value = data;

- if (lhq->key.length == sizeof(njs_value_t)) {
- start = (u_char *) value;
+ if (njs_is_string(value)) {
+ njs_string_get(value, &string);

} else {
- /*
- * Only primitive values are added into values_hash.
- * If size != sizeof(njs_value_t) it is a long string.
- */
- start = value->long_string.data->start;
+ string.start = (u_char *) value;
+ string.length = sizeof(njs_value_t);
}

- if (memcmp(lhq->key.start, start, lhq->key.length) == 0) {
+ if (lhq->key.length == string.length
+ && memcmp(lhq->key.start, string.start, string.length) == 0)
+ {
return NJS_OK;
}

@@ -4768,21 +4767,28 @@ njs_value_index(njs_vm_t *vm, const njs_
u_char *start;
uint32_t value_size, size, length;
njs_int_t ret;
+ njs_str_t str;
njs_bool_t long_string;
njs_value_t *value;
njs_string_t *string;
njs_lvlhsh_t *values_hash;
njs_lvlhsh_query_t lhq;

- long_string = src->type == NJS_STRING
- && src->short_string.size == NJS_STRING_LONG;
-
- if (long_string) {
- size = src->long_string.size;
- start = src->long_string.data->start;
+ long_string = 0;
+ value_size = sizeof(njs_value_t);
+
+ if (njs_is_string(src)) {
+ njs_string_get(src, &str);
+
+ size = (uint32_t) str.length;
+ start = str.start;
+
+ if (src->short_string.size == NJS_STRING_LONG) {
+ long_string = 1;
+ }

} else {
- size = sizeof(njs_value_t);
+ size = value_size;
start = (u_char *) src;
}

@@ -4798,22 +4804,18 @@ njs_value_index(njs_vm_t *vm, const njs_
value = lhq.value;

} else {
- value_size = 0;
-
if (long_string) {
- /* Long string value is allocated together with string. */
- value_size = sizeof(njs_value_t) + sizeof(njs_string_t);
-
length = src->long_string.data->length;

if (size != length && length > NJS_STRING_MAP_STRIDE) {
size = njs_string_map_offset(size)
+ njs_string_map_size(length);
}
+
+ value_size += sizeof(njs_string_t) + size;
}

- value = njs_mp_align(vm->mem_pool, sizeof(njs_value_t),
- value_size + size);
+ value = njs_mp_align(vm->mem_pool, sizeof(njs_value_t), value_size);
if (njs_slow_path(value == NULL)) {
return NJS_INDEX_NONE;
}
diff -r 50cc68d71326 -r d0d4fa8918ac src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c Mon Sep 16 17:21:36 2019 +0300
+++ b/src/test/njs_unit_test.c Tue Sep 17 09:20:24 2019 +0300
@@ -9593,6 +9593,22 @@ static njs_unit_test_t njs_test[] =
{ njs_str("String.length"),
njs_str("1") },

+ /* values_hash long vs long string collision. */
+ { njs_str("'XXXXXXXXXXXXXXXQWEEAB' + 'XXXXXXXXXXXXXXXZHGP'"),
+ njs_str("XXXXXXXXXXXXXXXQWEEABXXXXXXXXXXXXXXXZHGP") },
+
+ /* values_hash short vs long string collision. */
+ { njs_str("'SHAAAB' + 'XXXXXXXXXXXXXXXUETBF'"),
+ njs_str("SHAAABXXXXXXXXXXXXXXXUETBF") },
+
+ /* values_hash long vs short string collision. */
+ { njs_str("'XXXXXXXXXXXXXXXUETBF' + 'SHAAAB'"),
+ njs_str("XXXXXXXXXXXXXXXUETBFSHAAAB") },
+
+ /* values_hash short vs short string collision. */
+ { njs_str("'XUBAAAB' + 'XGYXKY'"),
+ njs_str("XUBAAABXGYXKY") },
+
{ njs_str("String.__proto__ === Function.prototype"),
njs_str("true") },

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[njs] Fixed njs_value_index().

Alexander Borisov 85 September 17, 2019 02:22AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 108
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready