Fix #698 and #702, problems caused by _cmp() giving the wrong

result in certain comparisons of rdata with relative and absolute
names.

5 files changed, 96 insertions, 11 deletions

diff --git a/dns/message.py b/dns/message.py
index 6fa90ca..2a7565a 100644
--- a/dns/message.py
+++ b/dns/message.py
@@ -236,8 +236,7 @@ class Message:
         return not self.__eq__(other)
 
     def is_response(self, other):
-        """Is *other*, also a ``dns.message.Message``, a response to this
-        message?
+        """Is *other* a response this message?
 
         Returns a ``bool``.
         """
diff --git a/dns/rdata.py b/dns/rdata.py
index 0831c41..8a79de7 100644
--- a/dns/rdata.py
+++ b/dns/rdata.py
@@ -39,6 +39,14 @@ import dns.ttl
 _chunksize = 32
 
 
+class NoRelativeRdataOrdering(dns.exception.DNSException):
+    """An attempt was made to do an ordered comparison of one or more
+    rdata with relative names.  The only reliable way of sorting rdata
+    is to use non-relativized rdata.
+
+    """
+
+
 def _wordbreak(data, chunksize=_chunksize, separator=b' '):
     """Break a binary string into chunks of chunksize characters separated by
     a space.
@@ -234,10 +242,12 @@ class Rdata:
 
         Return < 0 if self < other in the DNSSEC ordering, 0 if self
         == other, and > 0 if self > other.
-
         """
-        our = self.to_digestable(dns.name.root)
-        their = other.to_digestable(dns.name.root)
+        try:
+            our = self.to_digestable()
+            their = other.to_digestable()
+        except dns.name.NeedAbsoluteNameOrOrigin:
+            raise NoRelativeRdataOrdering
         if our == their:
             return 0
         elif our > their:
@@ -250,14 +260,28 @@ class Rdata:
             return False
         if self.rdclass != other.rdclass or self.rdtype != other.rdtype:
             return False
-        return self._cmp(other) == 0
+        our_relative = False
+        their_relative = False
+        try:
+            our = self.to_digestable()
+        except dns.name.NeedAbsoluteNameOrOrigin:
+            our = self.to_digestable(dns.name.root)
+            our_relative = True
+        try:
+            their = other.to_digestable()
+        except dns.name.NeedAbsoluteNameOrOrigin:
+            their = other.to_digestable(dns.name.root)
+            their_relative = True
+        if our_relative != their_relative:
+            return False
+        return our == their
 
     def __ne__(self, other):
         if not isinstance(other, Rdata):
             return True
         if self.rdclass != other.rdclass or self.rdtype != other.rdtype:
             return True
-        return self._cmp(other) != 0
+        return not self.__eq__(other)
 
     def __lt__(self, other):
         if not isinstance(other, Rdata) or \
diff --git a/dns/zone.py b/dns/zone.py
index 9c3204b..d154928 100644
--- a/dns/zone.py
+++ b/dns/zone.py
@@ -733,10 +733,11 @@ class Zone(dns.transaction.TransactionManager):
                     continue
                 rrfixed = struct.pack('!HHI', rdataset.rdtype,
                                       rdataset.rdclass, rdataset.ttl)
-                for rr in sorted(rdataset):
-                    rrdata = rr.to_digestable(self.origin)
-                    rrlen = struct.pack('!H', len(rrdata))
-                    hasher.update(rrnamebuf + rrfixed + rrlen + rrdata)
+                rdatas = [rdata.to_digestable(self.origin)
+                          for rdata in rdataset]
+                for rdata in sorted(rdatas):
+                    rrlen = struct.pack('!H', len(rdata))
+                    hasher.update(rrnamebuf + rrfixed + rrlen + rdata)
         return hasher.digest()
 
     def compute_digest(self, hash_algorithm, scheme=DigestScheme.SIMPLE):
diff --git a/tests/test_rdata.py b/tests/test_rdata.py
index 05ec6ca..ea3e3da 100644
--- a/tests/test_rdata.py
+++ b/tests/test_rdata.py
@@ -696,6 +696,52 @@ class RdataTestCase(unittest.TestCase):
                     rr = dns.rdata.from_text('IN', 'DNSKEY', input_variation)
                     new_text = rr.to_text(chunksize=chunksize)
                     self.assertEqual(output, new_text)
+                    
+    def test_relative_vs_absolute_compare(self):
+        r1 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'www.')
+        r2 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'www')
+        self.assertFalse(r1 == r2)
+        self.assertTrue(r1 != r2)
+        def bad1():
+            r1 < r2
+        def bad2():
+            r1 <= r2
+        def bad3():
+            r1 > r2
+        def bad4():
+            r1 >= r2
+        self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad1)
+        self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad2)
+        self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad3)
+        self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad4)
+
+    def test_absolute_vs_absolute_compare(self):
+        r1 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'www.')
+        r2 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'xxx.')
+        self.assertFalse(r1 == r2)
+        self.assertTrue(r1 != r2)
+        self.assertTrue(r1 < r2)
+        self.assertTrue(r1 <= r2)
+        self.assertFalse(r1 > r2)
+        self.assertFalse(r1 >= r2)
+
+    def test_relative_vs_relative_compare(self):
+        r1 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'www')
+        r2 = dns.rdata.from_text(dns.rdataclass.IN, dns.rdatatype.NS, 'xxx')
+        self.assertFalse(r1 == r2)
+        self.assertTrue(r1 != r2)
+        def bad1():
+            r1 < r2
+        def bad2():
+            r1 <= r2
+        def bad3():
+            r1 > r2
+        def bad4():
+            r1 >= r2
+        self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad1)
+        self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad2)
+        self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad3)
+        self.assertRaises(dns.rdata.NoRelativeRdataOrdering, bad4)
 
 
 class UtilTestCase(unittest.TestCase):
diff --git a/tests/test_zonedigest.py b/tests/test_zonedigest.py
index f98e5f7..d94be24 100644
--- a/tests/test_zonedigest.py
+++ b/tests/test_zonedigest.py
@@ -176,3 +176,18 @@ class ZoneDigestTestCase(unittest.TestCase):
         with self.assertRaises(dns.exception.SyntaxError):
             dns.rdata.from_text('IN', 'ZONEMD', '100 1 0 ' + self.sha384_hash)
 
+    sorting_zone = textwrap.dedent('''
+    @  86400  IN  SOA     ns1 admin 2018031900 (
+                                     1800 900 604800 86400 )
+              86400  IN  NS      ns1
+              86400  IN  NS      ns2
+              86400  IN  RP      n1.example. a.
+              86400  IN  RP      n1. b.
+    ''')
+    
+    def test_relative_zone_sorting(self):
+        z1 = dns.zone.from_text(self.sorting_zone, 'example.', relativize=True)
+        z2 = dns.zone.from_text(self.sorting_zone, 'example.', relativize=False)
+        zmd1 = z1.compute_digest(dns.zone.DigestHashAlgorithm.SHA384)
+        zmd2 = z2.compute_digest(dns.zone.DigestHashAlgorithm.SHA384)
+        self.assertEqual(zmd1, zmd2)