Browse Source

[2097] handle and test large # of RRs and SIGs.

JINMEI Tatuya 12 years ago
parent
commit
879a73596b

+ 31 - 8
src/lib/datasrc/memory/rdataset.cc

@@ -55,6 +55,7 @@ RdataSet*
 RdataSet::create(util::MemorySegment& mem_sgmt, RdataEncoder& encoder,
                  ConstRRsetPtr rrset, ConstRRsetPtr sig_rrset)
 {
+    // Check basic validity
     if (!rrset && !sig_rrset) {
         isc_throw(BadValue, "Both RRset and RRSIG are NULL");
     }
@@ -71,6 +72,18 @@ RdataSet::create(util::MemorySegment& mem_sgmt, RdataEncoder& encoder,
         }
     }
 
+    // Check assumptions on the number of RDATAs
+    if (rrset && rrset->getRdataCount() > MAX_RDATA_COUNT) {
+        isc_throw(RdataSetError, "Too many RDATAs for RdataSet: "
+                  << rrset->getRdataCount() << ", must be <= "
+                  << MAX_RDATA_COUNT);
+    }
+    if (sig_rrset && sig_rrset->getRdataCount() > 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());
@@ -96,13 +109,18 @@ RdataSet::create(util::MemorySegment& mem_sgmt, RdataEncoder& encoder,
         }
     }
 
+    const size_t rrsig_count = sig_rrset ? sig_rrset->getRdataCount() : 0;
+    const size_t ext_rrsig_count_len =
+        rrsig_count >= MANY_RRSIG_COUNT ? sizeof(uint16_t) : 0;
     const size_t data_len = encoder.getStorageLength();
-    void* p = mem_sgmt.allocate(sizeof(RdataSet) + data_len);
+    void* p = mem_sgmt.allocate(sizeof(RdataSet) + ext_rrsig_count_len +
+                                data_len);
     RdataSet* rdataset = new(p) RdataSet(rrtype,
                                          rrset ? rrset->getRdataCount() : 0,
-                                         sig_rrset ?
-                                         sig_rrset->getRdataCount() : 0,
-                                         rrttl);
+                                         rrsig_count, rrttl);
+    if (rrsig_count >= MANY_RRSIG_COUNT) {
+        *rdataset->getExtSIGCountBuf() = rrsig_count;
+    }
     encoder.encode(rdataset->getDataBuf(), data_len);
     return (rdataset);
 }
@@ -114,10 +132,13 @@ RdataSet::destroy(util::MemorySegment& mem_sgmt, RRClass rrclass,
     const size_t data_len =
         RdataReader(rrclass, rdataset->type,
                     reinterpret_cast<const uint8_t*>(rdataset->getDataBuf()),
-                    rdataset->rdata_count,
-                    rdataset->sig_rdata_count).getSize();
+                    rdataset->getRdataCount(),
+                    rdataset->getSigRdataCount()).getSize();
+    const size_t ext_rrsig_count_len =
+        rdataset->sig_rdata_count >= MANY_RRSIG_COUNT ? sizeof(uint16_t) : 0;
     rdataset->~RdataSet();
