BIND 10 master, updated. 483f1075942965f0340291e7ff7dae7806df22af [master] Merge branch 'trac1771-2'
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Jun 27 22:07:21 UTC 2012
The branch, master has been updated
via 483f1075942965f0340291e7ff7dae7806df22af (commit)
via d1e088cf6ecc8d99c293d195a05433d9d3742ba4 (commit)
via 930e5b9931e061578f77ac3acc9d796459c13633 (commit)
via f3e904c386e2a0d93cba0c7df8698c5ba1b441ce (commit)
via ed608bae6873c4597f2136804758d68da84754c2 (commit)
via ef571c297aabab0808ac3eecf3c457fdce55a73d (commit)
from 35b4c61aa77ffef14b5060709c03bc196ce19646 (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 483f1075942965f0340291e7ff7dae7806df22af
Merge: 35b4c61 d1e088c
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed Jun 27 14:41:16 2012 -0700
[master] Merge branch 'trac1771-2'
commit d1e088cf6ecc8d99c293d195a05433d9d3742ba4
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed Jun 27 14:29:51 2012 -0700
[1771-2] cleanup: minimized the scope of some local variables.
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/database.cc | 57 ++++++++------------
src/lib/datasrc/database.h | 15 +++---
src/lib/datasrc/tests/database_unittest.cc | 37 ++++++++-----
.../datasrc/tests/zone_finder_context_unittest.cc | 13 -----
src/lib/datasrc/zone.h | 10 ++++
5 files changed, 62 insertions(+), 70 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index 61dab39..a8ba55e 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -179,8 +179,7 @@ private:
DatabaseClient::Finder::FoundRRsets
DatabaseClient::Finder::getRRsets(const string& name, const WantedTypes& types,
- bool check_ns, const string* construct_name,
- bool any,
+ const string* construct_name, bool any,
DatabaseAccessor::IteratorContextPtr context)
{
RRsigStore sig_store;
@@ -204,9 +203,7 @@ DatabaseClient::Finder::getRRsets(const string& name, const WantedTypes& types,
const Name construct_name_object(*construct_name);
bool seen_cname(false);
- bool seen_ds(false);
bool seen_other(false);
- bool seen_ns(false);
while (context->getNext(columns)) {
// The domain is not empty
@@ -249,16 +246,12 @@ DatabaseClient::Finder::getRRsets(const string& name, const WantedTypes& types,
if (cur_type == RRType::CNAME()) {
seen_cname = true;
- } else if (cur_type == RRType::NS()) {
- seen_ns = true;
- } else if (cur_type == RRType::DS()) {
- seen_ds = true;
} else if (cur_type != RRType::RRSIG() &&
cur_type != RRType::NSEC3() &&
cur_type != RRType::NSEC()) {
// NSEC and RRSIG can coexist with anything, otherwise
// we've seen something that can't live together with potential
- // CNAME or NS
+ // CNAME.
//
// NSEC3 lives in separate namespace from everything, therefore
// we just ignore it here for these checks as well.
@@ -278,14 +271,10 @@ DatabaseClient::Finder::getRRsets(const string& name, const WantedTypes& types,
RDATA_COLUMN]);
}
}
- if (seen_cname && (seen_other || seen_ns || seen_ds)) {
+ if (seen_cname && seen_other) {
isc_throw(DataSourceError, "CNAME shares domain " << name <<
" with something else");
}
- if (check_ns && seen_ns && seen_other) {
- isc_throw(DataSourceError, "NS shares domain " << name <<
- " with something else");
- }
// Add signatures to all found RRsets
for (std::map<RRType, RRsetPtr>::iterator i(result.begin());
i != result.end(); ++ i) {
@@ -455,20 +444,20 @@ DatabaseClient::Finder::findDelegationPoint(const isc::dns::Name& name,
for (int i = remove_labels; i > 0; --i) {
const Name superdomain(name.split(i));
- // Note if this is the origin. (We don't count NS records at the origin
- // as a delegation so this controls whether NS RRs are included in
- // the results of some searches.)
- const bool not_origin = (i != remove_labels);
-
// Look if there's NS or DNAME at this point of the tree, but ignore
// the NS RRs at the apex of the zone.
const FoundRRsets found = getRRsets(superdomain.toText(),
- DELEGATION_TYPES(), not_origin);
+ DELEGATION_TYPES());
if (found.first) {
// This node contains either NS or DNAME RRs so it does exist.
const FoundIterator nsi(found.second.find(RRType::NS()));
const FoundIterator dni(found.second.find(RRType::DNAME()));
+ // Note if this is the origin. (We don't count NS records at the
+ // origin as a delegation so this controls whether NS RRs are
+ // included in the results of some searches.)
+ const bool not_origin = (i != remove_labels);
+
// An optimisation. We know that there is an exact match for
// something at this point in the tree so remember it. If we have
// to do a wildcard search, as we search upwards through the tree
@@ -477,7 +466,7 @@ DatabaseClient::Finder::findDelegationPoint(const isc::dns::Name& name,
last_known = superdomain.getLabelCount();
if (glue_ok && !first_ns && not_origin &&
- nsi != found.second.end()) {
+ nsi != found.second.end()) {
// If we are searching for glue ("glue OK" mode), store the
// highest NS record that we find that is not the apex. This
// is another optimisation for later, where we need the
@@ -590,8 +579,9 @@ DatabaseClient::Finder::findWildcardMatch(
// TODO Add a check for DNAME, as DNAME wildcards are discouraged (see
// RFC 4592 section 4.4).
// Search for a match. The types are the same as with original query.
- FoundRRsets found = getRRsets(wildcard, final_types, true,
- &construct_name, type == RRType::ANY());
+ const FoundRRsets found = getRRsets(wildcard, final_types,
+ &construct_name,
+ type == RRType::ANY());
if (found.first) {
// Found something - but what?
@@ -694,7 +684,7 @@ DatabaseClient::Finder::FindDNSSECContext::probe() {
// such cases).
const string origin = finder_.getOrigin().toText();
const FoundRRsets nsec3_found =
- finder_.getRRsets(origin, NSEC3PARAM_TYPES(), false);
+ finder_.getRRsets(origin, NSEC3PARAM_TYPES());
const FoundIterator nfi=
nsec3_found.second.find(RRType::NSEC3PARAM());
is_nsec3_ = (nfi != nsec3_found.second.end());
@@ -705,7 +695,7 @@ DatabaseClient::Finder::FindDNSSECContext::probe() {
// described in Section 10.4 of RFC 5155.
if (!is_nsec3_) {
const FoundRRsets nsec_found =
- finder_.getRRsets(origin, NSEC_TYPES(), false);
+ finder_.getRRsets(origin, NSEC_TYPES());
const FoundIterator nfi =
nsec_found.second.find(RRType::NSEC());
is_nsec_ = (nfi != nsec_found.second.end());
@@ -757,10 +747,8 @@ DatabaseClient::Finder::FindDNSSECContext::getDNSSECRRset(const Name &name,
try {
const Name& nsec_name =
covering ? finder_.findPreviousName(name) : name;
- const bool need_nscheck = (nsec_name != finder_.getOrigin());
const FoundRRsets found = finder_.getRRsets(nsec_name.toText(),
- NSEC_TYPES(),
- need_nscheck);
+ NSEC_TYPES());
const FoundIterator nci = found.second.find(RRType::NSEC());
if (nci != found.second.end()) {
return (nci->second);
@@ -984,16 +972,15 @@ DatabaseClient::Finder::findInternal(const Name& name, const RRType& type,
// - Requested name is a delegation point (NS only but not at the zone
// apex - DNAME is ignored here as it redirects DNS names subordinate to
// the owner name - the owner name itself is not redirected.)
- const bool is_origin = (name == getOrigin());
WantedTypes final_types(FINAL_TYPES());
final_types.insert(type);
const FoundRRsets found = getRRsets(name.toText(), final_types,
- !is_origin, NULL,
- type == RRType::ANY());
+ NULL, type == RRType::ANY());
FindDNSSECContext dnssec_ctx(*this, options);
if (found.first) {
// Something found at the domain name. Look into it further to get
// the final result.
+ const bool is_origin = (name == getOrigin());
return (findOnNameResult(name, type, options, is_origin, found, NULL,
target, dnssec_ctx));
} else {
@@ -1021,7 +1008,7 @@ DatabaseClient::Finder::findNSEC3(const Name& name, bool recursive) {
// Now, we need to get the NSEC3 params from the apex and create the hash
// creator for it.
const FoundRRsets nsec3param(getRRsets(getOrigin().toText(),
- NSEC3PARAM_TYPES(), false));
+ NSEC3PARAM_TYPES()));
const FoundIterator param(nsec3param.second.find(RRType::NSEC3PARAM()));
if (!nsec3param.first || param == nsec3param.second.end()) {
// No NSEC3 params? :-(
@@ -1061,7 +1048,7 @@ DatabaseClient::Finder::findNSEC3(const Name& name, bool recursive) {
}
const FoundRRsets nsec3(getRRsets(hash + "." + otext, NSEC3_TYPES(),
- false, NULL, false, context));
+ NULL, false, context));
if (nsec3.first) {
// We found an exact match against the current label.
@@ -1086,8 +1073,8 @@ DatabaseClient::Finder::findNSEC3(const Name& name, bool recursive) {
arg(labels).arg(prevHash);
context = accessor_->getNSEC3Records(prevHash, zone_id_);
const FoundRRsets prev_nsec3(getRRsets(prevHash + "." + otext,
- NSEC3_TYPES(), false, NULL,
- false, context));
+ NSEC3_TYPES(), NULL, false,
+ context));
if (!prev_nsec3.first) {
isc_throw(DataSourceError, "Hash " + prevHash + " returned "
diff --git a/src/lib/datasrc/database.h b/src/lib/datasrc/database.h
index 8ad1c5b..65ddfcc 100644
--- a/src/lib/datasrc/database.h
+++ b/src/lib/datasrc/database.h
@@ -963,17 +963,14 @@ public:
///
/// \param name Which domain name should be scanned.
/// \param types List of types the caller is interested in.
- /// \param check_ns If this is set to true, it checks nothing lives
- /// together with NS record (with few little exceptions, like RRSIG
- /// or NSEC). This check is meant for non-apex NS records.
/// \param construct_name If this is NULL, the resulting RRsets have
- /// their name set to name. If it is not NULL, it overrides the name
- /// and uses this one (this can be used for wildcard synthesized
- /// records).
+ /// their name set to name. If it is not NULL, it overrides the
+ /// name and uses this one (this can be used for wildcard
+ /// synthesized records).
/// \param any If this is true, it records all the types, not only the
/// ones requested by types. It also puts a NULL pointer under the
- /// ANY type into the result, if it finds any RRs at all, to easy the
- /// identification of success.
+ /// ANY type into the result, if it finds any RRs at all, to easy
+ /// the identification of success.
/// \param srcContext This can be set to non-NULL value to override the
/// iterator context used for obtaining the data. This can be used,
/// for example, to get data from the NSEC3 namespace.
@@ -986,7 +983,7 @@ public:
/// \throw DataSourceError If there's a low-level error with the
/// database or the database contains bad data.
FoundRRsets getRRsets(const std::string& name,
- const WantedTypes& types, bool check_ns,
+ const WantedTypes& types,
const std::string* construct_name = NULL,
bool any = false,
DatabaseAccessor::IteratorContextPtr srcContext =
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index c96a1d2..5f18283 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -168,13 +168,16 @@ const char* const TEST_RECORDS[][5] = {
{"child.insecdelegation.example.org.", "DS", "3600", "", "DS 5 3 3600 "
"20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE"},
- // Broken NS
+ // Delegation NS and other ordinary type of RR coexist at the same
+ // name. This is deviant (except for some special cases like the other
+ // RR could be used for addressing the NS name), but as long as the
+ // other records are hidden behind the delegation for normal queries
+ // it's not necessarily harmful. (so "broken" may be too strong, but we
+ // keep the name since it could be in a chain of sorted names for DNSSEC
+ // processing and renaming them may have other bad effects for tests).
{"brokenns1.example.org.", "A", "3600", "", "192.0.2.1"},
{"brokenns1.example.org.", "NS", "3600", "", "ns.example.com."},
- {"brokenns2.example.org.", "NS", "3600", "", "ns.example.com."},
- {"brokenns2.example.org.", "A", "3600", "", "192.0.2.1"},
-
// Now double DNAME, to test failure mode
{"baddname.example.org.", "DNAME", "3600", "", "dname1.example.com."},
{"baddname.example.org.", "DNAME", "3600", "", "dname2.example.com."},
@@ -2202,15 +2205,23 @@ TYPED_TEST(DatabaseClientTest, findDelegation) {
ZoneFinder::FIND_DEFAULT),
DataSourceError);
- // Broken NS - it lives together with something else
- EXPECT_THROW(finder->find(isc::dns::Name("brokenns1.example.org."),
- this->qtype_,
- ZoneFinder::FIND_DEFAULT),
- DataSourceError);
- EXPECT_THROW(finder->find(isc::dns::Name("brokenns2.example.org."),
- this->qtype_,
- ZoneFinder::FIND_DEFAULT),
- DataSourceError);
+ // NS and other type coexist: deviant and not necessarily harmful.
+ // It should normally just result in DELEGATION; if GLUE_OK is specified,
+ // the other RR should be visible.
+ this->expected_rdatas_.clear();
+ this->expected_rdatas_.push_back("ns.example.com");
+ doFindTest(*finder, Name("brokenns1.example.org"), this->qtype_,
+ RRType::NS(), this->rrttl_, ZoneFinder::DELEGATION,
+ this->expected_rdatas_, this->empty_rdatas_,
+ ZoneFinder::RESULT_DEFAULT);
+
+ this->expected_rdatas_.clear();
+ this->expected_rdatas_.push_back("192.0.2.1");
+ doFindTest(*finder, Name("brokenns1.example.org"), this->qtype_,
+ this->qtype_, this->rrttl_, ZoneFinder::SUCCESS,
+ this->expected_rdatas_, this->empty_rdatas_,
+ ZoneFinder::RESULT_DEFAULT, Name("brokenns1.example.org"),
+ ZoneFinder::FIND_GLUE_OK);
}
TYPED_TEST(DatabaseClientTest, findDS) {
diff --git a/src/lib/datasrc/tests/zone_finder_context_unittest.cc b/src/lib/datasrc/tests/zone_finder_context_unittest.cc
index 50d409e..b3c9b2d 100644
--- a/src/lib/datasrc/tests/zone_finder_context_unittest.cc
+++ b/src/lib/datasrc/tests/zone_finder_context_unittest.cc
@@ -208,13 +208,6 @@ TEST_P(ZoneFinderContextTest, getAdditionalDelegation) {
TEST_P(ZoneFinderContextTest, getAdditionalDelegationAtZoneCut) {
// Similar to the previous case, but one of the NS addresses is at the
// zone cut.
-
- // XXX: the current database-based data source incorrectly rejects this
- // setup (see #1771)
- if (GetParam() == createSQLite3Client) {
- return;
- }
-
ZoneFinderContextPtr ctx = finder_->find(Name("www.b.example.org"),
RRType::SOA());
EXPECT_EQ(ZoneFinder::DELEGATION, ctx->code);
@@ -316,12 +309,6 @@ TEST_P(ZoneFinderContextTest, getAdditionalMX) {
}
TEST_P(ZoneFinderContextTest, getAdditionalMXAtZoneCut) {
- // XXX: the current database-based data source incorrectly rejects this
- // setup (see #1771)
- if (GetParam() == createSQLite3Client) {
- return;
- }
-
ZoneFinderContextPtr ctx = finder_->find(Name("mxatcut.example.org."),
RRType::MX());
EXPECT_EQ(ZoneFinder::SUCCESS, ctx->code);
diff --git a/src/lib/datasrc/zone.h b/src/lib/datasrc/zone.h
index bf4b1ea..c41a2c2 100644
--- a/src/lib/datasrc/zone.h
+++ b/src/lib/datasrc/zone.h
@@ -376,6 +376,16 @@ public:
/// RRsets for that name are searched just like the normal case;
/// otherwise, if the search has encountered a zone cut, \c DELEGATION
/// with the information of the highest zone cut will be returned.
+ /// Note: the term "glue" in the DNS protocol standard may sometimes
+ /// cause confusion: some people use this term strictly for an address
+ /// record (type AAAA or A) for the name used in the RDATA of an NS RR;
+ /// some others seem to give it broader flexibility. Nevertheless,
+ /// in this API the "GLUE OK" simply means the search by find() can
+ /// continue beyond a zone cut; the derived class implementation does
+ /// not have to, and should not, check whether the type is an address
+ /// record or whether the query name is pointed by some NS RR.
+ /// It's up to the caller with which definition of "glue" the search
+ /// result with this option should be used.
/// - \c FIND_DNSSEC Request that DNSSEC data (like NSEC, RRSIGs) are
/// returned with the answer. It is allowed for the data source to
/// include them even when not requested.
More information about the bind10-changes
mailing list