From 27992163ce7e1c2d063b08c9956f6545e2b841dd Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 24 May 2025 07:57:32 -0700 Subject: [PATCH 1/6] wip experiment with YamlProvider un-escaped semi-colons --- octodns/provider/yaml.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 1a2a512..4c704ac 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -73,6 +73,10 @@ class YamlProvider(BaseProvider): # (optional, default False) disable_zonefile: false + # Whether or not ; in values, e.g. TXT, are escaped \\; + # (optional, default True) + escaped_semicolons: True + Note ---- @@ -192,13 +196,14 @@ class YamlProvider(BaseProvider): split_catchall=True, shared_filename=False, disable_zonefile=False, + escaped_semicolons=True, *args, **kwargs, ): klass = self.__class__.__name__ self.log = logging.getLogger(f'{klass}[{id}]') self.log.debug( - '__init__: id=%s, directory=%s, default_ttl=%d, enforce_order=%d, order_mode=%s, populate_should_replace=%s, supports_root_ns=%s, split_extension=%s, split_catchall=%s, shared_filename=%s, disable_zonefile=%s', + '__init__: id=%s, directory=%s, default_ttl=%d, enforce_order=%d, order_mode=%s, populate_should_replace=%s, supports_root_ns=%s, split_extension=%s, split_catchall=%s, shared_filename=%s, disable_zonefile=%s, escaped_semicolons=%s', id, directory, default_ttl, @@ -210,6 +215,7 @@ class YamlProvider(BaseProvider): split_catchall, shared_filename, disable_zonefile, + escaped_semicolons, ) super().__init__(id, *args, **kwargs) self.directory = directory @@ -222,6 +228,7 @@ class YamlProvider(BaseProvider): self.split_catchall = split_catchall self.shared_filename = shared_filename self.disable_zonefile = disable_zonefile + self.escaped_semicolons = escaped_semicolons def copy(self): kwargs = dict(self.__dict__) @@ -326,6 +333,12 @@ class YamlProvider(BaseProvider): if not isinstance(data, list): data = [data] for d in data: + _type = d.get('type') + if not self.escaped_semicolons and _type in ('SPF', 'TXT'): + if 'value' in d: + d['value'] = d[''].replace(';', '\\;') + if 'values' in d: + d['values'] = [v.replace(';', '\\;') for v in d['values']] if 'ttl' not in d: d['ttl'] = self.default_ttl record = Record.new( From 777a1a655a03e7bdd7a9703cf2979d329ce2301b Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 25 May 2025 16:43:52 -0700 Subject: [PATCH 2/6] working escaped_semicolons=false w/tests --- .../7a30ee9fb9dd433c9181ddd5628e5cd3.md | 4 ++ octodns/provider/yaml.py | 11 ++++-- tests/config-semis/escaped.semis.yaml | 8 ++++ tests/config-semis/unescaped.semis.yaml | 9 +++++ tests/test_octodns_provider_yaml.py | 39 +++++++++++++++++++ 5 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 .changelog/7a30ee9fb9dd433c9181ddd5628e5cd3.md create mode 100644 tests/config-semis/escaped.semis.yaml create mode 100644 tests/config-semis/unescaped.semis.yaml diff --git a/.changelog/7a30ee9fb9dd433c9181ddd5628e5cd3.md b/.changelog/7a30ee9fb9dd433c9181ddd5628e5cd3.md new file mode 100644 index 0000000..5dc0db7 --- /dev/null +++ b/.changelog/7a30ee9fb9dd433c9181ddd5628e5cd3.md @@ -0,0 +1,4 @@ +--- +type: minor +--- +Add unescaped semicolons support to YamlProvider \ No newline at end of file diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 4c704ac..6fa318c 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -334,11 +334,16 @@ class YamlProvider(BaseProvider): data = [data] for d in data: _type = d.get('type') - if not self.escaped_semicolons and _type in ('SPF', 'TXT'): + if not self.escaped_semicolons and _type in ( + 'SPF', + 'TXT', + ): if 'value' in d: - d['value'] = d[''].replace(';', '\\;') + d['value'] = d['value'].replace(';', '\\;') if 'values' in d: - d['values'] = [v.replace(';', '\\;') for v in d['values']] + d['values'] = [ + v.replace(';', '\\;') for v in d['values'] + ] if 'ttl' not in d: d['ttl'] = self.default_ttl record = Record.new( diff --git a/tests/config-semis/escaped.semis.yaml b/tests/config-semis/escaped.semis.yaml new file mode 100644 index 0000000..634ed15 --- /dev/null +++ b/tests/config-semis/escaped.semis.yaml @@ -0,0 +1,8 @@ +--- +one: + type: TXT + value: This has a semi-colon\; that is escaped. +two: + type: TXT + values: + - This has a semi-colon too\; that is escaped. diff --git a/tests/config-semis/unescaped.semis.yaml b/tests/config-semis/unescaped.semis.yaml new file mode 100644 index 0000000..d1c2a8d --- /dev/null +++ b/tests/config-semis/unescaped.semis.yaml @@ -0,0 +1,9 @@ +--- +one: + type: TXT + value: This has a semi-colon; that isn't escaped. +two: + type: TXT + values: + - This has a semi-colon too; that isn't escaped. + - ; diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 77a4f1e..035439d 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -471,6 +471,45 @@ xn--dj-kia8a: # make sure that we get the idna one back self.assertEqual(idna, provider._zone_sources(zone)) + def test_unescaped_semicolons(self): + source = YamlProvider( + 'test', + join(dirname(__file__), 'config-semis'), + escaped_semicolons=False, + ) + + zone = Zone('unescaped.semis.', []) + source.populate(zone) + self.assertEqual(2, len(zone.records)) + one = next(r for r in zone.records if r.name == 'one') + self.assertTrue(one) + self.assertEqual( + ["This has a semi-colon\\; that isn't escaped."], one.values + ) + two = next(r for r in zone.records if r.name == 'two') + self.assertTrue(two) + self.assertEqual( + ["This has a semi-colon too\\; that isn't escaped.", '\\;'], + two.values, + ) + + zone = Zone('escaped.semis.', []) + source.populate(zone) + self.assertEqual(2, len(zone.records)) + one = next(r for r in zone.records if r.name == 'one') + self.assertTrue(one) + # self.assertEqual( + # ["This has a semi-colon\\; that isn't escaped."], one.values + # ) + two = next(r for r in zone.records if r.name == 'two') + self.assertTrue(two) + + +# self.assertEqual( +# ["This has a semi-colon too\\; that isn't escaped.", '\\;'], +# two.values, +# ) + class TestSplitYamlProvider(TestCase): def test_list_all_yaml_files(self): From 5c2e5d5a1aee0e4453ac6ab360f94d4b3fd3e131 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 25 May 2025 17:10:37 -0700 Subject: [PATCH 3/6] Double escaped semi-colon validation & tests --- .../85710a9264524662becdc7e52e71e241.md | 4 ++++ octodns/record/chunked.py | 3 +++ tests/config-semis/escaped.semis.yaml | 1 + tests/test_octodns_provider_yaml.py | 24 +++++++------------ tests/test_octodns_record_chunked.py | 6 +++++ 5 files changed, 23 insertions(+), 15 deletions(-) create mode 100644 .changelog/85710a9264524662becdc7e52e71e241.md diff --git a/.changelog/85710a9264524662becdc7e52e71e241.md b/.changelog/85710a9264524662becdc7e52e71e241.md new file mode 100644 index 0000000..85d1f8c --- /dev/null +++ b/.changelog/85710a9264524662becdc7e52e71e241.md @@ -0,0 +1,4 @@ +--- +type: minor +--- +Add validation to TXT records to check for double escaped semi-colons \ No newline at end of file diff --git a/octodns/record/chunked.py b/octodns/record/chunked.py index a07acfd..433163a 100644 --- a/octodns/record/chunked.py +++ b/octodns/record/chunked.py @@ -44,6 +44,7 @@ class _ChunkedValuesMixin(ValuesMixin): class _ChunkedValue(str): _unescaped_semicolon_re = re.compile(r'\w;') + _double_escaped_semicolon_re = re.compile(r'\\\\;') @classmethod def parse_rdata_text(cls, value): @@ -62,6 +63,8 @@ class _ChunkedValue(str): for value in data: if cls._unescaped_semicolon_re.search(value): reasons.append(f'unescaped ; in "{value}"') + if cls._double_escaped_semicolon_re.search(value): + reasons.append(f'double escaped ; in "{value}"') try: value.encode('ascii') except UnicodeEncodeError: diff --git a/tests/config-semis/escaped.semis.yaml b/tests/config-semis/escaped.semis.yaml index 634ed15..241c3f8 100644 --- a/tests/config-semis/escaped.semis.yaml +++ b/tests/config-semis/escaped.semis.yaml @@ -6,3 +6,4 @@ two: type: TXT values: - This has a semi-colon too\; that is escaped. + - \; diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 035439d..f60f741 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -15,6 +15,7 @@ from octodns.idna import idna_encode from octodns.provider import ProviderException from octodns.provider.yaml import SplitYamlProvider, YamlProvider from octodns.record import Create, NsValue, Record, ValuesMixin +from octodns.record.exception import ValidationError from octodns.zone import SubzoneRecordException, Zone @@ -494,21 +495,14 @@ xn--dj-kia8a: ) zone = Zone('escaped.semis.', []) - source.populate(zone) - self.assertEqual(2, len(zone.records)) - one = next(r for r in zone.records if r.name == 'one') - self.assertTrue(one) - # self.assertEqual( - # ["This has a semi-colon\\; that isn't escaped."], one.values - # ) - two = next(r for r in zone.records if r.name == 'two') - self.assertTrue(two) - - -# self.assertEqual( -# ["This has a semi-colon too\\; that isn't escaped.", '\\;'], -# two.values, -# ) + with self.assertRaises(ValidationError) as ctx: + source.populate(zone) + self.assertEqual( + [ + 'double escaped ; in "This has a semi-colon\\\\; that is escaped."' + ], + ctx.exception.reasons, + ) class TestSplitYamlProvider(TestCase): diff --git a/tests/test_octodns_record_chunked.py b/tests/test_octodns_record_chunked.py index 4d7bbc0..6b40752 100644 --- a/tests/test_octodns_record_chunked.py +++ b/tests/test_octodns_record_chunked.py @@ -59,6 +59,12 @@ class TestChunkedValue(TestCase): _ChunkedValue.validate('hello; world', 'TXT'), ) + # double escaped ; + self.assertEqual( + ['double escaped ; in "hello\\\\; world"'], + _ChunkedValue.validate('hello\\\\; world', 'TXT'), + ) + # non-asci self.assertEqual( ['non ASCII character in "v=spf1 –all"'], From b4b914d7370b4edb551c0e28d75ea84322932030 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 25 May 2025 17:11:45 -0700 Subject: [PATCH 4/6] Remove unused/errant copy of _unescaped_semicolon_re --- octodns/record/chunked.py | 1 - 1 file changed, 1 deletion(-) diff --git a/octodns/record/chunked.py b/octodns/record/chunked.py index 433163a..ab68806 100644 --- a/octodns/record/chunked.py +++ b/octodns/record/chunked.py @@ -9,7 +9,6 @@ from .base import ValuesMixin class _ChunkedValuesMixin(ValuesMixin): CHUNK_SIZE = 255 - _unescaped_semicolon_re = re.compile(r'\w;') def chunked_value(self, value): value = value.replace('"', '\\"') From d2da48c06371abe5c1df1fcf8eea00d6f664ac34 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 5 Jul 2025 18:25:29 -0700 Subject: [PATCH 5/6] apply support for YamlProvider.escaped_semicolons=false --- octodns/provider/yaml.py | 5 +++++ tests/test_octodns_provider_yaml.py | 22 ++++++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 6fa318c..5761da2 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -420,6 +420,11 @@ class YamlProvider(BaseProvider): if record.ttl == self.default_ttl: # ttl is the default, we don't need to store it del d['ttl'] + if not self.escaped_semicolons and record._type in ('SPF', 'TXT'): + if 'value' in d: + d['value'] = d['value'].replace('\\;', ';') + if 'values' in d: + d['values'] = [v.replace('\\;', ';') for v in d['values']] # we want to output the utf-8 version of the name data[record.decoded_name].append(d) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index f60f741..61c1e14 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -494,9 +494,9 @@ xn--dj-kia8a: two.values, ) - zone = Zone('escaped.semis.', []) + escaped = Zone('escaped.semis.', []) with self.assertRaises(ValidationError) as ctx: - source.populate(zone) + source.populate(escaped) self.assertEqual( [ 'double escaped ; in "This has a semi-colon\\\\; that is escaped."' @@ -504,6 +504,24 @@ xn--dj-kia8a: ctx.exception.reasons, ) + with TemporaryDirectory() as td: + # Add some subdirs to make sure that it can create them + target = YamlProvider( + 'target', td.dirname, escaped_semicolons=False + ) + yaml_file = join(td.dirname, 'unescaped.semis.yaml') + + plan = target.plan(zone) + target.apply(plan) + + with open(yaml_file) as fh: + content = fh.read() + self.assertTrue('value: This has a semi-colon; that' in content) + self.assertTrue( + "- This has a semi-colon too; that isn't escaped." in content + ) + self.assertTrue('- ;' in content) + class TestSplitYamlProvider(TestCase): def test_list_all_yaml_files(self): From 484a04b248aa2ff969e33105dd9fec579dcb7df9 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 9 Jul 2025 15:57:54 -0700 Subject: [PATCH 6/6] very slight tweak to working for escaped_semicolons docstring --- octodns/provider/yaml.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 5761da2..e56a2c6 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -73,7 +73,7 @@ class YamlProvider(BaseProvider): # (optional, default False) disable_zonefile: false - # Whether or not ; in values, e.g. TXT, are escaped \\; + # Whether or not ; in values, e.g. TXT, need to be escaped \\; # (optional, default True) escaped_semicolons: True