BIND 10 trac1791, updated. 7610a7bacea56b07ae06975ee771d010846addd2 [1791] updated the log description for mismatched TTLs.
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Apr 9 20:29:07 UTC 2012
The branch, trac1791 has been updated
via 7610a7bacea56b07ae06975ee771d010846addd2 (commit)
via 0996aa92a103e7af59bf1ae9c501f5feb2a5894c (commit)
via f1f0bc00441057e7050241415ee0367a09c35032 (commit)
via fb9b8cd9f1200dcd6d7146a45fdbfd2c7675be56 (commit)
from 9858d6dd9af2640765266d6b99056e93dc368277 (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 7610a7bacea56b07ae06975ee771d010846addd2
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Apr 9 13:27:23 2012 -0700
[1791] updated the log description for mismatched TTLs.
it didn't seem correct to say "it's not allowed on the wire". this
restriction is more about the definition of the RRset concept, rather than
what we'd see in the wire. also added a reference to the relevant RFC (2181).
commit 0996aa92a103e7af59bf1ae9c501f5feb2a5894c
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Apr 9 13:21:08 2012 -0700
[1791] more cleanup: remove variable 'rdata' and make it a member variable.
this way we can also avoid creating the same RDATA twice. textual row
variables were renamed to *_txt_ for consistency.
commit f1f0bc00441057e7050241415ee0367a09c35032
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Apr 9 13:05:36 2012 -0700
[1791] fixed the bug in database iterator wrt RRs of mixed TTLs.
the previous version didn't always reset it to the smallest TTL; the new test
added in the previous commit uncovered it. this version passed that test.
also made some cleanups in the code reducing temporary variables for better
readability.
commit fb9b8cd9f1200dcd6d7146a45fdbfd2c7675be56
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Apr 9 11:10:49 2012 -0700
[1791] added a database iterator test that would uncover an (unrelated) bug.
it checks the case for RRsets with the TTL of the first RR is later than that
of the second. The original implementation has a bug in this case, but the
previous test was insufficient and fail to discover it.
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/database.cc | 76 ++++++++++--------
src/lib/datasrc/datasrc_messages.mes | 10 ++-
src/lib/datasrc/tests/database_unittest.cc | 120 +++++++++++-----------------
3 files changed, 96 insertions(+), 110 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index 22de3ff..107b656 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -1005,41 +1005,49 @@ public:
// At the end of zone
accessor_->commit();
ready_ = false;
- LOG_DEBUG(logger, DBG_TRACE_DETAILED,
- DATASRC_DATABASE_ITERATE_END);
+ LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_ITERATE_END);
return (ConstRRsetPtr());
}
- const string name_str(name_), rtype_str(rtype_), ttl(ttl_);
- const Name name(name_str);
- const RRType rtype(rtype_str);
- RRsetPtr rrset(new RRset(name, class_, rtype, RRTTL(ttl)));
- const ConstRdataPtr rdata_base =
- rdata::createRdata(rtype, class_, rdata_);
- ConstRdataPtr rdata;
- while (data_ready_) {
- bool same_type = true;
- if (rdata) { // for subsequent data, replace it with the new RDATA.
- const RRType next_rtype(rtype_);
- rdata = rdata::createRdata(next_rtype, class_, rdata_);
- same_type = isSameType(rtype, rdata_base, next_rtype, rdata);
- } else {
- rdata = rdata_base;
+ const RRType rtype(rtype_txt_);
+ RRsetPtr rrset(new RRset(Name(name_txt_), class_, rtype,
+ RRTTL(ttl_txt_)));
+ // For the first RR, rdata_ is null, so we need to create it here.
+ // After that, rdata_ will be updated in the while loop below.
+ if (!rdata_) {
+ rdata_ = rdata::createRdata(rtype, class_, rdata_txt_);
+ }
+ const ConstRdataPtr rdata_base = rdata_; // remember it for comparison
+ while (true) {
+ // Extend the RRset with the new RDATA.
+ rrset->addRdata(rdata_);
+
+ // Retrieve the next record from the database. If we reach the
+ // end of the zone, done; if we were requested to separate all RRs,
+ // just remember this record and return the single RR.
+ getData();
+ if (separate_rrs_ || !data_ready_) {
+ break;
}
- if (Name(name_) != name || !same_type) {
+
+ // Check if the next record belongs to the same RRset. If not,
+ // we are done. The next RDATA is stored in rdata_, which is used
+ // within this loop (if it belongs to the same RRset) or in the
+ // next call.
+ const RRType next_rtype(rtype_txt_);
+ rdata_ = rdata::createRdata(next_rtype, class_, rdata_txt_);
+ if (Name(name_txt_) != rrset->getName() ||
+ !isSameType(rtype, rdata_base, next_rtype, rdata_)) {
break;
}
- if (ttl_ != ttl) {
- if (ttl < ttl_) {
- ttl_ = ttl;
- rrset->setTTL(RRTTL(ttl));
+
+ // Adjust TTL if necessary
+ const RRTTL next_ttl(ttl_txt_);
+ if (next_ttl != rrset->getTTL()) {
+ if (next_ttl < rrset->getTTL()) {
+ rrset->setTTL(next_ttl);
}
LOG_WARN(logger, DATASRC_DATABASE_ITERATE_TTL_MISMATCH).
- arg(name_).arg(class_).arg(rtype_).arg(rrset->getTTL());
- }
- rrset->addRdata(rdata);
- getData();
- if (separate_rrs_) {
- break;
+ arg(name_txt_).arg(class_).arg(rtype).arg(rrset->getTTL());
}
}
LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_ITERATE_NEXT).
@@ -1069,10 +1077,10 @@ private:
void getData() {
string data[DatabaseAccessor::COLUMN_COUNT];
data_ready_ = context_->getNext(data);
- name_ = data[DatabaseAccessor::NAME_COLUMN];
- rtype_ = data[DatabaseAccessor::TYPE_COLUMN];
- ttl_ = data[DatabaseAccessor::TTL_COLUMN];
- rdata_ = data[DatabaseAccessor::RDATA_COLUMN];
+ name_txt_ = data[DatabaseAccessor::NAME_COLUMN];
+ rtype_txt_ = data[DatabaseAccessor::TYPE_COLUMN];
+ ttl_txt_ = data[DatabaseAccessor::TTL_COLUMN];
+ rdata_txt_= data[DatabaseAccessor::RDATA_COLUMN];
}
// The dedicated accessor
@@ -1086,7 +1094,9 @@ private:
// Status
bool ready_, data_ready_;
// Data of the next row
- string name_, rtype_, rdata_, ttl_;
+ string name_txt_, rtype_txt_, rdata_txt_, ttl_txt_;
+ // RDATA of the next row; created from rdata_txt_.
+ ConstRdataPtr rdata_;
// Whether to modify differing TTL values, or treat a different TTL as
// a different RRset
bool separate_rrs_;
diff --git a/src/lib/datasrc/datasrc_messages.mes b/src/lib/datasrc/datasrc_messages.mes
index ef46cb5..3801a41 100644
--- a/src/lib/datasrc/datasrc_messages.mes
+++ b/src/lib/datasrc/datasrc_messages.mes
@@ -145,10 +145,12 @@ While iterating through the zone, the program extracted next RRset from it.
The name and RRtype of the RRset is indicated in the message.
% DATASRC_DATABASE_ITERATE_TTL_MISMATCH TTL values differ for RRs of %1/%2/%3, setting to %4
-While iterating through the zone, the time to live for RRs of the given RRset
-were found to be different. This isn't allowed on the wire and is considered
-an error, so we set it to the lowest value we found (but we don't modify the
-database). The data in database should be checked and fixed.
+While iterating through the zone, the time to live for RRs of the
+given RRset were found to be different. Since an RRset cannot have
+multiple TTLs, we set it to the lowest value we found (but we don't
+modify the database). This is what the client would do when such RRs
+were given in a DNS response according to RFC2181. The data in
+database should be checked and fixed.
% DATASRC_DATABASE_JOURNALREADER_END %1/%2 on %3 from %4 to %5
This is a debug message indicating that the program (successfully)
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index 9362a46..bf4bb11 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -501,80 +501,46 @@ private:
}
// Return faked data for tests
- switch (step++) {
- case 0:
- data[DatabaseAccessor::NAME_COLUMN] = "example.org";
- data[DatabaseAccessor::TYPE_COLUMN] = "A";
- data[DatabaseAccessor::TTL_COLUMN] = "3600";
- data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.1";
- return (true);
- case 1:
- data[DatabaseAccessor::NAME_COLUMN] = "example.org";
- data[DatabaseAccessor::TYPE_COLUMN] = "SOA";
- data[DatabaseAccessor::TTL_COLUMN] = "3600";
- data[DatabaseAccessor::RDATA_COLUMN] = "ns1.example.org. admin.example.org. "
- "1234 3600 1800 2419200 7200";
- return (true);
- case 2:
- data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
- data[DatabaseAccessor::TYPE_COLUMN] = "A";
- data[DatabaseAccessor::TTL_COLUMN] = "300";
- data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.1";
- return (true);
- case 3:
- data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
- data[DatabaseAccessor::TYPE_COLUMN] = "A";
- data[DatabaseAccessor::TTL_COLUMN] = "300";
- data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.2";
- return (true);
- case 4:
- data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
- data[DatabaseAccessor::TYPE_COLUMN] = "AAAA";
- data[DatabaseAccessor::TTL_COLUMN] = "300";
- data[DatabaseAccessor::RDATA_COLUMN] = "2001:db8::1";
- return (true);
- case 5:
- data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
- data[DatabaseAccessor::TYPE_COLUMN] = "AAAA";
- data[DatabaseAccessor::TTL_COLUMN] = "300";
- data[DatabaseAccessor::RDATA_COLUMN] = "2001:db8::2";
- return (true);
- case 6:
- data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
- data[DatabaseAccessor::TYPE_COLUMN] = "RRSIG";
- data[DatabaseAccessor::TTL_COLUMN] = "300";
- data[DatabaseAccessor::RDATA_COLUMN] =
- "A 5 3 3600 20000101000000 20000201000000 12345 "
- "example.org. FAKEFAKEFAKE";
- return (true);
- case 7:
- // RRSIG for the same owner name but for a different type
- // to cover. These two should be distinguished.
- data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
- data[DatabaseAccessor::TYPE_COLUMN] = "RRSIG";
- data[DatabaseAccessor::TTL_COLUMN] = "300";
- data[DatabaseAccessor::RDATA_COLUMN] =
- "AAAA 5 3 3600 20000101000000 20000201000000 12345 "
- "example.org. FAKEFAKEFAKEFAKE";
- return (true);
- case 8:
- data[DatabaseAccessor::NAME_COLUMN] = "ttldiff.example.org";
- data[DatabaseAccessor::TYPE_COLUMN] = "A";
- data[DatabaseAccessor::TTL_COLUMN] = "300";
- data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.1";
- return (true);
- case 9:
- data[DatabaseAccessor::NAME_COLUMN] = "ttldiff.example.org";
- data[DatabaseAccessor::TYPE_COLUMN] = "A";
- data[DatabaseAccessor::TTL_COLUMN] = "600";
- data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.2";
- return (true);
- default:
- ADD_FAILURE() <<
- "Request past the end of iterator context";
- case 10:
- return (false);
+ // This is the sequence of zone data in the order of appearance
+ // in the returned sequence from this iterator.
+ typedef const char* ColumnText[4];
+ const ColumnText zone_data[] = {
+ // A couple of basic RRs at the zone origin.
+ {"example.org", "A", "3600", "192.0.2.1"},
+ {"example.org", "SOA", "3600", "ns1.example.org. "
+ "admin.example.org. 1234 3600 1800 2419200 7200"},
+ // RRsets sharing the same owner name with multiple RRs.
+ {"x.example.org", "A", "300", "192.0.2.1"},
+ {"x.example.org", "A", "300", "192.0.2.2"},
+ {"x.example.org", "AAAA", "300", "2001:db8::1"},
+ {"x.example.org", "AAAA", "300", "2001:db8::2"},
+ // RRSIGs. Covered types are different and these two should
+ // be distinguished.
+ {"x.example.org", "RRSIG", "300",
+ "A 5 3 3600 20000101000000 20000201000000 12345 "
+ "example.org. FAKEFAKEFAKE"},
+ {"x.example.org", "RRSIG", "300",
+ "AAAA 5 3 3600 20000101000000 20000201000000 12345 "
+ "example.org. FAKEFAKEFAKEFAKE"},
+ // Mixture of different TTLs. Covering both cases of small
+ // then large and large then small. In either case the smaller
+ // TTL should win.
+ {"ttldiff.example.org", "A", "300", "192.0.2.1"},
+ {"ttldiff.example.org", "A", "600", "192.0.2.2"},
+ {"ttldiff2.example.org", "AAAA", "600", "2001:db8::1"},
+ {"ttldiff2.example.org", "AAAA", "300", "2001:db8::2"}};
+ const size_t num_rrs = sizeof(zone_data) / sizeof(zone_data[0]);
+ if (step > num_rrs) {
+ ADD_FAILURE() << "Request past the end of iterator context";
+ } else if (step < num_rrs) {
+ data[DatabaseAccessor::NAME_COLUMN] = zone_data[step][0];
+ data[DatabaseAccessor::TYPE_COLUMN] = zone_data[step][1];
+ data[DatabaseAccessor::TTL_COLUMN] = zone_data[step][2];
+ data[DatabaseAccessor::RDATA_COLUMN] = zone_data[step][3];
+ ++step;
+ return (true);
}
+ return (false);
}
};
class EmptyIteratorContext : public IteratorContext {
@@ -1431,6 +1397,14 @@ TYPED_TEST(DatabaseClientTest, iterator) {
checkRRset(rrset, Name("ttldiff.example.org"), this->qclass_, RRType::A(),
RRTTL(300), this->expected_rdatas_);
+ rrset = it->getNextRRset();
+ ASSERT_NE(ConstRRsetPtr(), rrset);
+ this->expected_rdatas_.clear();
+ this->expected_rdatas_.push_back("2001:db8::1");
+ this->expected_rdatas_.push_back("2001:db8::2");
+ checkRRset(rrset, Name("ttldiff2.example.org"), this->qclass_,
+ RRType::AAAA(), RRTTL(300), this->expected_rdatas_);
+
EXPECT_EQ(ConstRRsetPtr(), it->getNextRRset());
}
More information about the bind10-changes
mailing list