From: JINMEI Tatuya Date: Fri, 15 Feb 2013 21:55:14 +0000 (-0800) Subject: [2440] mostly completed merge version of create(). X-Git-Tag: bind10-1.1.0beta1-release~70^2~10 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=017315cee2a710d8cdcfabc4deae5108e5f83c49;p=thirdparty%2Fkea.git [2440] mostly completed merge version of create(). with lot of duplicate code right now, and some corner cases are not yet handled. --- diff --git a/src/lib/datasrc/memory/rdataset.cc b/src/lib/datasrc/memory/rdataset.cc index 0c4332a08c..343fb5e162 100644 --- a/src/lib/datasrc/memory/rdataset.cc +++ b/src/lib/datasrc/memory/rdataset.cc @@ -127,12 +127,40 @@ RdataSet::create(util::MemorySegment& mem_sgmt, RdataEncoder& encoder, ConstRRsetPtr sig_rrset) { // TODO: consistency check and taking min + // Check basic validity + if (!rrset && !sig_rrset) { + isc_throw(BadValue, "Both RRset and RRSIG are NULL"); + } + if (rrset && rrset->getRdataCount() == 0) { + isc_throw(BadValue, "Empty RRset"); + } + if (sig_rrset && sig_rrset->getRdataCount() == 0) { + isc_throw(BadValue, "Empty SIG RRset"); + } + if (rrset && sig_rrset && rrset->getClass() != sig_rrset->getClass()) { + isc_throw(BadValue, "RR class doesn't match between RRset and RRSIG"); + } + + // Check assumptions on the number of RDATAs + const size_t old_rdata_count = old_rdataset.getRdataCount(); + const size_t old_sig_count = old_rdataset.getSigRdataCount(); + if (rrset && (rrset->getRdataCount() + old_rdata_count) > MAX_RDATA_COUNT) + { + isc_throw(RdataSetError, "Too many RDATAs for RdataSet: " + << rrset->getRdataCount() << ", must be <= " + << MAX_RDATA_COUNT); + } + if (sig_rrset && (sig_rrset->getRdataCount() + old_sig_count) > + MAX_RRSIG_COUNT) { + isc_throw(RdataSetError, "Too many RRSIGs for RdataSet: " + << sig_rrset->getRdataCount() << ", must be <= " + << MAX_RRSIG_COUNT); + } + const RRClass rrclass = rrset ? rrset->getClass() : sig_rrset->getClass(); const RRType rrtype = rrset ? rrset->getType() : getCoveredType(sig_rrset->getRdataIterator()->getCurrent()); const RRTTL rrttl = rrset ? rrset->getTTL() : sig_rrset->getTTL(); - const size_t old_rdata_count = old_rdataset.getRdataCount(); - const size_t old_sig_count = old_rdataset.getSigRdataCount(); encoder.start(rrclass, rrtype, old_rdataset.getDataBuf(), old_rdata_count, old_sig_count); if (rrset) { @@ -164,6 +192,9 @@ RdataSet::create(util::MemorySegment& mem_sgmt, RdataEncoder& encoder, old_rdata_count + (rrset ? rrset->getRdataCount() : 0); RdataSet* rdataset = new(p) RdataSet(rrtype, rdata_count, rrsig_count, rrttl); + if (rrsig_count >= MANY_RRSIG_COUNT) { + *rdataset->getExtSIGCountBuf() = rrsig_count; + } encoder.encode(rdataset->getDataBuf(), data_len); return (rdataset); } diff --git a/src/lib/datasrc/tests/memory/rdataset_unittest.cc b/src/lib/datasrc/tests/memory/rdataset_unittest.cc index 78055edd36..832c7f6c3b 100644 --- a/src/lib/datasrc/tests/memory/rdataset_unittest.cc +++ b/src/lib/datasrc/tests/memory/rdataset_unittest.cc @@ -68,6 +68,13 @@ protected: EXPECT_TRUE(mem_sgmt_.allMemoryDeallocated()); } + // Helper for checking common cases against both versions of create() + typedef boost::function CreateFn; + void checkCreateManyRRs(CreateFn create_fn, size_t n_old_rdata); + void checkCreateManyRRSIGs(CreateFn create_fn, size_t n_old_sig); + void checkBadCreate(CreateFn create_fn); + const RRClass rrclass; ConstRRsetPtr a_rrset_, rrsig_rrset_; isc::util::MemorySegmentLocal mem_sgmt_; @@ -78,6 +85,17 @@ protected: vector def_rrsig_txt_; }; +// A helper adaptor for some checkXXX functions as boost::bind() can't +// directly take a reference to non-copyable object. +RdataSet* +createWrapper(isc::util::MemorySegment& mem_sgmt, RdataEncoder& encoder, + const RdataSet* old_rdataset, ConstRRsetPtr rrset, + ConstRRsetPtr sig_rrset) +{ + return (RdataSet::create(mem_sgmt, encoder, *old_rdataset, rrset, + sig_rrset)); +} + // Convert the given 32-bit integer (network byte order) to the corresponding // RRTTL object. RRTTL @@ -307,27 +325,44 @@ getRRsetWithRdataCount(size_t rdata_count) { return (rrset); } -TEST_F(RdataSetTest, createManyRRs) { - // RRset with possible maximum number of RDATAs - RdataSet* rdataset = RdataSet::create(mem_sgmt_, encoder_, - getRRsetWithRdataCount(8191), - ConstRRsetPtr()); +void +RdataSetTest::checkCreateManyRRs(CreateFn create_fn, size_t n_old_rdata) { + // RRset with possible maximum number of RDATAs, taking into account + // "pre-existing" RDATAs + RdataSet* rdataset = create_fn(mem_sgmt_, encoder_, + getRRsetWithRdataCount(8191 - n_old_rdata), + ConstRRsetPtr()); EXPECT_EQ(8191, rdataset->getRdataCount()); EXPECT_EQ(0, rdataset->getSigRdataCount()); RdataSet::destroy(mem_sgmt_, rdataset, RRClass::IN()); // Exceeding that will result in an exception. - EXPECT_THROW(RdataSet::create(mem_sgmt_, encoder_, - getRRsetWithRdataCount(8192), - ConstRRsetPtr()), + EXPECT_THROW(create_fn(mem_sgmt_, encoder_, + getRRsetWithRdataCount(8192 - n_old_rdata), + ConstRRsetPtr()), RdataSetError); // To be very sure even try larger number than the threshold - EXPECT_THROW(RdataSet::create(mem_sgmt_, encoder_, - getRRsetWithRdataCount(65535), - ConstRRsetPtr()), + EXPECT_THROW(create_fn(mem_sgmt_, encoder_, + getRRsetWithRdataCount(65535 - n_old_rdata), + ConstRRsetPtr()), RdataSetError); } +TEST_F(RdataSetTest, createManyRRs) { + checkCreateManyRRs(boost::bind(&RdataSet::create, _1, _2, _3, _4), 0); +} + +TEST_F(RdataSetTest, mergeCreateManyRRs) { + ConstRRsetPtr rrset = textToRRset("example.com. 3600 IN TXT some-text"); + SegmentObjectHolder holder( + mem_sgmt_, + RdataSet::create(mem_sgmt_, encoder_, rrset, ConstRRsetPtr()), + RRClass::IN()); + + checkCreateManyRRs(boost::bind(createWrapper, _1, _2, holder.get(), _3, + _4), rrset->getRdataCount()); +} + TEST_F(RdataSetTest, createWithRRSIG) { // Normal case. RdataSet* rdataset = RdataSet::create(mem_sgmt_, encoder_, a_rrset_, @@ -372,38 +407,58 @@ getRRSIGWithRdataCount(size_t sig_count) { return (rrset); } -TEST_F(RdataSetTest, createManyRRSIGs) { +void +RdataSetTest::checkCreateManyRRSIGs(CreateFn create_fn, size_t n_old_sig) { // 7 has a special meaning in the implementation: if the number of the // RRSIGs reaches this value, an extra 'sig count' field will be created. - RdataSet* rdataset = RdataSet::create(mem_sgmt_, encoder_, a_rrset_, - getRRSIGWithRdataCount(7)); + RdataSet* rdataset = create_fn(mem_sgmt_, encoder_, a_rrset_, + getRRSIGWithRdataCount(7 - n_old_sig)); EXPECT_EQ(7, rdataset->getSigRdataCount()); RdataSet::destroy(mem_sgmt_, rdataset, RRClass::IN()); // 8 would cause overflow in the normal 3-bit field if there were no extra // count field. - rdataset = RdataSet::create(mem_sgmt_, encoder_, a_rrset_, - getRRSIGWithRdataCount(8)); + rdataset = create_fn(mem_sgmt_, encoder_, a_rrset_, + getRRSIGWithRdataCount(8 - n_old_sig)); EXPECT_EQ(8, rdataset->getSigRdataCount()); RdataSet::destroy(mem_sgmt_, rdataset, RRClass::IN()); // Up to 2^16-1 RRSIGs are allowed (although that would be useless // in practice) - rdataset = RdataSet::create(mem_sgmt_, encoder_, a_rrset_, - getRRSIGWithRdataCount(65535)); + rdataset = create_fn(mem_sgmt_, encoder_, a_rrset_, + getRRSIGWithRdataCount(65535 - n_old_sig)); EXPECT_EQ(65535, rdataset->getSigRdataCount()); RdataSet::destroy(mem_sgmt_, rdataset, RRClass::IN()); // Exceeding this limit will result in an exception. - EXPECT_THROW(RdataSet::create(mem_sgmt_, encoder_, a_rrset_, - getRRSIGWithRdataCount(65536)), + EXPECT_THROW(create_fn(mem_sgmt_, encoder_, a_rrset_, + getRRSIGWithRdataCount(65536 - n_old_sig)), RdataSetError); // To be very sure even try larger number than the threshold - EXPECT_THROW(RdataSet::create(mem_sgmt_, encoder_, a_rrset_, - getRRSIGWithRdataCount(70000)), + EXPECT_THROW(create_fn(mem_sgmt_, encoder_, a_rrset_, + getRRSIGWithRdataCount(70000 - n_old_sig)), RdataSetError); } +TEST_F(RdataSetTest, createManyRRSIGs) { + checkCreateManyRRSIGs(boost::bind(&RdataSet::create, _1, _2, _3, _4), 0); +} + +TEST_F(RdataSetTest, mergeCreateManyRRSIGs) { + // Create "old" RRSIG that shouldn't be a duplicate of ones created in + // checkCreateManyRRSIGs (signature is different). + ConstRRsetPtr rrsig = textToRRset( + "example.com. 3600 IN RRSIG A 5 2 3600 20120814220826 20120715220826 " + "1234 example.com. FAKEFAKE"); + SegmentObjectHolder holder( + mem_sgmt_, + RdataSet::create(mem_sgmt_, encoder_, ConstRRsetPtr(), rrsig), + rrclass); + + checkCreateManyRRSIGs(boost::bind(createWrapper, _1, _2, holder.get(), _3, + _4), rrsig->getRdataCount()); +} + TEST_F(RdataSetTest, createWithRRSIGOnly) { // A rare, but allowed, case: RdataSet without the main RRset but with // RRSIG. @@ -413,36 +468,37 @@ TEST_F(RdataSetTest, createWithRRSIGOnly) { RdataSet::destroy(mem_sgmt_, rdataset, RRClass::IN()); } -TEST_F(RdataSetTest, badCeate) { +// Checking initial validation for both versions of create(). +void +RdataSetTest::checkBadCreate(CreateFn create_fn) { // Neither the RRset nor RRSIG RRset is given - EXPECT_THROW(RdataSet::create(mem_sgmt_, encoder_, ConstRRsetPtr(), - ConstRRsetPtr()), isc::BadValue); + EXPECT_THROW(create_fn(mem_sgmt_, encoder_, ConstRRsetPtr(), + ConstRRsetPtr()), isc::BadValue); // Empty RRset (An RRset without RDATA) ConstRRsetPtr empty_rrset(new RRset(Name("example.com"), RRClass::IN(), RRType::A(), RRTTL(3600))); - EXPECT_THROW(RdataSet::create(mem_sgmt_, encoder_, empty_rrset, - ConstRRsetPtr()), isc::BadValue); + EXPECT_THROW(create_fn(mem_sgmt_, encoder_, empty_rrset, + ConstRRsetPtr()), isc::BadValue); ConstRRsetPtr empty_rrsig(new RRset(Name("example.com"), RRClass::IN(), RRType::RRSIG(), RRTTL(3600))); - EXPECT_THROW(RdataSet::create(mem_sgmt_, encoder_, ConstRRsetPtr(), - empty_rrsig), isc::BadValue); + EXPECT_THROW(create_fn(mem_sgmt_, encoder_, ConstRRsetPtr(), + empty_rrsig), isc::BadValue); // The RRset type and RRSIG's type covered don't match ConstRRsetPtr bad_rrsig(textToRRset( "www.example.com. 1076895760 IN RRSIG " "NS 5 2 3600 20120814220826 20120715220826 " "1234 example.com. FAKE")); - EXPECT_THROW(RdataSet::create(mem_sgmt_, encoder_, a_rrset_, bad_rrsig), + EXPECT_THROW(create_fn(mem_sgmt_, encoder_, a_rrset_, bad_rrsig), isc::BadValue); // Pass non RRSIG for the sig parameter - EXPECT_THROW(RdataSet::create(mem_sgmt_, encoder_, a_rrset_, a_rrset_), + EXPECT_THROW(create_fn(mem_sgmt_, encoder_, a_rrset_, a_rrset_), isc::BadValue); // Pass RRSIG for normal RRset (the RdataEncoder will catch this and throw) - EXPECT_THROW(RdataSet::create(mem_sgmt_, encoder_, rrsig_rrset_, - rrsig_rrset_), + EXPECT_THROW(create_fn(mem_sgmt_, encoder_, rrsig_rrset_, rrsig_rrset_), isc::BadValue); // RR class doesn't match between RRset and RRSIG @@ -451,8 +507,22 @@ TEST_F(RdataSetTest, badCeate) { "A 5 2 3600 20120814220826 " "20120715220826 1234 example.com. FAKE", RRClass::CH())); - EXPECT_THROW(RdataSet::create(mem_sgmt_, encoder_, a_rrset_, - badclass_rrsig), + EXPECT_THROW(create_fn(mem_sgmt_, encoder_, a_rrset_, badclass_rrsig), isc::BadValue); } + +TEST_F(RdataSetTest, badCeate) { + checkBadCreate(boost::bind(&RdataSet::create, _1, _2, _3, _4)); +} + +TEST_F(RdataSetTest, badMergeCeate) { + // The 'old RdataSet' for merge. Its content doesn't matter; the test + // should trigger exception before examining it. + SegmentObjectHolder holder( + mem_sgmt_, + RdataSet::create(mem_sgmt_, encoder_, a_rrset_, ConstRRsetPtr()), + RRClass::IN()); + + checkBadCreate(boost::bind(createWrapper, _1, _2, holder.get(), _3, _4)); +} }