[1614] make sure that RRSIGs of different types covered are regarded as

separate RRsets.  As noted in the previous comment the previous behavior
was okay in practice within the expected restriction of the current
implementation, but as we are probably going to accept more generic forms
of input it would be prudent to fix it at this timing.

diff --git a/src/lib/dns/masterload.cc b/src/lib/dns/masterload.cc
index a32d4b70f76e328be6be066ab6a5282661df4233..000487c4c0131de2bb7a897ba5b5b0948e50b6a9 100644 (file)
--- a/src/lib/dns/masterload.cc
+++ b/src/lib/dns/masterload.cc
@@ -25,6 +25,7 @@
 #include <dns/masterload.h>
 #include <dns/name.h>
 #include <dns/rdata.h>
+#include <dns/rdataclass.h>
 #include <dns/rrclass.h>
 #include <dns/rrset.h>
 #include <dns/rrttl.h>
@@ -58,6 +59,7 @@ masterLoad(istream& input, const Name& origin, const RRClass& zone_class,
            MasterLoadCallback callback)
 {
     RRsetPtr rrset;
+    ConstRdataPtr prev_rdata;   // placeholder for special case of RRSIGs
     string line;
     unsigned int line_count = 1;
 
@@ -145,8 +147,20 @@ masterLoad(istream& input, const Name& origin, const RRClass& zone_class,
         // Everything is okay.  Now create/update RRset with the new RR.
         // If this is the first RR or the RR type/name is new, we are seeing
         // a new RRset.
+        bool new_rrset = false;
         if (!rrset || rrset->getType() != *rrtype ||
             rrset->getName() != *owner) {
+            new_rrset = true;
+        } else if (rrset->getType() == RRType::RRSIG()) {
+            // We are seeing two consecutive RRSIGs of the same name.
+            // They can be combined iff they have the same type covered.
+            if (dynamic_cast<const generic::RRSIG&>(*rdata).typeCovered() !=
+                dynamic_cast<const generic::RRSIG&>(*prev_rdata).typeCovered())
+            {
+                new_rrset = true;
+            }
+        }
+        if (new_rrset) {
             // Commit the previous RRset, if any.
             if (rrset) {
                 callback(rrset);
@@ -154,6 +168,7 @@ masterLoad(istream& input, const Name& origin, const RRClass& zone_class,
             rrset = RRsetPtr(new RRset(*owner, *rrclass, *rrtype, *ttl));
         }
         rrset->addRdata(rdata);
+        prev_rdata = rdata;
     } while (++line_count, !input.eof());
 
     // Commit the last RRset, if any.

diff --git a/src/lib/dns/masterload.h b/src/lib/dns/masterload.h
index fe5c08fda52dd32de90b293cb69f33070a2c5f33..d062c5d814f254a3564a51d5f2693a85a3652f26 100644 (file)
--- a/src/lib/dns/masterload.h
+++ b/src/lib/dns/masterload.h
@@ -206,9 +206,8 @@ typedef boost::function<void(RRsetPtr)> MasterLoadCallback;
 /// - We may want to support incremental loading.
 /// - If we add these optional features we may want to introduce a class
 ///   that encapsulates loading status and options.
-/// - RRSIGs are currently identified as their owner name and RR type (RRSIG).
-///   In practice it should be sufficient, but technically we should also
-///   consider the Type Covered field.
+/// - RRSIGs are handled separate RRsets, that is, not set within the RRset
+///   that they cover.
 ///
 /// \param filename A path to a master zone file to be loaded.
 /// \param origin The origin name of the zone.

diff --git a/src/lib/dns/tests/masterload_unittest.cc b/src/lib/dns/tests/masterload_unittest.cc
index c47debdd38cb1515dc2efd1831a35e0427870c65..acb9d64c1417dbb65a23ced9a59f2604148a462e 100644 (file)
--- a/src/lib/dns/tests/masterload_unittest.cc
+++ b/src/lib/dns/tests/masterload_unittest.cc
@@ -72,6 +72,13 @@ const char* const a_rr2 = "www.example.com. 60 IN A 192.0.2.2\n";
 const char* const a_rr3 = "ftp.example.com. 60 IN A 192.0.2.3\n";
 // multi-field RR case
 const char* const soa_rr = "example.com. 7200 IN SOA . . 0 0 0 0 0\n";
+// A couple of RRSIGs, different type covered
+const char* const rrsig_rr1 =
+    "www.example.com. 60 IN RRSIG A 5 3 3600 20000101000000 20000201000000 "
+    "12345 example.com. FAKEFAKEFAKE\n";
+const char* const rrsig_rr2 =
+    "www.example.com. 60 IN RRSIG AAAA 5 3 3600 20000101000000 20000201000000 "
+    "12345 example.com. FAKEFAKEFAKE\n";
 
 TEST_F(MasterLoadTest, loadRRs) {
     // a simple case: loading 3 RRs, each consists of a single RRset.
@@ -147,6 +154,13 @@ TEST_F(MasterLoadTest, loadRRsetsInterleaved) {
     EXPECT_EQ(a_rr2, results[2]->toText());
 }
 
+TEST_F(MasterLoadTest, loadRRsigs) {
+    // RRSIGs of different types covered should be separated
+    rr_stream << rrsig_rr1 << rrsig_rr2;
+    masterLoad(rr_stream, origin, zclass, callback);
+    EXPECT_EQ(2, results.size());
+}
+
 TEST_F(MasterLoadTest, loadWithNoEOF) {
     // the input stream doesn't end with a new line (and the following blank
     // line).  It should be accepted.