Address Serhiy's review

vstinner · vstinner · commit b2ffbe438a95 · 2024-07-01T10:05:34.000+02:00
diff --git a/Lib/test/libregrtest/testresult.py b/Lib/test/libregrtest/testresult.py
@@ -9,7 +9,7 @@
 import traceback
 import unittest
 from test import support
-from test.libregrtest.utils import escape_xml
+from test.libregrtest.utils import sanitize_xml
 
 class RegressionTestResult(unittest.TextTestResult):
     USE_XML = False
@@ -66,10 +66,10 @@ def _add_result(self, test, capture=False, **args):
         if capture:
             if self._stdout_buffer is not None:
                 stdout = self._stdout_buffer.getvalue().rstrip()
-                ET.SubElement(e, 'system-out').text = escape_xml(stdout)
+                ET.SubElement(e, 'system-out').text = sanitize_xml(stdout)
             if self._stderr_buffer is not None:
                 stderr = self._stderr_buffer.getvalue().rstrip()
-                ET.SubElement(e, 'system-err').text = escape_xml(stderr)
+                ET.SubElement(e, 'system-err').text = sanitize_xml(stderr)
 
         for k, v in args.items():
             if not k or not v:
@@ -79,11 +79,11 @@ def _add_result(self, test, capture=False, **args):
             if hasattr(v, 'items'):
                 for k2, v2 in v.items():
                     if k2:
-                        e2.set(k2, escape_xml(str(v2)))
+                        e2.set(k2, sanitize_xml(str(v2)))
                     else:
-                        e2.text = escape_xml(str(v2))
+                        e2.text = sanitize_xml(str(v2))
             else:
-                e2.text = escape_xml(str(v))
+                e2.text = sanitize_xml(str(v))
 
     @classmethod
     def __makeErrorDict(cls, err_type, err_value, err_tb):
diff --git a/Lib/test/libregrtest/utils.py b/Lib/test/libregrtest/utils.py
@@ -717,15 +717,20 @@ def get_signal_name(exitcode):
 
 ILLEGAL_XML_CHARS_RE = re.compile(
     '['
-    '\x00-\x1F'      # ASCII control characters
-    '\uD800-\uDFFF'  # surrogate characters
+    # Control characters; newline (\x0A and \x0D) and TAB (\x09) are legal
+    '\x00-\x08\x0B\x0C\x0E-\x1F'
+    # Surrogate characters
+    '\uD800-\uDFFF'
+    # Special Unicode characters
     '\uFFFE'
     '\uFFFF'
-    ']')
+    # Match multiple sequential invalid characters for better effiency
+    ']+')
 
-def _escape_xml_replace(regs):
-    code_point = ord(regs[0])
-    return f"&#{code_point};"
+def _sanitize_xml_replace(regs):
+    text = regs[0]
+    return ''.join(f'\\x{ord(ch):02x}' if ch <= '\xff' else ascii(ch)[1:-1]
+                   for ch in text)
 
-def escape_xml(text):
-    return ILLEGAL_XML_CHARS_RE.sub(_escape_xml_replace, text)
+def sanitize_xml(text):
+    return ILLEGAL_XML_CHARS_RE.sub(_sanitize_xml_replace, text)
diff --git a/Lib/test/test_regrtest.py b/Lib/test/test_regrtest.py
@@ -2292,7 +2292,7 @@ def test_failed(self):
         self.assertEqual(testcase.get('result'), 'completed')
         self.assertGreater(float(testcase.get('time')), 0)
         for out in testcase.iter('system-out'):
-            self.assertEqual(out.text, "abc &#27; def")
+            self.assertEqual(out.text, r"abc \x1b def")
 
 
 class TestUtils(unittest.TestCase):
@@ -2477,21 +2477,23 @@ def id(self):
             self.assertTrue(match_test(test_chdir))
             self.assertFalse(match_test(test_copy))
 
-    def test_escape_xml(self):
-        escape_xml = utils.escape_xml
+    def test_sanitize_xml(self):
+        sanitize_xml = utils.sanitize_xml
 
         # escape invalid XML characters
-        self.assertEqual(escape_xml('abc \x1b def'),
-                         'abc &#27; def')
-        self.assertEqual(escape_xml('nul:\x00, bell:\x07'),
-                         'nul:&#0;, bell:&#7;')
-        self.assertEqual(escape_xml('surrogate:\uDC80'),
-                         'surrogate:&#56448;')
-        self.assertEqual(escape_xml('illegal \uFFFE and \uFFFF'),
-                         'illegal &#65534; and &#65535;')
+        self.assertEqual(sanitize_xml('abc \x1b\x1f def'),
+                         r'abc \x1b\x1f def')
+        self.assertEqual(sanitize_xml('nul:\x00, bell:\x07'),
+                         r'nul:\x00, bell:\x07')
+        self.assertEqual(sanitize_xml('surrogate:\uDC80'),
+                         r'surrogate:\udc80')
+        self.assertEqual(sanitize_xml('illegal \uFFFE and \uFFFF'),
+                         r'illegal \ufffe and \uffff')
 
         # no escape for valid XML characters
-        self.assertEqual(escape_xml('valid t\xe9xt \u20ac'),
+        self.assertEqual(sanitize_xml('a\n\tb'),
+                         'a\n\tb')
+        self.assertEqual(sanitize_xml('valid t\xe9xt \u20ac'),
                          'valid t\xe9xt \u20ac')
 
 
