Fix double escaping in string-list SVCB parameter to_text() (#1280)

The list-level _escapify escaped non-printable bytes into \ddd, and
dns.rdata._escapify() then escaped that backslash, emitting \ddd,
which parses back as the literal characters 'ddd'.  Make the inner
escape purely list-level (',' and '\') and let dns.rdata._escapify()
apply the character-string escaping once.

diff --git a/dns/rdtypes/svcbbase.py b/dns/rdtypes/svcbbase.py
index 0acdd6469a36caf3651893a5ffc1d0fd709f417f..b4aff6ebd5d08feb7862ae1d737f1bcecf055c46 100644 (file)
--- a/dns/rdtypes/svcbbase.py
+++ b/dns/rdtypes/svcbbase.py
@@ -82,20 +82,18 @@ def key_to_text(key):
     return ParamKey.to_text(key).replace("_", "-").lower()
 
 
-# Like rdata escapify, but escapes ',' too.
+# Escape the characters which are special in a comma-separated list; the
+# character-string escaping (dns.rdata._escapify) is applied on top of this.
 
-_escaped = b'",\\'
+_escaped = b",\\"
 
 
-def _escapify(qstring):
-    text = ""
+def _escapify(qstring: bytes) -> bytes:
+    text = b""
     for c in qstring:
         if c in _escaped:
-            text += "\\" + chr(c)
-        elif c >= 0x20 and c < 0x7F:
-            text += chr(c)
-        else:
-            text += f"\\{c:03d}"
+            text += b"\\"
+        text += b"%c" % (c)
     return text
 
 
@@ -257,8 +255,8 @@ class _StringList(Param):
         return cls(_split(_unescape(value)))
 
     def to_text(self):
-        value = ",".join([_escapify(id) for id in self.ids])
-        return '"' + dns.rdata._escapify(value.encode()) + '"'
+        value = b",".join([_escapify(id) for id in self.ids])
+        return '"' + dns.rdata._escapify(value) + '"'
 
     @classmethod
     def from_wire_parser(cls, parser, origin=None):  # pylint: disable=W0613

diff --git a/doc/whatsnew.rst b/doc/whatsnew.rst
index 40a45a48c92f78671d198adaebb21adbf3284c70..51364cfc6264059d240c0e7761ecec6b014e4243 100644 (file)
--- a/doc/whatsnew.rst
+++ b/doc/whatsnew.rst
@@ -25,6 +25,11 @@ TBD
   rejected by ``int()``, so they previously escaped the parser's validation.  This
   also makes zone files with such a TTL fail with a clean dns.exception.SyntaxError.
 
+* The to_text() of string-list SVCB parameters (alpn and docpath) escaped
+  non-printable bytes twice, rendering them as ``\\ddd`` instead of ``\ddd``,
+  so the output did not parse back to the original value.  Such bytes are now
+  escaped once, at the character-string level.
+
 2.8.0
 -----
 

diff --git a/tests/test_svcb.py b/tests/test_svcb.py
index 9b217f7d47a1894a95ab487a7caaef29c878a4de..ee04ba8d89aeb62ea0041437fd00533be14b7920 100644 (file)
--- a/tests/test_svcb.py
+++ b/tests/test_svcb.py
@@ -120,6 +120,46 @@ class SVCBTestCase(unittest.TestCase):
         )
         self.check_invalid_inputs(invalid_inputs)
 
+    def test_svcb_alpn_escaping(self):
+        # Bytes outside 0x20-0x7e are escaped once, at the character-string
+        # level, as they don't need any escaping at the list level.
+        valid_inputs_non_printable = (
+            '1 . alpn="h\\0002,h3"',
+            "1 . alpn=h\\0002,h3",
+            "1 . key1=\\003h\\0002\\002h3",
+        )
+        self.check_valid_inputs(valid_inputs_non_printable)
+
+        valid_inputs_high_bit = (
+            '1 . alpn="\\255h2"',
+            "1 . alpn=\\255h2",
+            "1 . key1=\\003\\255h2",
+        )
+        self.check_valid_inputs(valid_inputs_high_bit)
+
+        valid_inputs_quote = (
+            '1 . alpn="h\\"2"',
+            "1 . alpn=h\\0342",
+            "1 . key1=\\003h\\0342",
+        )
+        self.check_valid_inputs(valid_inputs_quote)
+
+        # to_text() output must parse back to the same value, both from
+        # text and from wire.
+        for ids in (
+            (b"h\x002",),
+            (b"\xff", b"h2"),
+            (b"h,3", b"h\\3"),
+            (b'h"2',),
+            (b"h2", b"h3"),
+        ):
+            param = dns.rdtypes.svcbbase.ALPNParam(ids)
+            rr = dns.rdata.from_text("IN", "SVCB", "1 . alpn=" + param.to_text())
+            alpn = rr.params[dns.rdtypes.svcbbase.ParamKey.ALPN]
+            self.assertEqual(alpn.ids, ids)
+            rr2 = dns.rdata.from_text("IN", "SVCB", rr.to_generic().to_text())
+            self.assertEqual(rr2, rr)
+
     def test_svcb_no_default_alpn(self):
         valid_inputs = (
             '1 . alpn="h2" no-default-alpn',
@@ -268,6 +308,15 @@ class SVCBTestCase(unittest.TestCase):
         )
         self.check_invalid_inputs(invalid_inputs)
 
+    def test_svcb_docpath_escaping(self):
+        # docpath shares ALPN's string list escaping.
+        valid_inputs = (
+            '1 . docpath="n\\000s,t"',
+            "1 . docpath=n\\000s,t",
+            "1 . key10=\\003n\\000s\\001t",
+        )
+        self.check_valid_inputs(valid_inputs)
+
     def test_svcb_unknown(self):
         valid_inputs_one_key = (
             '1 . key23="key45"',
@@ -340,8 +389,9 @@ class SVCBTestCase(unittest.TestCase):
 
     def test_misc_escape(self):
         rdata = dns.rdata.from_text("in", "svcb", "1 . alpn=\\010\\010")
-        expected = '1 . alpn="\\\\010\\\\010"'
+        expected = '1 . alpn="\\010\\010"'
         self.assertEqual(rdata.to_text(), expected)
+        self.assertEqual(dns.rdata.from_text("in", "svcb", expected), rdata)
         with self.assertRaises(dns.exception.SyntaxError):
             dns.rdata.from_text("in", "svcb", "1 . alpn=\\0")
         with self.assertRaises(dns.exception.SyntaxError):