-    mem_sgmt.deallocate(rdataset, sizeof(RdataSet) + data_len);
+    mem_sgmt.deallocate(rdataset,
+                        sizeof(RdataSet) + ext_rrsig_count_len + data_len);
 }
 
 namespace {
@@ -140,7 +161,9 @@ convertTTL(RRTTL ttl) {
 
 RdataSet::RdataSet(RRType type_param, size_t rdata_count_param,
                    size_t sig_rdata_count_param, RRTTL ttl_param) :
-    type(type_param), sig_rdata_count(sig_rdata_count_param),
+    type(type_param),
+    sig_rdata_count(sig_rdata_count_param >= MANY_RRSIG_COUNT ?
+                    MANY_RRSIG_COUNT : sig_rdata_count_param),
     rdata_count(rdata_count_param), ttl(convertTTL(ttl_param))
 {
     // Make sure an RRType object is essentially a plain 16-bit value, so

+ 38 - 5
src/lib/datasrc/memory/rdataset.h

@@ -31,6 +31,16 @@ namespace datasrc {
 namespace memory {
 class RdataEncoder;
 
+/// \brief General error on creating RdataSet.
+///
+/// This is thrown when creating \c RdataSet encounters a rare, unsupported
+/// situation.
+class RdataSetError : public Exception {
+public:
+    RdataSetError(const char* file, size_t line, const char* what) :
+        Exception(file, line, what) {}
+};
+
 class RdataSet {
 public:
     static RdataSet* create(util::MemorySegment& mem_sgmt,
@@ -43,9 +53,9 @@ public:
     typedef boost::interprocess::offset_ptr<RdataSet> RdataSetPtr;
     typedef boost::interprocess::offset_ptr<const RdataSet> ConstRdataSetPtr;
 
-    // Note: the size and order of the members are important.  Don't change
-    // them unless there's strong reason for that and the consequences are
-    // considered.
+    // Note: the size and order of the members are carefully chosen to
+    // maximize efficiency.  Don't change them unless there's strong reason
+    // for that and the consequences are considered.
 
     RdataSetPtr next;
     const dns::RRType type;
@@ -53,13 +63,36 @@ private:
     const uint16_t sig_rdata_count : 3;
     const uint16_t rdata_count : 13;
     const uint32_t ttl;       ///< TTL of the RdataSet, net byte order
+
+    static const size_t MAX_RDATA_COUNT = (1 << 13) - 1;
+    static const size_t MAX_RRSIG_COUNT = (1 << 16) - 1;
+    static const size_t MANY_RRSIG_COUNT = (1 << 3) - 1;
 public:
     size_t getRdataCount() const { return (rdata_count); }
-    size_t getSigRdataCount() const { return (sig_rdata_count); }
+    size_t getSigRdataCount() const {
+        if (sig_rdata_count < MANY_RRSIG_COUNT) {
+            return (sig_rdata_count);
+        } else {
+            return (*getExtSIGCountBuf());
+        }
+    }
     const void* getTTLData() const { return (&ttl); }
-    void* getDataBuf() { return (this + 1); }
+    void* getDataBuf() {
+        if (sig_rdata_count < MANY_RRSIG_COUNT) {
+            return (this + 1);
+        } else {
+            return (getExtSIGCountBuf() + 1);
+        }
+    }
 
 private:
+    uint16_t* getExtSIGCountBuf() {
+        return (reinterpret_cast<uint16_t*>(this + 1));
+    }
+    const uint16_t* getExtSIGCountBuf() const {
+        return (reinterpret_cast<const uint16_t*>(this + 1));
+    }
+
     RdataSet(dns::RRType type, size_t rdata_count, size_t sig_rdata_count,
              dns::RRTTL ttl);
     ~RdataSet() {}

+ 80 - 0
src/lib/datasrc/memory/tests/rdataset_unittest.cc

@@ -17,6 +17,7 @@
 #include <util/buffer.h>
 #include <util/memory_segment_local.h>
 
+#include <dns/rdata.h>
 #include <dns/rrset.h>
 #include <dns/rrclass.h>
 #include <dns/rrtype.h>
@@ -29,9 +30,14 @@
 
 #include <gtest/gtest.h>
 
+#include <boost/lexical_cast.hpp>
+
+#include <string>
+
 using namespace isc::dns;
 using namespace isc::datasrc::memory;
 using namespace isc::testutils;
+using boost::lexical_cast;
 
 namespace {
 
@@ -76,6 +82,33 @@ TEST_F(RdataSetTest, create) {
     RdataSet::destroy(mem_sgmt_, RRClass::IN(), rdataset);
 }
 
+ConstRRsetPtr
+getRRsetWithRdataCount(size_t rdata_count) {
+    RRsetPtr rrset(new RRset(Name("example.com"), RRClass::IN(), RRType::TXT(),
+                             RRTTL(3600)));
+    for (size_t i = 0; i < rdata_count; ++i) {
+        rrset->addRdata(rdata::createRdata(RRType::TXT(), RRClass::IN(),
+                                           lexical_cast<std::string>(i)));
+    }
+    return (rrset);
+}
+
+TEST_F(RdataSetTest, createManyRRs) {
+    // RRset with possible maximum number of RDATAs
+    RdataSet* rdataset = RdataSet::create(mem_sgmt_, encoder_,
+                                          getRRsetWithRdataCount(8191),
+                                          ConstRRsetPtr());
+    EXPECT_EQ(8191, rdataset->getRdataCount());
+    EXPECT_EQ(0, rdataset->getSigRdataCount());
+    RdataSet::destroy(mem_sgmt_, RRClass::IN(), rdataset);
+
+    // Exceeding that will result in an exception.
+    EXPECT_THROW(RdataSet::create(mem_sgmt_, encoder_,
+                                  getRRsetWithRdataCount(8192),
+                                  ConstRRsetPtr()),
+                 RdataSetError);
+}
+
 TEST_F(RdataSetTest, createWithRRSIG) {
     // Normal case.
     RdataSet* rdataset = RdataSet::create(mem_sgmt_, encoder_, a_rrset_,
@@ -95,6 +128,48 @@ TEST_F(RdataSetTest, createWithRRSIG) {
     RdataSet::destroy(mem_sgmt_, RRClass::IN(), rdataset);
 }
 
+ConstRRsetPtr
+getRRSIGWithRdataCount(size_t sig_count) {
+    RRsetPtr rrset(new RRset(Name("example.com"), RRClass::IN(),
+                             RRType::RRSIG(), RRTTL(3600)));
+    for (size_t i = 0; i < sig_count; ++i) {
+        rrset->addRdata(rdata::createRdata(
+                            RRType::RRSIG(), RRClass::IN(),
+                            "A 5 2 3600 20120814220826 20120715220826 " +
+                            lexical_cast<std::string>(i) +
+                            " example.com. FAKE"));
+    }
+    return (rrset);
+}
+
+TEST_F(RdataSetTest, createManyRRSIGs) {
+    // 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));
+    EXPECT_EQ(7, rdataset->getSigRdataCount());
+    RdataSet::destroy(mem_sgmt_, RRClass::IN(), rdataset);
+
+    // 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));
+    EXPECT_EQ(8, rdataset->getSigRdataCount());
+    RdataSet::destroy(mem_sgmt_, RRClass::IN(), rdataset);
+
+    // 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));
+    EXPECT_EQ(65535, rdataset->getSigRdataCount());
+    RdataSet::destroy(mem_sgmt_, RRClass::IN(), rdataset);
+
+    // Exceeding this limit will result in an exception.
+    EXPECT_THROW(RdataSet::create(mem_sgmt_, encoder_, a_rrset_,
+                                  getRRSIGWithRdataCount(65536)),
+                 RdataSetError);
+}
+
 TEST_F(RdataSetTest, createWithRRSIGOnly) {
     // A rare, but allowed, case: RdataSet without the main RRset but with
     // RRSIG.
@@ -134,6 +209,11 @@ TEST_F(RdataSetTest, badCeate) {
     EXPECT_THROW(RdataSet::create(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_),
+                 isc::BadValue);
+
     // RR class doesn't match between RRset and RRSIG
     ConstRRsetPtr badclass_rrsig(textToRRset(
                                      "www.example.com. 1076895760 CH RRSIG "