From: Ondřej Surý <ondrej@isc.org>
Date: Thu, 8 Dec 2022 09:46:09 +0000 (+0100)
Subject: Allow zero length keys in isc_hashmap
X-Git-Tag: v9.19.9~81^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7cefcb6184952074e3a3289ba67c7c37cb8a3cac;p=thirdparty%2Fbind9.git

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.
---

diff --git a/lib/isc/hashmap.c b/lib/isc/hashmap.c
index 0abb1220c2f..2ea7a251821 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 334ed8d64f4..99b7951f57c 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)