[1502] made sure comparing RRSIG type covered in compact() to avoid incorrect

compaction of non compatible RRSIGs.

src/lib/python/isc/xfrin/diff.py

@@ -162,12 +162,26 @@ class Diff:
         and do more merging, but such diffs should be rare in practice anyway,
         so we don't bother and do it this simple way.
         """
+        def same_type(rrset1, rrset2):
+            '''A helper routine to identify whether two RRsets are of the
+            same 'type'.  For RRSIGs we should consider type covered, too.
+            '''
+            if rrset1.get_type() != rrset2.get_type():
+                return False
+            if rrset1.get_type() != isc.dns.RRType.RRSIG():
+                return rrset1.get_type() == rrset2.get_type()
+            # RR type of the both RRsets is RRSIG.  Compare type covered.
+            # We know they have exactly one RDATA.
+            sigdata1 = rrset1.get_rdata()[0].to_text().split()[0]
+            sigdata2 = rrset2.get_rdata()[0].to_text().split()[0]
+            return sigdata1 == sigdata2
+
         buf = []
         for (op, rrset) in self.__buffer:
             old = buf[-1][1] if len(buf) > 0 else None
             if old is None or op != buf[-1][0] or \
                 rrset.get_name() != old.get_name() or \
-                rrset.get_type() != old.get_type():
+                (not same_type(rrset, old)):
                 buf.append((op, isc.dns.RRset(rrset.get_name(),
                                               rrset.get_class(),
                                               rrset.get_type(),

src/lib/python/isc/xfrin/tests/diff_tests.py

@@ -99,6 +99,8 @@ class DiffTest(unittest.TestCase):
         in the tested module.
         """
         self.__warn_called = True
+        # Also log the message so we can check the log format (manually)
+        self.orig_logger.warn(*args)
 
     def commit(self):
         """
@@ -430,7 +432,7 @@ class DiffTest(unittest.TestCase):
         Test the TTL handling. A warn function should have been called if they
         differ, but that's all, it should not crash or raise.
         """
-        orig_logger = isc.xfrin.diff.logger
+        self.orig_logger = isc.xfrin.diff.logger
         try:
             isc.xfrin.diff.logger = self
             diff = Diff(self, Name('example.org.'))
@@ -451,7 +453,30 @@ class DiffTest(unittest.TestCase):
             self.assertEqual(self.__ttl, diff.get_buffer()[0][1].get_ttl())
             self.assertTrue(self.__warn_called)
         finally:
-            isc.xfrin.diff.logger = orig_logger
+            isc.xfrin.diff.logger = self.orig_logger
+
+    def test_rrsig_ttl(self):
+        '''Similar to the previous test, but for RRSIGs of different covered
+        types.
+
+        They shouldn't be compacted.
+
+        '''
+        diff = Diff(self, Name('example.org.'))
+        rrsig1 = RRset(Name('example.org'), self.__rrclass,
+                       RRType.RRSIG(), RRTTL(3600))
+        rrsig1.add_rdata(Rdata(RRType.RRSIG(), self.__rrclass,
+                               'A 5 3 3600 20000101000000 20000201000000 ' +
+                               '0 example.org. FAKEFAKEFAKE'))
+        diff.add_data(rrsig1)
+        rrsig2 = RRset(Name('example.org'), self.__rrclass,
+                       RRType.RRSIG(), RRTTL(1800))
+        rrsig2.add_rdata(Rdata(RRType.RRSIG(), self.__rrclass,
+                               'AAAA 5 3 3600 20000101000000 20000201000000 ' +
+                               '1 example.org. FAKEFAKEFAKE'))
+        diff.add_data(rrsig2)
+        diff.compact()
+        self.assertEqual(2, len(diff.get_buffer()))
 
     def test_relpace(self):
         """
@@ -463,4 +488,5 @@ class DiffTest(unittest.TestCase):
 
 if __name__ == "__main__":
     isc.log.init("bind10")
+    isc.log.resetUnitTestRootLogger()
     unittest.main()