From f2b3505d4313eb0c6a4f4a926c16f9e072d4e80b Mon Sep 17 00:00:00 2001
From: Ross McFarland <rwmcfa1@gmail.com>
Date: Fri, 4 Feb 2022 15:04:18 -0800
Subject: [PATCH] Falidate record fields that should hold FQDNs w/tests

---
 octodns/record/__init__.py   | 20 ++++++++++++++-----
 tests/test_octodns_record.py | 38 ++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py
index 9aaf111..287e1a9 100644
--- a/octodns/record/__init__.py
+++ b/octodns/record/__init__.py
@@ -1091,7 +1091,10 @@ class MxValue(EqualityTupleMixin):
             exchange = None
             try:
                 exchange = value.get('exchange', None) or value['value']
-                if not exchange.endswith('.'):
+                if not FQDN(str(exchange), allow_underscores=True).is_valid:
+                    reasons.append(f'Invalid MX exchange "{exchange}" is not '
+                                   'a valid FQDN.')
+                elif not exchange.endswith('.'):
                     reasons.append(f'MX value "{exchange}" missing trailing .')
             except KeyError:
                 reasons.append('missing exchange')
@@ -1225,7 +1228,10 @@ class _NsValue(object):
             data = (data,)
         reasons = []
         for value in data:
-            if not value.endswith('.'):
+            if not FQDN(str(value), allow_underscores=True).is_valid:
+                reasons.append(f'Invalid NS value "{value}" is not '
+                               'a valid FQDN.')
+            elif not value.endswith('.'):
                 reasons.append(f'NS value "{value}" missing trailing .')
         return reasons
 
@@ -1413,9 +1419,13 @@ class SrvValue(EqualityTupleMixin):
             except ValueError:
                 reasons.append(f'invalid port "{value["port"]}"')
             try:
-                if not value['target'].endswith('.'):
-                    reasons.append(f'SRV value "{value["target"]}" missing '
-                                   'trailing .')
+                target = value['target']
+                if not target.endswith('.'):
+                    reasons.append(f'SRV value "{target}" missing trailing .')
+                if target != '.' and \
+                   not FQDN(str(target), allow_underscores=True).is_valid:
+                    reasons.append(f'Invalid SRV target "{target}" is not '
+                                   'a valid FQDN.')
             except KeyError:
                 reasons.append('missing target')
         return reasons
diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py
index dd0f8de..2a60c7f 100644
--- a/tests/test_octodns_record.py
+++ b/tests/test_octodns_record.py
@@ -2694,6 +2694,19 @@ class TestRecordValidation(TestCase):
         self.assertEqual(['MX value "foo.bar.com" missing trailing .'],
                          ctx.exception.reasons)
 
+        # exchange must be a valid FQDN
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, '', {
+                'type': 'MX',
+                'ttl': 600,
+                'value': {
+                    'preference': 10,
+                    'exchange': '100 foo.bar.com.'
+                }
+            })
+        self.assertEqual(['Invalid MX exchange "100 foo.bar.com." is not a '
+                          'valid FQDN.'], ctx.exception.reasons)
+
     def test_NXPTR(self):
         # doesn't blow up
         Record.new(self.zone, '', {
@@ -2792,6 +2805,16 @@ class TestRecordValidation(TestCase):
         self.assertEqual(['NS value "foo.bar" missing trailing .'],
                          ctx.exception.reasons)
 
+        # exchange must be a valid FQDN
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, '', {
+                'type': 'NS',
+                'ttl': 600,
+                'value': '100 foo.bar.com.'
+            })
+        self.assertEqual(['Invalid NS value "100 foo.bar.com." is not a '
+                          'valid FQDN.'], ctx.exception.reasons)
+
     def test_PTR(self):
         # doesn't blow up (name & zone here don't make any sense, but not
         # important)
@@ -3109,6 +3132,21 @@ class TestRecordValidation(TestCase):
         self.assertEqual(['SRV value "foo.bar.baz" missing trailing .'],
                          ctx.exception.reasons)
 
+        # target must be a valid FQDN
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, '_srv._tcp', {
+                'type': 'SRV',
+                'ttl': 600,
+                'value': {
+                    'priority': 1,
+                    'weight': 2,
+                    'port': 3,
+                    'target': '100 foo.bar.com.'
+                }
+            })
+        self.assertEqual(['Invalid SRV target "100 foo.bar.com." is not a '
+                          'valid FQDN.'], ctx.exception.reasons)
+
     def test_TXT(self):
         # doesn't blow up (name & zone here don't make any sense, but not
         # important)