BIND 10 trac1775, updated. 868ecf071728924a23e3f0d3d8967cde5fbf8c30 [1775] ensure wildcard expanded RRsets are considered of same kind if they are.
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Mar 22 23:43:00 UTC 2012
The branch, trac1775 has been updated
via 868ecf071728924a23e3f0d3d8967cde5fbf8c30 (commit)
via 475284be04f902de8e20e1756bbfeb74d7f63779 (commit)
via 0eaac383a129e72f6a0d1236cbdf725520c63bf0 (commit)
from 40d727bf4c011c28575fb099fa8370591afa4a2a (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 868ecf071728924a23e3f0d3d8967cde5fbf8c30
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Thu Mar 22 16:35:40 2012 -0700
[1775] ensure wildcard expanded RRsets are considered of same kind if they are.
in the previous version, if one is returned from find() and the other is
from getAdditional() they are not recognized as of the same kind, because
the underlying pointers are different.
commit 475284be04f902de8e20e1756bbfeb74d7f63779
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Thu Mar 22 15:04:17 2012 -0700
[1775] cleanup: make sure wildcard and glue don't coexist.
findNode() should have ensured that, so assert() is used for this.
commit 0eaac383a129e72f6a0d1236cbdf725520c63bf0
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Thu Mar 22 14:52:49 2012 -0700
[1775] handled corner cases with wildcard expansion: empty or multiple nodes.
The previous version crashes if the wildcard node is empty. Also, the
previous version didn't work well if there were multiple wildcard matches
because the auxiliary tree changes as we add more nodes and stored
pointer can become invalid. The fix to this issue is a bit complicated;
I needed to make the process two-stage.
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/memory_datasrc.cc | 98 +++++++++++++++++---
src/lib/datasrc/rbtree.h | 5 +-
src/lib/datasrc/tests/testdata/contexttest.zone | 10 ++-
.../datasrc/tests/zone_finder_context_unittest.cc | 19 ++++-
4 files changed, 112 insertions(+), 20 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory_datasrc.cc b/src/lib/datasrc/memory_datasrc.cc
index 4a5b99c..a3a2110 100644
--- a/src/lib/datasrc/memory_datasrc.cc
+++ b/src/lib/datasrc/memory_datasrc.cc
@@ -93,6 +93,11 @@ const DomainNode::Flags WILD = DomainNode::FLAG_USER1;
// realistic to keep this flag update for all affected nodes, and we may
// have to reconsider the mechanism.
const DomainNode::Flags GLUE = DomainNode::FLAG_USER2;
+
+// This flag indicates the node is generated as a result of wildcard
+// expansion. In this implementation, this flag can be set only in
+// the separate auxiliary tree of ZoneData (see the structure description).
+const DomainNode::Flags WILD_EXPANDED = DomainNode::FLAG_USER3;
};
// Separate storage for NSEC3 RRs (and their RRSIGs). It's an STL map
@@ -711,11 +716,22 @@ private:
if (!glue_ok && additional.node_->getFlag(domain_flag::GLUE)) {
continue;
}
+ const bool wild_expanded =
+ additional.node_->getFlag(domain_flag::WILD_EXPANDED);
BOOST_FOREACH(const RRType& rrtype, requested_types) {
Domain::const_iterator found =
additional.node_->getData()->find(rrtype);
if (found != additional.node_->getData()->end()) {
- result.push_back(found->second);
+ // If the additional node was generated as a result of
+ // wildcard expansion, we return the underlying RRset,
+ // in case the caller has the same RRset but as a result
+ // of normal find() and needs to know they are of the same
+ // kind; otherwise we simply use the stored RBNodeRRset.
+ if (wild_expanded) {
+ result.push_back(found->second->getUnderlyingRRset());
+ } else {
+ result.push_back(found->second);
+ }
}
}
}
@@ -1421,8 +1437,11 @@ convertAndInsert(const DomainPair& rrset_item, DomainPtr dst_domain,
}
void
-addAdditional(RBNodeRRset* rrset, ZoneData* zone_data) {
+addAdditional(RBNodeRRset* rrset, ZoneData* zone_data,
+ vector<RBNodeRRset*>* wild_rrsets)
+{
RdataIteratorPtr rdata_iterator = rrset->getRdataIterator();
+ bool match_wild = false; // will be true if wildcard match is found
for (; !rdata_iterator->isLast(); rdata_iterator->next()) {
// For each domain name that requires additional section processing
// in each RDATA, search the tree for the name and remember it if
@@ -1430,7 +1449,6 @@ addAdditional(RBNodeRRset* rrset, ZoneData* zone_data) {
// child zone), mark the node as "GLUE", so we can selectively
// include/exclude them when we use it.
- DomainNode* node = NULL;
const Name& name = getAdditionalName(rrset->getType(),
rdata_iterator->getCurrent());
const ZoneData::FindMutableNodeResult result =
@@ -1440,7 +1458,7 @@ addAdditional(RBNodeRRset* rrset, ZoneData* zone_data) {
// We are not interested in anything but a successful match.
continue;
}
- node = result.node;
+ DomainNode* node = result.node;
assert(node != NULL);
if ((result.flags & ZoneData::FindNodeResult::FIND_ZONECUT) != 0 ||
(node->getFlag(DomainNode::FLAG_CALLBACK) &&
@@ -1456,21 +1474,67 @@ addAdditional(RBNodeRRset* rrset, ZoneData* zone_data) {
// should be pretty rare. On the other hand we won't have to worry
// about wildcard expansion in getAdditional, which is quite
// performance sensitive.
+ DomainNode* wildnode = NULL;
if ((result.flags & ZoneData::FindNodeResult::FIND_WILDCARD) != 0) {
- DomainNode* wildnode = NULL;
- zone_data->getAuxWildDomains().insert(name, &wildnode);
- if (wildnode->isEmpty()) {
+ // Wildcard and glue shouldn't coexist. Make it sure here.
+ assert(!node->getFlag(domain_flag::GLUE));
+
+ if (zone_data->getAuxWildDomains().insert(name, &wildnode)
+ == DomainTree::SUCCESS) {
+ // If we first insert the node, copy the RRsets. If the
+ // original node was empty, we add empty data so
+ // addWildAdditional() can get an exactmatch for this name.
DomainPtr dst_domain(new Domain);
- for_each(node->getData()->begin(), node->getData()->end(),
- boost::bind(convertAndInsert, _1, dst_domain, &name));
+ if (!node->isEmpty()) {
+ for_each(node->getData()->begin(), node->getData()->end(),
+ boost::bind(convertAndInsert, _1, dst_domain,
+ &name));
+ }
wildnode->setData(dst_domain);
+ // Mark the node as "wildcard expanded" so it can be
+ // distinguished at lookup time.
+ wildnode->setFlag(domain_flag::WILD_EXPANDED);
}
+ match_wild = true;
node = wildnode;
}
- // Note that node may be empty. We should keep it in the list
- // in case we dynamically update the tree and it becomes non empty
- // (which is not supported yet)
- rrset->addAdditionalNode(AdditionalNodeInfo(node));
+
+ // If this name wasn't subject to wildcard substitution, we can add
+ // the additional information to the RRset now; otherwise I'll defer
+ // it until the entire auxiliary tree is built (pointers may be
+ // invalidated as we build it).
+ if (wildnode == NULL) {
+ // Note that node may be empty. We should keep it in the list
+ // in case we dynamically update the tree and it becomes non empty
+ // (which is not supported yet)
+ rrset->addAdditionalNode(AdditionalNodeInfo(node));
+ }
+ }
+
+ if (match_wild) {
+ wild_rrsets->push_back(rrset);
+ }
+}
+
+void
+addWildAdditional(RBNodeRRset* rrset, ZoneData* zone_data) {
+ // Similar to addAdditional(), but due to the first stage we know that
+ // the rrset should contain a name stored in the auxiliary trees, and
+ // that it should be found as an exact match. The RRset may have other
+ // names that didn't require wildcard expansion, but we can simply ignore
+ // them in this context. (Note that if we find an exact match in the
+ // auxiliary tree, it shouldn't be in the original zone; otherwise it
+ // shouldn't have resulted in wildcard in the first place).
+
+ RdataIteratorPtr rdata_iterator = rrset->getRdataIterator();
+ for (; !rdata_iterator->isLast(); rdata_iterator->next()) {
+ const Name& name = getAdditionalName(rrset->getType(),
+ rdata_iterator->getCurrent());
+ DomainNode* wildnode = NULL;
+ if (zone_data->getAuxWildDomains().find(name, &wildnode) ==
+ DomainTree::EXACTMATCH) {
+ rrset->addAdditionalNode(AdditionalNodeInfo(wildnode));
+ }
}
}
}
@@ -1491,8 +1555,14 @@ InMemoryZoneFinder::load(const string& filename) {
// For each RRset in need_additionals, identify the corresponding
// RBnode for additional processing and associate it in the RRset.
+ // If some additional names in an RRset RDATA as additional need wildcard
+ // expansion, we'll remember them in a separate vector, and handle them
+ // with addWildAdditional.
+ vector<RBNodeRRset*> wild_additionals;
for_each(need_additionals.begin(), need_additionals.end(),
- boost::bind(addAdditional, _1, tmp.get()));
+ boost::bind(addAdditional, _1, tmp.get(), &wild_additionals));
+ for_each(wild_additionals.begin(), wild_additionals.end(),
+ boost::bind(addWildAdditional, _1, tmp.get()));
// If the zone is NSEC3-signed, check if it has NSEC3PARAM
if (tmp->nsec3_data_) {
diff --git a/src/lib/datasrc/rbtree.h b/src/lib/datasrc/rbtree.h
index 161affb..1200f58 100644
--- a/src/lib/datasrc/rbtree.h
+++ b/src/lib/datasrc/rbtree.h
@@ -124,7 +124,8 @@ public:
enum Flags {
FLAG_CALLBACK = 1, ///< Callback enabled. See \ref callback
FLAG_USER1 = 0x80000000U, ///< Application specific flag
- FLAG_USER2 = 0x40000000U ///< Application specific flag
+ FLAG_USER2 = 0x40000000U, ///< Application specific flag
+ FLAG_USER3 = 0x20000000U ///< Application specific flag
};
private:
// Some flag values are expected to be used for internal purposes
@@ -133,7 +134,7 @@ private:
// explicitly defined in \c Flags. This constant represents all
// such flags.
static const uint32_t SETTABLE_FLAGS = (FLAG_CALLBACK | FLAG_USER1 |
- FLAG_USER2);
+ FLAG_USER2 | FLAG_USER3);
public:
diff --git a/src/lib/datasrc/tests/testdata/contexttest.zone b/src/lib/datasrc/tests/testdata/contexttest.zone
index 425866a..e499e0d 100644
--- a/src/lib/datasrc/tests/testdata/contexttest.zone
+++ b/src/lib/datasrc/tests/testdata/contexttest.zone
@@ -1,7 +1,7 @@
;; test zone file used for ZoneFinderContext tests.
;; RRSIGs are (obviouslly) faked ones for testing.
-example.org. 3600 IN SOA ns1.example.org. bugs.x.w.example.org. 56 3600 300 3600000 3600
+example.org. 3600 IN SOA ns1.example.org. bugs.x.w.example.org. 67 3600 300 3600000 3600
example.org. 3600 IN NS ns1.example.org.
example.org. 3600 IN NS ns2.example.org.
example.org. 3600 IN MX 1 mx1.example.org.
@@ -57,9 +57,15 @@ d.example.org. 3600 IN NS ns1.example.org.
foo.ns.empty.example.org. 3600 IN A 192.0.2.13
bar.ns.empty.example.org. 3600 IN A 192.0.2.14
-;; delegation; the NS name matches a wildcard (and there's no exact match)
+;; delegation; the NS name matches a wildcard (and there's no exact
+;; match). One of the NS names matches an empty wildcard node, for
+;; which no additional record should be provided (or any other
+;; disruption should happen).
e.example.org. 3600 IN NS ns.wild.example.org.
+e.example.org. 3600 IN NS ns.emptywild.example.org.
+e.example.org. 3600 IN NS ns2.example.org.
*.wild.example.org. 3600 IN A 192.0.2.15
+a.*.emptywild.example.org. 3600 IN AAAA 2001:db8::2
;; additional for an answer RRset (MX) as a result of wildcard
;; expansion
diff --git a/src/lib/datasrc/tests/zone_finder_context_unittest.cc b/src/lib/datasrc/tests/zone_finder_context_unittest.cc
index 47b1f99..cb48e7e 100644
--- a/src/lib/datasrc/tests/zone_finder_context_unittest.cc
+++ b/src/lib/datasrc/tests/zone_finder_context_unittest.cc
@@ -262,13 +262,28 @@ TEST_P(ZoneFinderContextTest, getAdditionalDelegationWithEmptyName) {
}
TEST_P(ZoneFinderContextTest, getAdditionalDelegationWithWild) {
- // The NS name needs to be expanded by a wildcard.
+ // An NS name needs to be expanded by a wildcard. Another NS name
+ // also matches a wildcard, but it's an empty node, so there's no
+ // corresponding additional RR. The other NS name isn't subject to
+ // wildcard expansion, which shouldn't cause any disruption.
ZoneFinderContextPtr ctx = finder_->find(Name("www.e.example.org"),
RRType::AAAA());
EXPECT_EQ(ZoneFinder::DELEGATION, ctx->code);
ctx->getAdditional(REQUESTED_BOTH, result_sets_);
- rrsetsCheck("ns.wild.example.org. 3600 IN A 192.0.2.15\n",
+ rrsetsCheck("ns.wild.example.org. 3600 IN A 192.0.2.15\n"
+ "ns2.example.org. 3600 IN A 192.0.2.2\n",
result_sets_.begin(), result_sets_.end());
+
+ // ns.wild.example.org/A (expanded from a wildcard) should be considered
+ // the same kind, whether it's a direct result of find() or a result of
+ // getAdditional().
+ ctx = finder_->find(Name("ns.wild.example.org"), RRType::A());
+ EXPECT_EQ(ZoneFinder::SUCCESS, ctx->code);
+ for (vector<ConstRRsetPtr>::const_iterator it = result_sets_.begin();
+ it != result_sets_.end(); ++it) {
+ const bool same_kind = (*it)->isSameKind(*ctx->rrset);
+ EXPECT_EQ((*it)->getName() == ctx->rrset->getName(), same_kind);
+ }
}
TEST_P(ZoneFinderContextTest, getAdditionalDelegationForWild) {
More information about the bind10-changes
mailing list