Merge pull request #1203 from octodns/double-dots

Record and Zone name validations for double dots

CHANGELOG.md

@@ -1,5 +1,8 @@
 ## v1.?.? - 2024-??-?? - ???
 
+* Zone name validation checking for double dots, and throwing InvalidNameError
+  rather than base Exception
+* Record validation checks for double dots in names
 * MetaProcessor.include_extra to add support for arbitrary extra values to be
   set on the meta record.
 * Correctly handled quoted svcparams when parsing SVCB/HTTPS rdata text

octodns/record/base.py

@@ -102,6 +102,9 @@ class Record(EqualityTupleMixin):
                     f'invalid label, "{label}" is too long at {n}'
                     ' chars, max is 63'
                 )
+        # in the case of endswith there's an implicit second . from the Zone
+        if '..' in name or name.endswith('.'):
+            reasons.append(f'invalid name, double `.` in "{idna_decode(fqdn)}"')
         # TODO: look at the idna lib for a lot more potential validations...
         try:
             ttl = int(data['ttl'])

octodns/zone.py

@@ -53,6 +53,10 @@ class InvalidNodeException(Exception):
         super().__init__(msg)
 
 
+class InvalidNameError(Exception):
+    pass
+
+
 class Zone(object):
     log = getLogger('Zone')
 
@@ -64,9 +68,17 @@ class Zone(object):
         delete_pcent_threshold=None,
     ):
         if not name[-1] == '.':
-            raise Exception(f'Invalid zone name {name}, missing ending dot')
+            raise InvalidNameError(
+                f'Invalid zone name {name}, missing ending dot'
+            )
+        elif '..' in name:
+            raise InvalidNameError(
+                f'Invalid zone name {name}, double dot not allowed'
+            )
         elif ' ' in name or '\t' in name:
-            raise Exception(f'Invalid zone name {name}, whitespace not allowed')
+            raise InvalidNameError(
+                f'Invalid zone name {name}, whitespace not allowed'
+            )
 
         # internally everything is idna
         self.name = idna_encode(str(name)) if name else name

tests/test_octodns_processor_filter.py

@@ -487,7 +487,9 @@ class TestZoneNameFilter(TestCase):
         with_dot = zone.copy()
         with_dot.add_record(
             Record.new(
-                zone, zone.name, {'type': 'A', 'ttl': 43, 'value': '1.2.3.4'}
+                zone,
+                zone.name[:-1],
+                {'type': 'A', 'ttl': 43, 'value': '1.2.3.4'},
             )
         )
         self.assertEqual(3, len(with_dot.records))
@@ -514,7 +516,9 @@ class TestZoneNameFilter(TestCase):
         zone = Zone('unit.tests.', [])
         zone.add_record(
             Record.new(
-                zone, zone.name, {'type': 'A', 'ttl': 43, 'value': '1.2.3.4'}
+                zone,
+                zone.name[:-1],
+                {'type': 'A', 'ttl': 43, 'value': '1.2.3.4'},
             )
         )
         with self.assertRaises(ValidationError) as ctx:

tests/test_octodns_record.py

@@ -616,6 +616,7 @@ class TestRecordValidation(TestCase):
 
         # should not raise with dots
         name = 'xxxxxxxx.' * 10
+        name = name[:-1]
         Record.new(
             self.zone, name, {'ttl': 300, 'type': 'A', 'value': '1.2.3.4'}
         )
@@ -665,6 +666,32 @@ class TestRecordValidation(TestCase):
         # does)
         self.assertEqual('Label too long', reason)
 
+        # double dots are not valid, ends with
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(
+                self.zone,
+                'this.ends.with.a.dot.',
+                {'ttl': 301, 'type': 'A', 'value': '1.2.3.4'},
+            )
+        reason = ctx.exception.reasons[0]
+        self.assertEqual(
+            'invalid name, double `.` in "this.ends.with.a.dot..unit.tests."',
+            reason,
+        )
+
+        # double dots are not valid when eplxicit
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(
+                self.zone,
+                'this.has.double..dots',
+                {'ttl': 301, 'type': 'A', 'value': '1.2.3.4'},
+            )
+        reason = ctx.exception.reasons[0]
+        self.assertEqual(
+            'invalid name, double `.` in "this.has.double..dots.unit.tests."',
+            reason,
+        )
+
         # no ttl
         with self.assertRaises(ValidationError) as ctx:
             Record.new(self.zone, '', {'type': 'A', 'value': '1.2.3.4'})

tests/test_octodns_zone.py

@@ -19,6 +19,7 @@ from octodns.record import (
 )
 from octodns.zone import (
     DuplicateRecordException,
+    InvalidNameError,
     InvalidNodeException,
     SubzoneRecordException,
     Zone,
@@ -247,12 +248,21 @@ class TestZone(TestCase):
         self.assertIsInstance(changes[0], Delete)
 
     def test_missing_dot(self):
-        with self.assertRaises(Exception) as ctx:
+        with self.assertRaises(InvalidNameError) as ctx:
             Zone('not.allowed', [])
         self.assertTrue('missing ending dot' in str(ctx.exception))
 
+    def test_double_dot(self):
+        with self.assertRaises(InvalidNameError) as ctx:
+            Zone('ending.double.dot..', [])
+        self.assertTrue('double dot not allowed' in str(ctx.exception))
+
+        with self.assertRaises(InvalidNameError) as ctx:
+            Zone('mid.double..dot.', [])
+        self.assertTrue('double dot not allowed' in str(ctx.exception))
+
     def test_whitespace(self):
-        with self.assertRaises(Exception) as ctx:
+        with self.assertRaises(InvalidNameError) as ctx:
             Zone('space not allowed.', [])
         self.assertTrue('whitespace not allowed' in str(ctx.exception))