From: JINMEI Tatuya Date: Mon, 10 Dec 2012 22:54:20 +0000 (-0800) Subject: [2470] handled some advanced/unusual cases of RRCollator. X-Git-Tag: bind10-1.0.0-beta-release~15^2~16 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0585f306eca6145a94aa8af700bbf64f49207d15;p=thirdparty%2Fkea.git [2470] handled some advanced/unusual cases of RRCollator. --- diff --git a/src/lib/dns/rrcollator.cc b/src/lib/dns/rrcollator.cc index f5924c2b49..1d30ce6e39 100644 --- a/src/lib/dns/rrcollator.cc +++ b/src/lib/dns/rrcollator.cc @@ -13,6 +13,7 @@ // PERFORMANCE OF THIS SOFTWARE. #include +#include #include #include #include @@ -22,8 +23,11 @@ #include +#include + namespace isc { namespace dns { +using namespace rdata; class RRCollator::Impl { public: @@ -31,24 +35,47 @@ public: void addRR(const Name& name, const RRClass& rrclass, const RRType& rrtype, const RRTTL& rrttl, - const rdata::RdataPtr& rdata); + const RdataPtr& rdata); RRsetPtr current_rrset_; AddRRsetCallback callback_; }; +namespace { +inline bool +isSameType(RRType type1, const ConstRdataPtr& rdata1, + const ConstRRsetPtr& rrset) +{ + if (type1 != rrset->getType()) { + return (false); + } + if (type1 == RRType::RRSIG()) { + RdataIteratorPtr rit = rrset->getRdataIterator(); + return (dynamic_cast(*rdata1).typeCovered() + == dynamic_cast( + rit->getCurrent()).typeCovered()); + } + return (true); +} +} + void RRCollator::Impl::addRR(const Name& name, const RRClass& rrclass, const RRType& rrtype, const RRTTL& rrttl, - const rdata::RdataPtr& rdata) + const RdataPtr& rdata) { - if (current_rrset_ && current_rrset_->getType() != rrtype) { + if (current_rrset_ && (!isSameType(rrtype, rdata, current_rrset_) || + current_rrset_->getClass() != rrclass || + current_rrset_->getName() != name)) { callback_(current_rrset_); current_rrset_.reset(); } if (!current_rrset_) { current_rrset_ = RRsetPtr(new RRset(name, rrclass, rrtype, rrttl)); + } else if (current_rrset_->getTTL() != rrttl) { + // RRs with different TTLs are given. Smaller TTL should win. + current_rrset_->setTTL(std::min(current_rrset_->getTTL(), rrttl)); } current_rrset_->addRdata(rdata); } @@ -69,8 +96,10 @@ RRCollator::getCallback() { void RRCollator::finish() { - // TODO: case when !current_rrset_ - impl_->callback_(impl_->current_rrset_); + if (impl_->current_rrset_) { + impl_->callback_(impl_->current_rrset_); + impl_->current_rrset_.reset(); + } } } // end namespace dns diff --git a/src/lib/dns/tests/rrcollator_unittest.cc b/src/lib/dns/tests/rrcollator_unittest.cc index dd6a88b28f..56d9112d60 100644 --- a/src/lib/dns/tests/rrcollator_unittest.cc +++ b/src/lib/dns/tests/rrcollator_unittest.cc @@ -45,21 +45,25 @@ protected: collator_(boost::bind(addRRset, _1, &rrsets_)), rr_callback_(collator_.getCallback()) { - RRsetPtr rrset(new RRset(origin_, rrclass_, RRType::A(), rrttl_)); a_rdata1_ = createRdata(RRType::A(), rrclass_, "192.0.2.1"); a_rdata2_ = createRdata(RRType::A(), rrclass_, "192.0.2.2"); - rrset->addRdata(a_rdata1_); - rrset->addRdata(a_rdata2_); - a_rrset_ = rrset; - rrset = RRsetPtr(new RRset(origin_, rrclass_, RRType::AAAA(), rrttl_)); - aaaa_rdata_ = createRdata(RRType::AAAA(), rrclass_, "2001:db8::1"); - aaaa_rrset_ = rrset; + txt_rdata_ = createRdata(RRType::TXT(), rrclass_, "test"); + + sig_rdata1_ = createRdata(RRType::RRSIG(), rrclass_, + "A 5 3 3600 20000101000000 20000201000000 " + "12345 example.com. FAKE\n"); + sig_rdata2_ = createRdata(RRType::RRSIG(), rrclass_, + "NS 5 3 3600 20000101000000 20000201000000 " + "12345 example.com. FAKE\n"); } void checkRRset(const Name& expected_name, const RRClass& expected_class, const RRType& expected_type, const RRTTL& expected_ttl, const vector& expected_rdatas) { + SCOPED_TRACE(expected_name.toText(true) + "/" + + expected_class.toText() + "/" + expected_type.toText()); + // This test always clears rrsets_ to confirm RRsets are added // one-by-one ASSERT_EQ(1, rrsets_.size()); @@ -85,15 +89,13 @@ protected: const RRClass rrclass_; const RRTTL rrttl_; vector rrsets_; - RdataPtr a_rdata1_, a_rdata2_, aaaa_rdata_; - ConstRRsetPtr a_rrset_, aaaa_rrset_; + RdataPtr a_rdata1_, a_rdata2_, txt_rdata_, sig_rdata1_, sig_rdata2_; RRCollator collator_; AddRRCallback rr_callback_; + vector rdatas_; // placeholder for expected data }; -TEST_F(RRCollatorTest, add) { - vector rdatas; - +TEST_F(RRCollatorTest, basicCases) { // Add two RRs belonging to the same RRset. These will be buffered. rr_callback_(origin_, rrclass_, RRType::A(), rrttl_, a_rdata1_); EXPECT_TRUE(rrsets_.empty()); // not yet given as an RRset @@ -102,16 +104,72 @@ TEST_F(RRCollatorTest, add) { // Add another type of RR. This completes the construction of the A RRset, // which will be given via the callback. - rr_callback_(origin_, rrclass_, RRType::AAAA(), rrttl_, aaaa_rdata_); - rdatas.push_back(a_rdata1_); - rdatas.push_back(a_rdata2_); - checkRRset(origin_, rrclass_, RRType::A(), rrttl_, rdatas); + rr_callback_(origin_, rrclass_, RRType::TXT(), rrttl_, txt_rdata_); + rdatas_.push_back(a_rdata1_); + rdatas_.push_back(a_rdata2_); + checkRRset(origin_, rrclass_, RRType::A(), rrttl_, rdatas_); + + // Add the same type of RR but of different name. This should make another + // callback for the previous TXT RR. + rr_callback_(Name("txt.example.com"), rrclass_, RRType::TXT(), rrttl_, + txt_rdata_); + rdatas_.clear(); + rdatas_.push_back(txt_rdata_); + checkRRset(origin_, rrclass_, RRType::TXT(), rrttl_, rdatas_); + + // Add the same type and name of RR but of different class (rare case + // in practice) + rr_callback_(Name("txt.example.com"), RRClass::CH(), RRType::TXT(), rrttl_, + txt_rdata_); + rdatas_.clear(); + rdatas_.push_back(txt_rdata_); + checkRRset(Name("txt.example.com"), rrclass_, RRType::TXT(), rrttl_, + rdatas_); + + // Tell the collator we are done, then we'll see the last RR as an RRset. + collator_.finish(); + checkRRset(Name("txt.example.com"), RRClass::CH(), RRType::TXT(), rrttl_, + rdatas_); + + // Redundant finish() will be no-op. + collator_.finish(); + EXPECT_TRUE(rrsets_.empty()); +} + +TEST_F(RRCollatorTest, minTTLFirst) { + // RRs of the same RRset but has different TTLs. The first RR has + // the smaller TTL, which should be used for the TTL of the RRset. + rr_callback_(origin_, rrclass_, RRType::A(), RRTTL(10), a_rdata1_); + rr_callback_(origin_, rrclass_, RRType::A(), RRTTL(20), a_rdata2_); + rdatas_.push_back(a_rdata1_); + rdatas_.push_back(a_rdata2_); + collator_.finish(); + checkRRset(origin_, rrclass_, RRType::A(), RRTTL(10), rdatas_); +} + +TEST_F(RRCollatorTest, maxTTLFirst) { + // RRs of the same RRset but has different TTLs. The second RR has + // the smaller TTL, which should be used for the TTL of the RRset. + rr_callback_(origin_, rrclass_, RRType::A(), RRTTL(20), a_rdata1_); + rr_callback_(origin_, rrclass_, RRType::A(), RRTTL(10), a_rdata2_); + rdatas_.push_back(a_rdata1_); + rdatas_.push_back(a_rdata2_); + collator_.finish(); + checkRRset(origin_, rrclass_, RRType::A(), RRTTL(10), rdatas_); +} + +TEST_F(RRCollatorTest, addRRSIGs) { + // RRSIG is special; they are also distinguished by their covered types. + rr_callback_(origin_, rrclass_, RRType::RRSIG(), rrttl_, sig_rdata1_); + rr_callback_(origin_, rrclass_, RRType::RRSIG(), rrttl_, sig_rdata2_); + + rdatas_.push_back(sig_rdata1_); + checkRRset(origin_, rrclass_, RRType::RRSIG(), rrttl_, rdatas_); +} - // Let the collator we are done, then we'll see the AAAA RR as an RRset. +TEST_F(RRCollatorTest, emptyFinish) { collator_.finish(); - rdatas.clear(); - rdatas.push_back(aaaa_rdata_); - checkRRset(origin_, rrclass_, RRType::AAAA(), rrttl_, rdatas); + EXPECT_TRUE(rrsets_.empty()); } }