diff --git a/lib/dns/deleg.c b/lib/dns/deleg.c index dc7f5a42da..28fb777ef8 100644 --- a/lib/dns/deleg.c +++ b/lib/dns/deleg.c @@ -267,14 +267,22 @@ getparentnode(dns_qpchain_t *chain, delegdb_node_t **node, dns_ttl_t now) { size_t len = dns_qpchain_length(chain); while (len >= 2) { - *node = NULL; - dns_qpchain_node(chain, len - 2, (void **)node, NULL); + delegdb_node_t *parent = NULL; + dns_qpchain_node(chain, len - 2, (void **)&parent, NULL); - if (isactive(*node, now)) { - break; + if (isactive(parent, now)) { + *node = parent; + return; } len--; } + + /* + * No active proper ancestor was found in the chain. Signal + * "no parent" so the caller does not mistake the original + * matched node for an ancestor. + */ + *node = NULL; } /* @@ -309,27 +317,36 @@ dns__deleg_lookup(dns_delegdb_t *delegdb, dns_qpread_t *qpr, dns_name_copy(&node->zonecut, deepestzonecut); } - if (result == ISC_R_SUCCESS && (noexact || !isactive(node, now))) { - getparentnode(&chain, &node, now); - } else if (result == DNS_R_PARTIALMATCH && !isactive(node, now)) { + /* + * Walk up the chain when: + * - we have an exact match but the caller asked for NOEXACT + * (i.e. the caller wants the deepest *proper* ancestor), or + * - the matched node is no longer active and we need to fall + * back to the closest still-active ancestor (this applies + * equally to exact and partial matches). + * + * getparentnode() sets 'node' to NULL when no active ancestor + * exists in the chain, so we must NULL-check before dereferencing + * 'node' below. + */ + if ((result == ISC_R_SUCCESS && noexact) || !isactive(node, now)) { getparentnode(&chain, &node, now); } - result = isactive(node, now) ? ISC_R_SUCCESS : ISC_R_NOTFOUND; - if (result == ISC_R_SUCCESS) { + if (node != NULL && isactive(node, now)) { dns_name_copy(&node->zonecut, zonecut); INSIST(node->delegset); dns_delegset_attach(node->delegset, delegsetp); ISC_SIEVE_MARK(node, visited); - } else { - /* - * FIXME: if we lookup something that has expired, we need - * either the "deadnodes" (see qpcache) mechanism here - or call - * something like isc_async_run(delete_me, node). - */ + return ISC_R_SUCCESS; } - return result; + /* + * FIXME: if we lookup something that has expired, we need + * either the "deadnodes" (see qpcache) mechanism here - or call + * something like isc_async_run(delete_me, node). + */ + return ISC_R_NOTFOUND; } isc_result_t diff --git a/tests/dns/deleg_test.c b/tests/dns/deleg_test.c index 904eaeef00..e27b7b1eb7 100644 --- a/tests/dns/deleg_test.c +++ b/tests/dns/deleg_test.c @@ -124,13 +124,16 @@ lookupdb(dns_delegdb_t *db, const char *namestr, isc_stdtime_t now, *expectedzc = dns_fixedname_initname(&fexpectedzc), *zonecut = dns_fixedname_initname(&fzonecut); - dns_name_fromstring(expectedzc, expectedzcstr, NULL, 0, NULL); + if (expectedzcstr != NULL) { + dns_name_fromstring(expectedzc, expectedzcstr, NULL, 0, NULL); + } dns_name_fromstring(name, namestr, NULL, 0, NULL); result = dns_delegdb_lookup(db, name, now, options, zonecut, NULL, delegsetp); if (result == ISC_R_SUCCESS) { assert_non_null(*delegsetp); + assert_non_null(expectedzcstr); assert_true(dns_name_equal(zonecut, expectedzc)); } else { assert_null(*delegsetp); @@ -411,15 +414,22 @@ noexacttests(ISC_ATTR_UNUSED void *arg) { const char *name; const char *expected; const char *noexactexpected; + isc_result_t noexactresult; dns_ttl_t ttl; } zonecuts[] = { - { "stuff.", "stuff.", "stuff.", 30 }, - { "foo.stuff.", "foo.stuff.", "stuff.", 30 }, - { "expired.foo.stuff.", "foo.stuff.", "foo.stuff.", 1 }, + /* + * "stuff." has no proper ancestor in the trie, so a + * NOEXACT lookup must return NOTFOUND rather than the + * exact match itself. + */ + { "stuff.", "stuff.", NULL, ISC_R_NOTFOUND, 30 }, + { "foo.stuff.", "foo.stuff.", "stuff.", ISC_R_SUCCESS, 30 }, + { "expired.foo.stuff.", "foo.stuff.", "foo.stuff.", + ISC_R_SUCCESS, 1 }, { "bar.expired.foo.stuff.", "bar.expired.foo.stuff.", - "foo.stuff.", 30 }, + "foo.stuff.", ISC_R_SUCCESS, 30 }, { "baz.bar.expired.foo.stuff.", "baz.bar.expired.foo.stuff.", - "bar.expired.foo.stuff.", 30 } + "bar.expired.foo.stuff.", ISC_R_SUCCESS, 30 } }; for (size_t i = 0; i < ARRAY_SIZE(zonecuts); i++) { @@ -440,8 +450,10 @@ noexacttests(ISC_ATTR_UNUSED void *arg) { result = lookupdb(db, zonecuts[i].name, now + 1, DNS_DBFIND_NOEXACT, zonecuts[i].noexactexpected, &delegset); - assert_int_equal(result, ISC_R_SUCCESS); - dns_delegset_detach(&delegset); + assert_int_equal(result, zonecuts[i].noexactresult); + if (result == ISC_R_SUCCESS) { + dns_delegset_detach(&delegset); + } } result = lookupdb(db, "gee.expired.foo.stuff.", now + 1, 0,