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

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.

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.

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.