From b4466188150a50872bc3c426242bc7bba4c5f38d Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 5 Apr 2012 14:46:55 -0700 Subject: [PATCH] [1791] make sure DB iterator distinguishes 2 RRSIGs of different types covered. this could be a separate bug fix, but this branch would be easier to implement with this fix, so I chose to fix it here. --- src/lib/datasrc/database.cc | 35 +++++++++++++++++-- src/lib/datasrc/tests/database_unittest.cc | 40 ++++++++++++++++++++-- 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc index 08c7b26b31..22de3ffdb5 100644 --- a/src/lib/datasrc/database.cc +++ b/src/lib/datasrc/database.cc @@ -1013,7 +1013,21 @@ public: const Name name(name_str); const RRType rtype(rtype_str); RRsetPtr rrset(new RRset(name, class_, rtype, RRTTL(ttl))); - while (data_ready_ && name_ == name_str && rtype_str == rtype_) { + 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; + } + if (Name(name_) != name || !same_type) { + break; + } if (ttl_ != ttl) { if (ttl < ttl_) { ttl_ = ttl; @@ -1022,7 +1036,7 @@ public: LOG_WARN(logger, DATASRC_DATABASE_ITERATE_TTL_MISMATCH). arg(name_).arg(class_).arg(rtype_).arg(rrset->getTTL()); } - rrset->addRdata(rdata::createRdata(rtype, class_, rdata_)); + rrset->addRdata(rdata); getData(); if (separate_rrs_) { break; @@ -1034,6 +1048,23 @@ public: } private: + // Check two RDATA types are equivalent. Basically it's a trivial + // comparison, but if both are of RRSIG, we should also compare the types + // covered. + bool isSameType(RRType type1, ConstRdataPtr rdata1, + RRType type2, ConstRdataPtr rdata2) + { + if (type1 != type2) { + return (false); + } + if (type1 == RRType::RRSIG()) { + return (dynamic_cast(*rdata1).typeCovered() + == dynamic_cast(*rdata2). + typeCovered()); + } + return (true); + } + // Load next row of data void getData() { string data[DatabaseAccessor::COLUMN_COUNT]; diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc index 0e9c924cc7..9362a46f11 100644 --- a/src/lib/datasrc/tests/database_unittest.cc +++ b/src/lib/datasrc/tests/database_unittest.cc @@ -501,7 +501,7 @@ private: } // Return faked data for tests - switch (step ++) { + switch (step++) { case 0: data[DatabaseAccessor::NAME_COLUMN] = "example.org"; data[DatabaseAccessor::TYPE_COLUMN] = "A"; @@ -540,12 +540,30 @@ private: 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 7: + case 9: data[DatabaseAccessor::NAME_COLUMN] = "ttldiff.example.org"; data[DatabaseAccessor::TYPE_COLUMN] = "A"; data[DatabaseAccessor::TTL_COLUMN] = "600"; @@ -554,7 +572,7 @@ private: default: ADD_FAILURE() << "Request past the end of iterator context"; - case 8: + case 10: return (false); } } @@ -1389,6 +1407,22 @@ TYPED_TEST(DatabaseClientTest, iterator) { checkRRset(rrset, Name("x.example.org"), this->qclass_, RRType::AAAA(), RRTTL(300), this->expected_rdatas_); + rrset = it->getNextRRset(); + this->expected_rdatas_.clear(); + this->expected_rdatas_.push_back( + "A 5 3 3600 20000101000000 20000201000000 " + "12345 example.org. FAKEFAKEFAKE"); + checkRRset(rrset, Name("x.example.org"), this->qclass_, RRType::RRSIG(), + RRTTL(300), this->expected_rdatas_); + + rrset = it->getNextRRset(); + this->expected_rdatas_.clear(); + this->expected_rdatas_.push_back( + "AAAA 5 3 3600 20000101000000 20000201000000 " + "12345 example.org. FAKEFAKEFAKEFAKE"); + checkRRset(rrset, Name("x.example.org"), this->qclass_, RRType::RRSIG(), + RRTTL(300), this->expected_rdatas_); + rrset = it->getNextRRset(); ASSERT_NE(ConstRRsetPtr(), rrset); this->expected_rdatas_.clear(); -- 2.47.3