From 7cefcb6184952074e3a3289ba67c7c37cb8a3cac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 8 Dec 2022 10:46:09 +0100 Subject: [PATCH] Allow zero length keys in isc_hashmap In case, we are trying to hash the empty key into the hashmap, the key is going to have zero length. This might happen in the unit test. Allow this and add a unit test to ensure the empty zero-length key doesn't hash to slot 0 as SipHash 2-4 (our hash function of choice) has no problem with zero-length inputs. --- lib/isc/hashmap.c | 9 ++++----- tests/isc/hashmap_test.c | 25 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/lib/isc/hashmap.c b/lib/isc/hashmap.c index 0abb1220c2..2ea7a25182 100644 --- a/lib/isc/hashmap.c +++ b/lib/isc/hashmap.c @@ -134,7 +134,7 @@ try_nexttable(const isc_hashmap_t *hashmap, uint8_t idx) { static void hashmap_node_init(hashmap_node_t *node, const uint32_t hashval, const uint8_t *key, const uint32_t keysize, void *value) { - REQUIRE(key != NULL && keysize > 0 && keysize <= UINT16_MAX); + REQUIRE(key != NULL && keysize <= UINT16_MAX); *node = (hashmap_node_t){ .value = value, @@ -318,7 +318,6 @@ uint32_t isc_hashmap_hash(const isc_hashmap_t *hashmap, const void *key, uint32_t keysize) { REQUIRE(ISC_HASHMAP_VALID(hashmap)); - REQUIRE(key != NULL && keysize > 0 && keysize <= UINT16_MAX); uint32_t hashval; @@ -332,7 +331,7 @@ isc_result_t isc_hashmap_find(const isc_hashmap_t *hashmap, const uint32_t *hashvalp, const void *key, uint32_t keysize, void **valuep) { REQUIRE(ISC_HASHMAP_VALID(hashmap)); - REQUIRE(key != NULL && keysize > 0 && keysize <= UINT16_MAX); + REQUIRE(key != NULL && keysize <= UINT16_MAX); hashmap_node_t *node; uint8_t idx = hashmap->hindex; @@ -487,7 +486,7 @@ isc_result_t isc_hashmap_delete(isc_hashmap_t *hashmap, const uint32_t *hashvalp, const void *key, uint32_t keysize) { REQUIRE(ISC_HASHMAP_VALID(hashmap)); - REQUIRE(key != NULL && keysize > 0 && keysize <= UINT16_MAX); + REQUIRE(key != NULL && keysize <= UINT16_MAX); hashmap_node_t *node; isc_result_t result = ISC_R_NOTFOUND; @@ -600,7 +599,7 @@ isc_result_t isc_hashmap_add(isc_hashmap_t *hashmap, const uint32_t *hashvalp, const void *key, uint32_t keysize, void *value) { REQUIRE(ISC_HASHMAP_VALID(hashmap)); - REQUIRE(key != NULL && keysize > 0 && keysize <= UINT16_MAX); + REQUIRE(key != NULL && keysize <= UINT16_MAX); isc_result_t result; uint32_t hashval = (hashvalp != NULL) diff --git a/tests/isc/hashmap_test.c b/tests/isc/hashmap_test.c index 334ed8d64f..99b7951f57 100644 --- a/tests/isc/hashmap_test.c +++ b/tests/isc/hashmap_test.c @@ -351,6 +351,30 @@ ISC_RUN_TEST_IMPL(isc_hashmap_iterator) { return; } +ISC_RUN_TEST_IMPL(isc_hashmap_hash_zero_length) { + isc_hashmap_t *hashmap = NULL; + uint32_t hashval; + bool again = false; + +again: + isc_hashmap_create(mctx, 1, ISC_HASHMAP_CASE_SENSITIVE, &hashmap); + + hashval = isc_hashmap_hash(hashmap, "", 0); + + isc_hashmap_destroy(&hashmap); + + if (hashval == 0 && !again) { + /* + * We could be extremely unlock and the siphash could hash the + * zero length string to 0, so try one more time. + */ + again = true; + goto again; + } + + assert_int_not_equal(hashval, 0); +} + ISC_RUN_TEST_IMPL(isc_hashmap_case) { isc_result_t result; isc_hashmap_t *hashmap = NULL; @@ -401,6 +425,7 @@ ISC_RUN_TEST_IMPL(isc_hashmap_case) { } ISC_TEST_LIST_START +ISC_TEST_ENTRY(isc_hashmap_hash_zero_length) ISC_TEST_ENTRY(isc_hashmap_case) ISC_TEST_ENTRY(isc_hashmap_1_120) ISC_TEST_ENTRY(isc_hashmap_6_1000)