From f2b3505d4313eb0c6a4f4a926c16f9e072d4e80b Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 4 Feb 2022 15:04:18 -0800 Subject: [PATCH 1/3] 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) From 5fdb63ea079ba918e0611d44cb0487672dde9cf7 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 4 Feb 2022 15:21:39 -0800 Subject: [PATCH 2/3] Changelog entry for added fqdn validations --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 397f808..0d8ebcf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,11 @@ records created or updated using this version and matching the said edge-case will not be read/parsed correctly by the older version and will show a diff. +#### Stuff + +* Additional FQDN validation to ALIAS/CNAME value, MX exchange, SRV target and + tests of the functionality. + ## v0.9.14 - 2021-10-10 - A new supports system #### Noteworthy changes From 99da4abd9ffb00d948afef1f9a919a730ff36c32 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 4 Feb 2022 15:50:25 -0800 Subject: [PATCH 3/3] Include a CNAMe test for url with path too --- tests/test_octodns_record.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 2a60c7f..b01d91a 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -2291,6 +2291,16 @@ class TestRecordValidation(TestCase): self.assertEqual(['CNAME value "https://google.com" is not a valid ' 'FQDN'], ctx.exception.reasons) + # doesn't allow urls with paths + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'www', { + 'type': 'CNAME', + 'ttl': 600, + 'value': 'https://google.com/a/b/c', + }) + self.assertEqual(['CNAME value "https://google.com/a/b/c" is not a ' + 'valid FQDN'], ctx.exception.reasons) + # doesn't allow paths with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'www', {