BIND 10 trac859, updated. 7ab14337f13be6a6deaa5152085e78540c205d26 [trac846] Split the logic to separate function
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Apr 26 19:00:37 UTC 2011
The branch, trac859 has been updated
via 7ab14337f13be6a6deaa5152085e78540c205d26 (commit)
from abdb0ff5e8cdd781d7c4960bb51d8538f4e3b44e (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 7ab14337f13be6a6deaa5152085e78540c205d26
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Apr 26 20:59:59 2011 +0200
[trac846] Split the logic to separate function
-----------------------------------------------------------------------
Summary of changes:
src/lib/resolve/recursive_query.cc | 78 ++++++++++++---------
src/lib/resolve/tests/recursive_query_unittest.cc | 42 +++++++++--
2 files changed, 78 insertions(+), 42 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/resolve/recursive_query.cc b/src/lib/resolve/recursive_query.cc
index 7f28462..34411ee 100644
--- a/src/lib/resolve/recursive_query.cc
+++ b/src/lib/resolve/recursive_query.cc
@@ -66,6 +66,48 @@ hasAddress(const Name& name, const RRClass& rrClass,
}
+/// \brief Find deepest usable delegation in the cache
+///
+/// This finds the deepest delegation we have in cache and is safe to use.
+/// It is not public function, therefore it's not in header. But it's not
+/// in anonymous namespace, so we can call it from unittests.
+/// \param name The name we want to delegate to.
+/// \param cache The place too look for known delegations.
+std::string
+deepestDelegation(Name name, RRClass rrclass,
+ isc::cache::ResolverCache& cache)
+{
+ RRsetPtr cachedNS;
+ // Look for delegation point from bottom, until we find one with
+ // IP address or get to root.
+ //
+ // We need delegation with IP address so we can ask it right away.
+ // If we don't have the IP address, we would need to ask above it
+ // anyway in the best case, and the NS could be inside the zone,
+ // and we could get all loopy with the NSAS in the worst case.
+ while (name.getLabelCount() > 1 &&
+ (cachedNS = cache.lookupDeepestNS(name, rrclass)) != RRsetPtr()) {
+ // Look if we have an IP address for the NS
+ for (RdataIteratorPtr ns(cachedNS->getRdataIterator());
+ !ns->isLast(); ns->next()) {
+ // Do we have IP for this specific NS?
+ if (hasAddress(dynamic_cast<const rdata::generic::NS&>(
+ ns->getCurrent()).getNSName(), rrclass,
+ cache)) {
+ // Found one, stop checking and use this zone
+ // (there may be more addresses, that's only better)
+ return (cachedNS->getName().toText());
+ }
+ }
+ // We don't have anything for this one, so try something higher
+ if (name.getLabelCount() > 1) {
+ name = name.split(1);
+ }
+ }
+ // Fallback, nothing found, start at root
+ return (".");
+}
+
typedef std::vector<std::pair<std::string, uint16_t> > AddressVector;
// Here we do not use the typedef above, as the SunStudio compiler
@@ -273,40 +315,8 @@ private:
}
} else {
dlog("doLookup: get lowest usable delegation from cache");
- cur_zone_ = ".";
- bool foundAddress(false);
- RRsetPtr cachedNS;
- Name tryName(question_.getName());
- // Look for delegation point from bottom, until we find one with
- // IP address or get to root.
- //
- // We need delegation with IP address so we can ask it right away.
- // If we don't have the IP address, we would need to ask above it
- // anyway in the best case, and the NS could be inside the zone,
- // and we could get all loopy with the NSAS in the worst case.
- while (!foundAddress && tryName.getLabelCount() > 1 &&
- (cachedNS = cache_.lookupDeepestNS(tryName,
- question_.getClass())) !=
- RRsetPtr()) {
- // Look if we have an IP address for the NS
- for (RdataIteratorPtr ns(cachedNS->getRdataIterator());
- !ns->isLast(); ns->next()) {
- // Do we have IP for this specific NS?
- if (hasAddress(dynamic_cast<const rdata::generic::NS&>(
- ns->getCurrent()).getNSName(),
- question_.getClass(), cache_)) {
- // Found one, stop checking and use this zone
- // (there may be more addresses, that's only better)
- foundAddress = true;
- cur_zone_ = cachedNS->getName().toText();
- break;
- }
- }
- // We don't have anything for this one, so try something higher
- if (!foundAddress && tryName.getLabelCount() > 1) {
- tryName = tryName.split(1);
- }
- }
+ cur_zone_ = deepestDelegation(question_.getName(),
+ question_.getClass(), cache_);
send();
}
diff --git a/src/lib/resolve/tests/recursive_query_unittest.cc b/src/lib/resolve/tests/recursive_query_unittest.cc
index 85f78ba..2520994 100644
--- a/src/lib/resolve/tests/recursive_query_unittest.cc
+++ b/src/lib/resolve/tests/recursive_query_unittest.cc
@@ -61,6 +61,18 @@ using namespace isc::asiolink;
using namespace isc::dns;
using namespace isc::util;
+namespace isc {
+namespace asiodns {
+
+// This is defined in recursive_query.cc, but not in header (it's not public
+// function). So bring it in to be tested.
+std::string
+deepestDelegation(Name name, RRClass rrclass,
+ isc::cache::ResolverCache& cache);
+
+}
+}
+
namespace {
const char* const TEST_SERVER_PORT = "53535";
const char* const TEST_CLIENT_PORT = "53536";
@@ -861,6 +873,11 @@ TEST_F(RecursiveQueryTest, DISABLED_recursiveSendNXDOMAIN) {
TEST_F(RecursiveQueryTest, CachedNS) {
setDNSService(true, true);
+ // Check we have a reasonable fallback - if there's nothing of interest
+ // in the cache, start at root.
+ EXPECT_EQ(".", deepestDelegation(Name("www.somewhere.deep.example.org"),
+ RRClass::IN(), cache_));
+
// Prefill the cache. There's a zone with a NS and IP address for one
// of them (to see that one is enough) and another deeper one, with NS,
// but without IP.
@@ -889,6 +906,22 @@ TEST_F(RecursiveQueryTest, CachedNS) {
ASSERT_NE(RRsetPtr(), deepest);
ASSERT_EQ(nsLower->getName(), deepest->getName());
+ // Direct check of the function that chooses the delegation point
+ // It should not use nsLower, because we don't have IP address for
+ // that one. But it can choose nsUpper.
+ EXPECT_EQ("example.org.",
+ deepestDelegation(Name("www.somewhere.deep.example.org"),
+ RRClass::IN(), cache_));
+
+ // Now more complex and indirect test:
+ // We ask it to resolve the name for us. It will pick up a delegation
+ // point and ask NSAS for it. NSAS will in turn ask resolver for NS record
+ // of the delegation point. We then pick it up from the fake resolver
+ // and check it is the correct one. This checks the delegation point
+ // travels safely trough the whole path there (it would be enough to check
+ // it up to NSAS, but replacing NSAS is more complicated, so we just
+ // include in the test as well for simplicity).
+
// Prepare the recursive query
vector<pair<string, uint16_t> > roots;
roots.push_back(pair<string, uint16_t>("192.0.2.2", 53));
@@ -909,14 +942,7 @@ TEST_F(RecursiveQueryTest, CachedNS) {
// We don't need to run the service in this test. We are interested only
// in the place it starts resolving at
- // Now, it should not start at the nsLower, because we don't have IP
- // address for it and that could cause a loop. But it can be the nsUpper,
- // we don't need to go to root.
- //
- // We use the trick that NSAS asks the resolver for the NS record, because
- // it doesn't know it. This isn't the best way to write a unittest, but
- // it isn't easy to replace the NSAS with something else, so this is the
- // least painfull way.
+ // Look what is asked by NSAS - it should be our delegation point.
EXPECT_NO_THROW(EXPECT_EQ(nsUpper->getName(),
(*resolver_)[0]->getName()) <<
"It starts resolving at the wrong place") <<
More information about the bind10-changes
mailing list