mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-30 14:07:59 +00:00
Code Issues Releases Wiki Activity

and fix yet another dns_qp_lookup() iterator bug

Browse Source 
This one is similar to the bug when searching for a key, reaching a
dead-end branch that doesn't match, because the branch offset point
is after the point where the search key differs.

This fixes the case where we are multiple levels deep. In other
words, we had a more-than-one matches *after* the point where the
search key differs.

For example, consider the following qp-trie:

branch: "[e]", "[m]":
 - leaf: "a.b.c.d.e"
 - branch: "moo[g]", "moo[k]", "moo[n]":
   - leaf: "moog"
   - branch: "mook[e]", "mook[o]"
     - leaf: "mooker"
     - leaf: "mooko"
   - leaf: "moon"

If searching for a key "monky", we would reach the branch with
twigs "moo[k]" and "moo[n]". The key matches on the 'k' on offset=4,
and reaches the branch with twigs "mook[e]" and "mook[o]". This time
we cannot find a twig that matches our key at offset=5, there is no
twig for 'y'. The closest name we found was "mooker".

Note that on a branch it can't detect it is on a dead branch because the
key is not encapsulated in a branch node.

In the previous code we considered "mooker" to be the successor of
"monky" and so we needed to the predecessor of "mooker" to find the
predecessor for "monky". However, since the search key alread differed
before entering this branch, this is not enough. We would be left with
"moog" as the predecessor of "monky", while in this example "a.b.c.d.e"
is the actual predecessor.

Instead, we need to go up a level, find the predecessor and check
again if we are on the right branch, and repeat the process until we
are.

Unit tests to cover the scenario are now added.
This commit is contained in:
Matthijs Mekking 2023-12-07 11:51:23 +01:00 committed by Evan Hunt
parent 276bdcf5cf
commit ab8a0c4b5a
2 changed files with 24 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
lib/dns/qp.c
View File

@ -2139,12 +2139,14 @@ dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name,
*/
size_t to;
dns_qpnode_t *least = n;
least:
while (is_branch(least)) {
least = branch_twigs(qp, least);
}
foundlen = leaf_qpkey(qp, least, found);
to = qpkey_compare(search, searchlen, found, foundlen);
if (to == offset) {
if (to >= offset) {
/*
* we're on the right branch, so find
* the best match.
@ -2180,9 +2182,12 @@ dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name,
* we wanted, so iterate back to the
* predecessor.
*/
iter->sp--;
prevleaf(iter);
n = iter->stack[iter->sp--];
} else {
n = iter->stack[iter->sp];
least = n;
goto least;
} else if (is_branch(n)) {
/*
* every leaf is less than the one we
* wanted, so get the highest.

21
tests/dns/qp_test.c
View File

@ -635,11 +635,12 @@ check_predecessors(dns_qp_t *qp, struct check_predecessors check[]) {
ISC_RUN_TEST_IMPL(predecessors) {
dns_qp_t *qp = NULL;
const char insert[][16] = { "a.", "b.", "c.b.a.",
"e.d.c.b.a.", "c.b.b.", "c.d.",
"a.b.c.d.", "a.b.c.d.e.", "b.a.",
"x.k.c.d.", "moog.", "mook.",
"moon.", "moops.", "" };
const char insert[][16] = {
"a.", "b.", "c.b.a.", "e.d.c.b.a.",
"c.b.b.", "c.d.", "a.b.c.d.", "a.b.c.d.e.",
"b.a.", "x.k.c.d.", "moog.", "mooker.",
"mooko.", "moon.", "moops.", ""
};
int i = 0;
dns_qp_create(mctx, &string_methods, NULL, &qp);
@ -669,14 +670,19 @@ ISC_RUN_TEST_IMPL(predecessors) {
{ "moor.", "moops.", ISC_R_NOTFOUND },
{ "mopbop.", "moops.", ISC_R_NOTFOUND },
{ "moppop.", "moops.", ISC_R_NOTFOUND },
{ "mopps.", "moops.", ISC_R_NOTFOUND },
{ "mopzop.", "moops.", ISC_R_NOTFOUND },
{ "mop.", "moops.", ISC_R_NOTFOUND },
{ "monbop.", "a.b.c.d.e.", ISC_R_NOTFOUND },
{ "monpop.", "a.b.c.d.e.", ISC_R_NOTFOUND },
{ "monps.", "a.b.c.d.e.", ISC_R_NOTFOUND },
{ "monzop.", "a.b.c.d.e.", ISC_R_NOTFOUND },
{ "mon.", "a.b.c.d.e.", ISC_R_NOTFOUND },
{ "moop.", "moon.", ISC_R_NOTFOUND },
{ "moopser.", "moops.", ISC_R_NOTFOUND },
{ "monky.", "a.b.c.d.e.", ISC_R_NOTFOUND },
{ "monkey.", "a.b.c.d.e.", ISC_R_NOTFOUND },
{ "monker.", "a.b.c.d.e.", ISC_R_NOTFOUND },
{ NULL, NULL, 0 }
};
@ -706,14 +712,19 @@ ISC_RUN_TEST_IMPL(predecessors) {
{ "moor.", "moops.", DNS_R_PARTIALMATCH },
{ "mopbop.", "moops.", DNS_R_PARTIALMATCH },
{ "moppop.", "moops.", DNS_R_PARTIALMATCH },
{ "mopps.", "moops.", DNS_R_PARTIALMATCH },
{ "mopzop.", "moops.", DNS_R_PARTIALMATCH },
{ "mop.", "moops.", DNS_R_PARTIALMATCH },
{ "monbop.", "a.b.c.d.e.", DNS_R_PARTIALMATCH },
{ "monpop.", "a.b.c.d.e.", DNS_R_PARTIALMATCH },
{ "monps.", "a.b.c.d.e.", DNS_R_PARTIALMATCH },
{ "monzop.", "a.b.c.d.e.", DNS_R_PARTIALMATCH },
{ "mon.", "a.b.c.d.e.", DNS_R_PARTIALMATCH },
{ "moop.", "moon.", DNS_R_PARTIALMATCH },
{ "moopser.", "moops.", DNS_R_PARTIALMATCH },
{ "monky.", "a.b.c.d.e.", DNS_R_PARTIALMATCH },
{ "monkey.", "a.b.c.d.e.", DNS_R_PARTIALMATCH },
{ "monker.", "a.b.c.d.e.", DNS_R_PARTIALMATCH },
{ NULL, NULL, 0 }
};