From 2a724a808db8111365dcb54b1bd379d02cb135f1 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Tue, 19 Mar 2024 09:45:59 +0100
Subject: [PATCH 1/2] Add a test case for fix_iterator hang

When fixing the iterator, when every leaf on this branch is greater
than the one we wanted we go back to the parent branch and iterate back
to the predecessor from that point.

But if there are no more previous leafs, it means the queried name
precedes the entire range of names in the database, so we would just
move the iterator one step back and continue from there.

This could end in a loop because the queried name precedes the entire
range of names and so none of those names are the predecessor of the
queried name.
---
 tests/dns/qp_test.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tests/dns/qp_test.c b/tests/dns/qp_test.c
index b9c3afcc5f..7dc0f68c2a 100644
--- a/tests/dns/qp_test.c
+++ b/tests/dns/qp_test.c
@@ -777,6 +777,7 @@ ISC_RUN_TEST_IMPL(fixiterator) {
 				    "private-dnskey.dynamic.",
 				    "rrsig.dynamic.",
 				    "txt.dynamic.",
+				    "trailing.",
 				    "" };
 	int i = 0;
 
@@ -786,9 +787,11 @@ ISC_RUN_TEST_IMPL(fixiterator) {
 	}
 
 	static struct check_predecessors check1[] = {
-		{ "newtext.dynamic.", "mx.dynamic.", DNS_R_PARTIALMATCH, 6 },
-		{ "absent.", "txt.dynamic.", ISC_R_NOTFOUND, 0 },
-		{ "nonexistent.", "txt.dynamic.", ISC_R_NOTFOUND, 0 },
+		{ "newtext.dynamic.", "mx.dynamic.", DNS_R_PARTIALMATCH, 7 },
+		{ "d.", "trailing.", ISC_R_NOTFOUND, 0 },
+		{ "absent.", "trailing.", ISC_R_NOTFOUND, 0 },
+		{ "nonexistent.", "txt.dynamic.", ISC_R_NOTFOUND, 1 },
+		{ "wayback.", "trailing.", ISC_R_NOTFOUND, 0 },
 		{ NULL, NULL, 0, 0 }
 	};
 

From 77d4bb9751b36811ea59e1d8bef61de3461765b6 Mon Sep 17 00:00:00 2001
From: Matthijs Mekking <matthijs@isc.org>
Date: Tue, 19 Mar 2024 08:48:56 +0100
Subject: [PATCH 2/2] Fix fix_iterator hang

If there are no more previous leaves, it means the queried name
precedes the entire range of names in the database, so we should just
move the iterator one step back and return, instead of continuing our
search for the predecessor.

This is similar to an earlier bug fixed in an earlier commit:

    ea9a8cb392ff59438a911485742b220d40f24d6f
---
 lib/dns/qp.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/lib/dns/qp.c b/lib/dns/qp.c
index 4e70fa0dac..dff011ba66 100644
--- a/lib/dns/qp.c
+++ b/lib/dns/qp.c
@@ -2126,6 +2126,7 @@ fix_iterator(dns_qpreader_t *qp, dns_qpiter_t *iter, dns_qpnode_t *start,
 		dns_qpiter_init(qp, iter);
 		return (prevleaf(iter));
 	}
+
 	/*
 	 * As long as the branch offset point is after the point where the
 	 * search key differs, we need to branch up and find a better leaf
@@ -2139,7 +2140,17 @@ fix_iterator(dns_qpreader_t *qp, dns_qpiter_t *iter, dns_qpnode_t *start,
 			 * go to the parent branch and iterate back to the
 			 * predecessor from that point.
 			 */
-			n = prevleaf(iter);
+			isc_result_t result = dns_qpiter_prev(iter, NULL, NULL,
+							      NULL);
+			if (result == ISC_R_NOMORE) {
+				/*
+				 * This was the first domain; move the iterator
+				 * one step back from the origin and return.
+				 */
+				return (prevleaf(iter));
+			}
+			RUNTIME_CHECK(result == ISC_R_SUCCESS);
+			n = iter->stack[iter->sp];
 			leaf = n;
 		} else {
 			if (is_branch(n)) {