BIND 10 master, updated. bf86e0c9ea67c5ddd6e70ddec63a1f46d8c62630 [master] Add ChangeLog for #2440, #2441
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Mar 7 03:06:18 UTC 2013
The branch, master has been updated
via bf86e0c9ea67c5ddd6e70ddec63a1f46d8c62630 (commit)
via 0860ae366d73314446d4886a093f4e86e94863d4 (commit)
via 8ab5a4d30ab27be1f14082a2f40e2d7f53b5e512 (commit)
via 83d9d50018d7a2cf6b1850cfa732323e69c1d411 (commit)
via 1adbd6a03cb9827f87be0496d71718fb1c9b6cb2 (commit)
via a04f885fad995fb689bd064cf024ee60a71625de (commit)
via f23495620fafe83545866d6cdac59e3660365253 (commit)
via 28ebeea1c461508fbca3995e34509b2c63a102a2 (commit)
via e6750157521cadc40bbe57db8d1cdda8328effa9 (commit)
from 25201656d56854345b11d4a3d553655da2a43bfa (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 bf86e0c9ea67c5ddd6e70ddec63a1f46d8c62630
Author: Mukund Sivaraman <muks at isc.org>
Date: Thu Mar 7 08:02:32 2013 +0530
[master] Add ChangeLog for #2440, #2441
commit 0860ae366d73314446d4886a093f4e86e94863d4
Merge: 2520165 8ab5a4d
Author: Mukund Sivaraman <muks at isc.org>
Date: Thu Mar 7 08:00:28 2013 +0530
Merge branch 'trac2441'
-----------------------------------------------------------------------
Summary of changes:
ChangeLog | 10 +++
src/lib/datasrc/memory/rdataset.h | 9 ++-
src/lib/datasrc/memory/zone_data_loader.cc | 42 ++++++++-----
src/lib/datasrc/memory/zone_data_updater.cc | 44 ++++++++++----
.../datasrc/tests/memory/memory_client_unittest.cc | 64 ++++++++++++++------
.../testdata/example.org-duplicate-type-bad.zone | 5 +-
.../tests/memory/zone_data_updater_unittest.cc | 10 ++-
7 files changed, 128 insertions(+), 56 deletions(-)
-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index d6f5e8f..bdb35bf 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+585. [func] jinmei, muks
+ The zone data loader now accepts RRs in any order during load.
+ Before it used to reject adding non-consecutive RRsets. It
+ expected records for a single owner name and its type to be
+ grouped together. These restrictions are now removed. It now also
+ suppresses any duplicate RRs in the zone file when loading them
+ into memory.
+ (Trac #2440, git 232307060189c47285121f696d4efb206f632432)
+ (Trac #2441, git 0860ae366d73314446d4886a093f4e86e94863d4)
+
584. [bug] jinmei
Fixed build failure with Boost 1.53 (and probably higher) in the
internal utility library. Note that with -Werror it may still
diff --git a/src/lib/datasrc/memory/rdataset.h b/src/lib/datasrc/memory/rdataset.h
index 250af93..58201b1 100644
--- a/src/lib/datasrc/memory/rdataset.h
+++ b/src/lib/datasrc/memory/rdataset.h
@@ -121,12 +121,15 @@ public:
/// returns a pointer to it.
///
/// If the optional \c old_rdataset parameter is set to non NULL,
- /// The given \c RdataSet, RRset, RRSIG will be merged in the new
- /// \c Rdataset object: the new object contain the union set of all
+ /// the given \c RdataSet, RRset, RRSIG will be merged in the new
+ /// \c Rdataset object: the new object will contain the union set of all
/// RDATA and RRSIGs contained in these. Obviously \c old_rdataset
- /// was previously generated for the same RRClass and RRType as those
+ /// must be previously generated for the same RRClass and RRType as those
/// for the given RRsets; it's the caller's responsibility to ensure
/// this condition. If it's not met the result will be undefined.
+ /// No reference to \c old_rdataset is maintained in the newly
+ /// returned \c RdataSet, so if the caller does not need \c
+ /// old_rdataset anymore, it may be freed by the caller.
///
/// In both cases, this method ensures the stored RDATA and RRSIG are
/// unique. Any duplicate data (in the sense of the comparison in the
diff --git a/src/lib/datasrc/memory/zone_data_loader.cc b/src/lib/datasrc/memory/zone_data_loader.cc
index e3c9c3f..de3a749 100644
--- a/src/lib/datasrc/memory/zone_data_loader.cc
+++ b/src/lib/datasrc/memory/zone_data_loader.cc
@@ -50,14 +50,15 @@ typedef boost::function<void(isc::dns::ConstRRsetPtr)> LoadCallback;
// A helper internal class for \c loadZoneData(). make it non-copyable
// to avoid accidental copy.
//
-// The current internal implementation expects that both a normal
-// (non RRSIG) RRset and (when signed) its RRSIG are added at once.
-// Also in the current implementation, the input sequence of RRsets
-// are grouped with their owner name (so once a new owner name is encountered,
-// no subsequent RRset has the previous owner name), but the ordering
-// in the same group is not fixed. So we hold all RRsets of the same
-// owner name in node_rrsets_ and node_rrsigsets_, and add the matching
-// pairs of RRsets to the zone when we see a new owner name.
+// The current internal implementation no longer expects that both a
+// normal (non RRSIG) RRset and (when signed) its RRSIG are added at
+// once, but we do that here anyway to avoid merging RdataSets every
+// single time which can be inefficient.
+//
+// We hold all RRsets of the same owner name in node_rrsets_ and
+// node_rrsigsets_, and add the matching pairs of RRsets to the zone
+// when we see a new owner name. We do this to limit the size of
+// NodeRRsets below. However, RRsets can occur in any order.
//
// The caller is responsible for adding the RRsets of the last group
// in the input sequence by explicitly calling flushNodeRRsets() at the
@@ -86,6 +87,7 @@ private:
private:
NodeRRsets node_rrsets_;
NodeRRsets node_rrsigsets_;
+ std::vector<isc::dns::ConstRRsetPtr> non_consecutive_rrsets_;
ZoneDataUpdater updater_;
};
@@ -93,22 +95,20 @@ void
ZoneDataLoader::addFromLoad(const ConstRRsetPtr& rrset) {
// If we see a new name, flush the temporary holders, adding the
// pairs of RRsets and RRSIGs of the previous name to the zone.
- if ((!node_rrsets_.empty() || !node_rrsigsets_.empty()) &&
+ if ((!node_rrsets_.empty() || !node_rrsigsets_.empty() ||
+ !non_consecutive_rrsets_.empty()) &&
(getCurrentName() != rrset->getName())) {
flushNodeRRsets();
}
- // Store this RRset until it can be added to the zone. The current
- // implementation requires RRs of the same RRset should be added at
- // once, so we check the "duplicate" here.
+ // Store this RRset until it can be added to the zone. If an rrtype
+ // that's already been seen is found, queue it in a different vector
+ // to be merged later.
const bool is_rrsig = rrset->getType() == RRType::RRSIG();
NodeRRsets& node_rrsets = is_rrsig ? node_rrsigsets_ : node_rrsets_;
const RRType& rrtype = is_rrsig ? getCoveredType(rrset) : rrset->getType();
if (!node_rrsets.insert(NodeRRsetsVal(rrtype, rrset)).second) {
- isc_throw(ZoneDataUpdater::AddError,
- "Duplicate add of the same type of"
- << (is_rrsig ? " RRSIG" : "") << " RRset: "
- << rrset->getName() << "/" << rrtype);
+ non_consecutive_rrsets_.insert(non_consecutive_rrsets_.begin(), rrset);
}
if (rrset->getRRsig()) {
@@ -137,8 +137,18 @@ ZoneDataLoader::flushNodeRRsets() {
updater_.add(ConstRRsetPtr(), val.second);
}
+ // Add any non-consecutive rrsets too.
+ BOOST_FOREACH(ConstRRsetPtr rrset, non_consecutive_rrsets_) {
+ if (rrset->getType() == RRType::RRSIG()) {
+ updater_.add(ConstRRsetPtr(), rrset);
+ } else {
+ updater_.add(rrset, ConstRRsetPtr());
+ }
+ }
+
node_rrsets_.clear();
node_rrsigsets_.clear();
+ non_consecutive_rrsets_.clear();
}
const Name&
diff --git a/src/lib/datasrc/memory/zone_data_updater.cc b/src/lib/datasrc/memory/zone_data_updater.cc
index 5bde6d4..4d7e7e0 100644
--- a/src/lib/datasrc/memory/zone_data_updater.cc
+++ b/src/lib/datasrc/memory/zone_data_updater.cc
@@ -270,8 +270,12 @@ ZoneDataUpdater::addNSEC3(const Name& name, const ConstRRsetPtr rrset,
ZoneNode* node;
nsec3_data->insertName(mem_sgmt_, name, &node);
- RdataSet* rdataset = RdataSet::create(mem_sgmt_, encoder_, rrset, rrsig);
- RdataSet* old_rdataset = node->setData(rdataset);
+ // Create a new RdataSet, merging any existing NSEC3 data for this
+ // name.
+ RdataSet* old_rdataset = node->getData();
+ RdataSet* rdataset = RdataSet::create(mem_sgmt_, encoder_, rrset, rrsig,
+ old_rdataset);
+ old_rdataset = node->setData(rdataset);
if (old_rdataset != NULL) {
RdataSet::destroy(mem_sgmt_, old_rdataset, rrclass_);
}
@@ -298,16 +302,34 @@ ZoneDataUpdater::addRdataSet(const Name& name, const RRType& rrtype,
contextCheck(*rrset, rdataset_head);
}
- if (RdataSet::find(rdataset_head, rrtype, true) != NULL) {
- isc_throw(AddError,
- "RRset of the type already exists: "
- << name << " (type: " << rrtype << ")");
- }
-
+ // Create a new RdataSet, merging any existing data for this
+ // type.
+ RdataSet* old_rdataset = RdataSet::find(rdataset_head, rrtype, true);
RdataSet* rdataset_new = RdataSet::create(mem_sgmt_, encoder_,
- rrset, rrsig);
- rdataset_new->next = rdataset_head;
- node->setData(rdataset_new);
+ rrset, rrsig, old_rdataset);
+ if (old_rdataset == NULL) {
+ // There is no existing RdataSet. Prepend the new RdataSet
+ // to the list.
+ rdataset_new->next = rdataset_head;
+ node->setData(rdataset_new);
+ } else {
+ // Replace the old RdataSet in the list with the newly
+ // created one, and destroy the old one.
+ for (RdataSet* cur = rdataset_head, *prev = NULL;
+ cur != NULL;
+ prev = cur, cur = cur->getNext()) {
+ if (cur == old_rdataset) {
+ rdataset_new->next = cur->getNext();
+ if (prev == NULL) {
+ node->setData(rdataset_new);
+ } else {
+ prev->next = rdataset_new;
+ }
+ break;
+ }
+ }
+ RdataSet::destroy(mem_sgmt_, old_rdataset, rrclass_);
+ }
// Ok, we just put it in.
diff --git a/src/lib/datasrc/tests/memory/memory_client_unittest.cc b/src/lib/datasrc/tests/memory/memory_client_unittest.cc
index 74e7917..5984de5 100644
--- a/src/lib/datasrc/tests/memory/memory_client_unittest.cc
+++ b/src/lib/datasrc/tests/memory/memory_client_unittest.cc
@@ -74,6 +74,7 @@ const char* rrset_data[] = {
const char* rrset_data_separated[] = {
"example.org. 3600 IN SOA ns1.example.org. bugs.x.w.example.org. "
"68 3600 300 3600000 3600",
+ "example.org. 3600 IN NS ns1.example.org.",
"a.example.org. 3600 IN A 192.168.0.1", // these two belong to the same
"a.example.org. 3600 IN A 192.168.0.2", // RRset, but are separated.
NULL
@@ -83,6 +84,7 @@ const char* rrset_data_separated[] = {
const char* rrset_data_sigseparated[] = {
"example.org. 3600 IN SOA ns1.example.org. bugs.x.w.example.org. "
"68 3600 300 3600000 3600",
+ "example.org. 3600 IN NS ns1.example.org.",
"a.example.org. 3600 IN A 192.168.0.1",
"a.example.org. 3600 IN RRSIG A 5 3 3600 20150420235959 20051021000000 "
"40430 example.org. FAKEFAKE",
@@ -261,13 +263,13 @@ TEST_F(MemoryClientTest, loadFromIterator) {
EXPECT_TRUE(rrset);
EXPECT_EQ(RRType::NS(), rrset->getType());
- // RRType::MX() RRset
+ // RRType::A() RRset
rrset = iterator->getNextRRset();
EXPECT_TRUE(rrset);
EXPECT_EQ(RRType::MX(), rrset->getType());
EXPECT_EQ(1, rrset->getRRsigDataCount()); // this RRset is signed
- // RRType::A() RRset
+ // RRType::MX() RRset
rrset = iterator->getNextRRset();
EXPECT_TRUE(rrset);
EXPECT_EQ(RRType::A(), rrset->getType());
@@ -279,19 +281,17 @@ TEST_F(MemoryClientTest, loadFromIterator) {
// Iterating past the end should result in an exception
EXPECT_THROW(iterator->getNextRRset(), isc::Unexpected);
- // Loading the zone with an iterator separating RRs of the same RRset
- // will fail because the resulting sequence doesn't meet assumptions of
- // the (current) in-memory implementation.
- EXPECT_THROW(client_->load(Name("example.org"),
- *MockIterator::makeIterator(
- rrset_data_separated)),
- ZoneDataUpdater::AddError);
+ // Loading the zone with an iterator separating RRs of the same
+ // RRset should not fail. It is acceptable to load RRs of the same
+ // type again.
+ client_->load(Name("example.org"),
+ *MockIterator::makeIterator(
+ rrset_data_separated));
// Similar to the previous case, but with separated RRSIGs.
- EXPECT_THROW(client_->load(Name("example.org"),
- *MockIterator::makeIterator(
- rrset_data_sigseparated)),
- ZoneDataUpdater::AddError);
+ client_->load(Name("example.org"),
+ *MockIterator::makeIterator(
+ rrset_data_sigseparated));
// Emulating bogus iterator implementation that passes empty RRSIGs.
EXPECT_THROW(client_->load(Name("example.org"),
@@ -439,15 +439,41 @@ TEST_F(MemoryClientTest, loadReloadZone) {
}
TEST_F(MemoryClientTest, loadDuplicateType) {
- // This should not result in any exceptions:
+ // This should not result in any exceptions (multiple records of the
+ // same name, type are present, one after another in sequence).
client_->load(Name("example.org"),
TEST_DATA_DIR "/example.org-duplicate-type.zone");
- // This should throw:
- EXPECT_THROW(client_->load(Name("example.org"),
- TEST_DATA_DIR
- "/example.org-duplicate-type-bad.zone"),
- ZoneDataUpdater::AddError);
+ // This should not result in any exceptions (multiple records of the
+ // same name, type are present, but not one after another in
+ // sequence).
+ client_->load(Name("example.org"),
+ TEST_DATA_DIR
+ "/example.org-duplicate-type-bad.zone");
+
+ const ZoneData* zone_data =
+ client_->findZoneData(Name("example.org"));
+ EXPECT_NE(static_cast<const ZoneData*>(NULL), zone_data);
+
+ /* Check ns1.example.org */
+ const ZoneTree& tree = zone_data->getZoneTree();
+ const ZoneNode* node;
+ ZoneTree::Result zresult(tree.find(Name("ns1.example.org"), &node));
+ EXPECT_EQ(ZoneTree::EXACTMATCH, zresult);
+
+ const RdataSet* set = node->getData();
+ EXPECT_NE(static_cast<const RdataSet*>(NULL), set);
+ EXPECT_EQ(RRType::AAAA(), set->type);
+
+ set = set->getNext();
+ EXPECT_NE(static_cast<const RdataSet*>(NULL), set);
+ EXPECT_EQ(RRType::A(), set->type);
+ // 192.168.0.1 and 192.168.0.2
+ EXPECT_EQ(2, set->getRdataCount());
+
+ set = set->getNext();
+ EXPECT_EQ(static_cast<const RdataSet*>(NULL), set);
+
// Teardown checks for memory segment leaks
}
diff --git a/src/lib/datasrc/tests/memory/testdata/example.org-duplicate-type-bad.zone b/src/lib/datasrc/tests/memory/testdata/example.org-duplicate-type-bad.zone
index 06c7dff..1198934 100644
--- a/src/lib/datasrc/tests/memory/testdata/example.org-duplicate-type-bad.zone
+++ b/src/lib/datasrc/tests/memory/testdata/example.org-duplicate-type-bad.zone
@@ -1,4 +1,7 @@
-example.org. 3600 IN SOA ns1.example.org. bugs.x.w.example.org. 77 3600 300 3600000 3600
+example.org. 3600 IN SOA ns1.example.org. bugs.x.w.example.org. 78 3600 300 3600000 3600
+example.org. 3600 IN NS ns1.example.org.
+example.org. 3600 IN NS ns2.example.org.
ns1.example.org. 3600 IN A 192.168.0.1
ns1.example.org. 3600 IN AAAA ::1
+someother.example.org. 3600 IN AAAA ::1
ns1.example.org. 3600 IN A 192.168.0.2
diff --git a/src/lib/datasrc/tests/memory/zone_data_updater_unittest.cc b/src/lib/datasrc/tests/memory/zone_data_updater_unittest.cc
index 93ca0c9..6399923 100644
--- a/src/lib/datasrc/tests/memory/zone_data_updater_unittest.cc
+++ b/src/lib/datasrc/tests/memory/zone_data_updater_unittest.cc
@@ -111,12 +111,10 @@ TEST_F(ZoneDataUpdaterTest, rrsigOnly) {
EXPECT_EQ(0, rdset->getRdataCount());
EXPECT_EQ(1, rdset->getSigRdataCount());
- // The RRSIG covering A prohibits an actual A RRset from being added.
- // This should be loosened in future version, but we check the current
- // behavior.
- EXPECT_THROW(updater_->add(
- textToRRset("www.example.org. 3600 IN A 192.0.2.1"),
- ConstRRsetPtr()), ZoneDataUpdater::AddError);
+ // The RRSIG covering A must not prohibit an actual A RRset from
+ // being added later.
+ updater_->add(textToRRset("www.example.org. 3600 IN A 192.0.2.1"),
+ ConstRRsetPtr());
// The special "wildcarding" node mark should be added for the RRSIG-only
// case, too.
More information about the bind10-changes
mailing list