Merge branch '4717-qp-lookup-creates-incorrect-chain' into 'main'

Fix dns_qp_lookup() chain inconsistency after fix_iterator()

Closes #4717

See merge request isc-projects/bind9!9028
This commit is contained in:
Evan Hunt 2024-05-14 20:36:11 +00:00
commit 0748847400
2 changed files with 81 additions and 24 deletions

View file

@ -2148,6 +2148,19 @@ fix_iterator(dns_qpreader_t *qp, dns_qpiter_t *iter, dns_qpkey_t search,
RUNTIME_CHECK(result == ISC_R_SUCCESS);
n = iter->stack[iter->sp];
foundlen = leaf_qpkey(qp, n, found);
size_t nto = qpkey_compare(search, searchlen, found,
foundlen);
if (nto < to) {
/*
* We've moved to a new leaf and it differs at
* an even earlier point, so no further
* improvement is possible.
*/
return;
}
to = nto;
} else {
if (to <= searchlen && to <= foundlen && iter->sp > 0) {
/*
@ -2187,18 +2200,6 @@ fix_iterator(dns_qpreader_t *qp, dns_qpiter_t *iter, dns_qpkey_t search,
return;
}
foundlen = leaf_qpkey(qp, n, found);
size_t nto = qpkey_compare(search, searchlen, found, foundlen);
if (nto < to) {
/*
* We've moved to a new leaf and it differs at an
* even earlier point, so no further improvement is
* possible.
*/
return;
}
to = nto;
}
if (is_branch(n)) {
@ -2240,6 +2241,23 @@ fix_iterator(dns_qpreader_t *qp, dns_qpiter_t *iter, dns_qpkey_t search,
}
}
/*
* When searching for a requested name in dns_qp_lookup(), we might add
* a leaf node to the chain, then subsequently determine that it was a
* dead end. When this happens, the chain can be left holding a node
* that is *not* an ancestor of the requested name. We correct for that
* here.
*/
static void
fix_chain(dns_qpchain_t *chain, size_t offset) {
while (chain->len > 0 && chain->chain[chain->len - 1].offset >= offset)
{
chain->len--;
chain->chain[chain->len].node = NULL;
chain->chain[chain->len].offset = 0;
}
}
isc_result_t
dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name,
dns_name_t *foundname, dns_qpiter_t *iter, dns_qpchain_t *chain,
@ -2363,16 +2381,20 @@ dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name,
foundlen = leaf_qpkey(qp, n, found);
offset = qpkey_compare(search, searchlen, found, foundlen);
/* the search ended with an exact or partial match */
if (offset == QPKEY_EQUAL || offset == foundlen) {
isc_result_t result = ISC_R_SUCCESS;
if (offset == foundlen) {
fix_chain(chain, offset);
result = DNS_R_PARTIALMATCH;
}
add_link(chain, n, offset);
SET_IF_NOT_NULL(pval_r, leaf_pval(n));
SET_IF_NOT_NULL(ival_r, leaf_ival(n));
maybe_set_name(qp, n, foundname);
add_link(chain, n, offset);
if (offset == QPKEY_EQUAL) {
return (ISC_R_SUCCESS);
} else {
return (DNS_R_PARTIALMATCH);
}
return (result);
}
/*

View file

@ -520,7 +520,8 @@ struct check_qpchain {
};
static void
check_qpchain(dns_qp_t *qp, struct check_qpchain check[]) {
check_qpchainiter(dns_qp_t *qp, struct check_qpchain check[],
dns_qpiter_t *iter) {
for (int i = 0; check[i].query != NULL; i++) {
isc_result_t result;
dns_fixedname_t fn1;
@ -529,9 +530,8 @@ check_qpchain(dns_qp_t *qp, struct check_qpchain check[]) {
dns_qpchain_init(qp, &chain);
dns_test_namefromstring(check[i].query, &fn1);
result = dns_qp_lookup(qp, name, NULL, NULL, &chain, NULL,
result = dns_qp_lookup(qp, name, NULL, iter, &chain, NULL,
NULL);
#if 0
fprintf(stderr, "%s %s (expected %s), "
"len %d (expected %d)\n", check[i].query,
@ -539,13 +539,13 @@ check_qpchain(dns_qp_t *qp, struct check_qpchain check[]) {
isc_result_totext(check[i].result),
dns_qpchain_length(&chain), check[i].length);
#endif
assert_int_equal(result, check[i].result);
assert_int_equal(dns_qpchain_length(&chain), check[i].length);
for (unsigned int j = 0; j < check[i].length; j++) {
dns_fixedname_t fn2, fn3;
dns_name_t *expected = dns_fixedname_initname(&fn2);
dns_name_t *found = dns_fixedname_initname(&fn3);
dns_test_namefromstring(check[i].names[j], &fn2);
dns_qpchain_node(&chain, j, found, NULL, NULL);
#if 0
@ -559,6 +559,14 @@ check_qpchain(dns_qp_t *qp, struct check_qpchain check[]) {
}
}
static void
check_qpchain(dns_qp_t *qp, struct check_qpchain check[]) {
dns_qpiter_t iter;
dns_qpiter_init(qp, &iter);
check_qpchainiter(qp, check, NULL);
check_qpchainiter(qp, check, &iter);
}
ISC_RUN_TEST_IMPL(qpchain) {
dns_qp_t *qp = NULL;
const char insert[][16] = { ".", "a.", "b.",
@ -592,6 +600,24 @@ ISC_RUN_TEST_IMPL(qpchain) {
check_qpchain(qp, check1);
dns_qp_destroy(&qp);
const char insert2[][16] = { "a.", "d.b.a.", "z.d.b.a.", "" };
i = 0;
dns_qp_create(mctx, &string_methods, NULL, &qp);
while (insert2[i][0] != '\0') {
insert_str(qp, insert2[i++]);
}
static struct check_qpchain check2[] = {
{ "f.c.b.a.", DNS_R_PARTIALMATCH, 1, { "a." } },
{ NULL, 0, 0, { NULL } },
};
check_qpchain(qp, check2);
dns_qp_destroy(&qp);
}
struct check_predecessors {
@ -602,7 +628,8 @@ struct check_predecessors {
};
static void
check_predecessors(dns_qp_t *qp, struct check_predecessors check[]) {
check_predecessors_withchain(dns_qp_t *qp, struct check_predecessors check[],
dns_qpchain_t *chain) {
isc_result_t result;
dns_fixedname_t fn1, fn2;
dns_name_t *name = dns_fixedname_initname(&fn1);
@ -626,7 +653,7 @@ check_predecessors(dns_qp_t *qp, struct check_predecessors check[]) {
result = dns_name_tostring(expred, &predstr, mctx);
assert_int_equal(result, ISC_R_SUCCESS);
result = dns_qp_lookup(qp, name, NULL, &it, NULL, NULL, NULL);
result = dns_qp_lookup(qp, name, NULL, &it, chain, NULL, NULL);
#if 0
fprintf(stderr, "%s: expected %s got %s\n", check[i].query,
isc_result_totext(check[i].result),
@ -685,6 +712,14 @@ check_predecessors(dns_qp_t *qp, struct check_predecessors check[]) {
}
}
static void
check_predecessors(dns_qp_t *qp, struct check_predecessors check[]) {
dns_qpchain_t chain;
dns_qpchain_init(qp, &chain);
check_predecessors_withchain(qp, check, NULL);
check_predecessors_withchain(qp, check, &chain);
}
ISC_RUN_TEST_IMPL(predecessors) {
dns_qp_t *qp = NULL;
const char insert[][16] = {