[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.

diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index 08c7b26b312ab274922ba7069f7a6a0ef7071679..22de3ffdb5bb1d764c3b6901553a7297dcf59a5c 100644 (file)
--- 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<const generic::RRSIG&>(*rdata1).typeCovered()
+                    == dynamic_cast<const generic::RRSIG&>(*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 0e9c924cc75af5ee21d648ad10a9ebcbbed46b6b..9362a46f119690bbd14555b391b1872bf358e448 100644 (file)
--- 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();