Change in vdsm[master]: guestagent: Speed up xml character filtering

nsoffer at redhat.com nsoffer at redhat.com
Wed Dec 23 05:23:26 UTC 2015


Nir Soffer has uploaded a new change for review.

Change subject: guestagent: Speed up xml character filtering
......................................................................

guestagent: Speed up xml character filtering

Use regular expression matching the invalid characters, simplifying the
code and speeding up filtering (40X):

    Before: 2.745 seconds
    After:  0.067 seconds

Also improve the documentation, explaining the logic, and mention the
original bug from 2010, which is the reason we need this filtering.

Change-Id: Ic7990dbe9787089b15f63e89de284c8695472b66
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
M tests/guestagentTests.py
M vdsm/virt/guestagent.py
2 files changed, 47 insertions(+), 48 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/45/50945/1

diff --git a/tests/guestagentTests.py b/tests/guestagentTests.py
index 778d36e..ae5cfc7 100644
--- a/tests/guestagentTests.py
+++ b/tests/guestagentTests.py
@@ -90,27 +90,28 @@
 class TestFiltering(TestCaseBase):
 
     @permutations([
-        [u""],
-        [u"ascii"],
-        [u"\u2122"],
+        [u"\u0009-\u000a"],
+        [u"\u000d"],
+        [u"\u0020-\u007e"],
+        [u"\u0085"],
+        [u"\u00a0-\ud7ff"],
+        [u"\ue000-\ufffd"],
+        [u"\u10000-\u10ffff"],
     ])
     def test_filter_xml_chars_valid(self, value):
         self.assertEqual(value, guestagent._filterXmlChars(value))
 
     @permutations([
-        [u"\u0000"],
-        [u"\uffff"],
-        [u"\ufffe"],
-        [u"\ud800"],
-        [u"\udc79"],
+        [u"\u0000-\u0008"],
+        [u"\u000b-\u000c"],
+        [u"\u000e-\u001f"],
+        [u"\u007f-\u0084"],
+        [u"\u0086-\u009f"],
+        [u"\ud800-\udfff"],
+        [u"\ufffe-\uffff"],
     ])
-    def test_filter_xml_chars_replace_invalid(self, value):
-        self.assertEqual(u'\ufffd', guestagent._filterXmlChars(value))
-
-    def test_filter_xml_chars_replace_restricted(self):
-        restricted = u''.join(guestagent._RESTRICTED_CHARS)
-        filtered = guestagent._REPLACEMENT_CHAR * len(restricted)
-        self.assertEqual(filtered, guestagent._filterXmlChars(restricted))
+    def test_filter_xml_chars_invalid(self, value):
+        self.assertEqual(u'\ufffd-\ufffd', guestagent._filterXmlChars(value))
 
     @slowtest
     def test_filter_xml_chars_timing(self):
diff --git a/vdsm/virt/guestagent.py b/vdsm/virt/guestagent.py
index 35a6fdb..81f7746 100644
--- a/vdsm/virt/guestagent.py
+++ b/vdsm/virt/guestagent.py
@@ -18,13 +18,12 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
-import array
 import logging
 import time
 import socket
 import errno
 import json
-import unicodedata
+import re
 
 from vdsm import supervdsm
 
@@ -37,42 +36,41 @@
     'set-number-of-cpus': 1}
 
 _REPLACEMENT_CHAR = u'\ufffd'
-_RESTRICTED_CHARS = frozenset(unichr(c) for c in
-                              list(range(8 + 1)) +
-                              list(range(0xB, 0xC + 1)) +
-                              list(range(0xE, 0x1F + 1)) +
-                              list(range(0x7F, 0x84 + 1)) +
-                              list(range(0x86, 0x9F + 1)) +
-                              [0xFFFE, 0xFFFF])
+
+# The set of characters allowed in XML documents is described in
+# http://www.w3.org/TR/xml11/#charsets
+#
+# Char is defined as any Unicode character, excluding the surrogate blocks,
+# FFFE, and FFFF:
+#
+#     [#x1-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]
+#
+# But according to bug 606281, we should also avoid RestrictedChar character
+# ranges:
+#
+#     [#x1-#x8] | [#xB-#xC] | [#xE-#x1F] | [#x7F-#x84] | [#x86-#x9F]
+#
+# The following ranges are the results of substructing the the RestrictedChar
+# ranges from Char ranges, and adding 0x00, FFFE, and FFFF. Any character in
+# these ranges will be replaced by the unicode replacement character.
+
+_FILTERED_CHARS = (
+    u"\u0000-\u0008"
+    u"\u000b-\u000c"
+    u"\u000e-\u001f"
+    u"\u007f-\u0084"
+    u"\u0086-\u009f"
+    u"\ud800-\udfff"
+    u"\ufffe-\uffff"
+)
+
+_filter_chars_re = re.compile(u'[%s]' % _FILTERED_CHARS)
 
 
 def _filterXmlChars(u):
-    """
-    The set of characters allowed in XML documents is described in
-    http://www.w3.org/TR/xml11/#charsets
-
-    "Char" is defined as any unicode character except the surrogate blocks,
-    \ufffe and \uffff.
-    "RestrictedChar" is defiend as the code points in _RESTRICTED_CHARS above
-
-    It's a little hard to follow, but the upshot is an XML document
-    must contain only characters in Char that are not in
-    RestrictedChar.
-
-    Note that Python's xmlcharrefreplace option is not relevant here -
-    that's about handling characters which can't be encoded in a given
-    charset encoding, not which aren't permitted in XML.
-    """
-
     if not isinstance(u, unicode):
         raise TypeError
-
-    chars = array.array('u', u)
-    for i, c in enumerate(chars):
-        if (c > u'\U00010fff' or unicodedata.category(c) == 'Cs'
-                or c in _RESTRICTED_CHARS):
-            chars[i] = _REPLACEMENT_CHAR
-    return chars.tounicode()
+    return _filter_chars_re.sub(_REPLACEMENT_CHAR, u)
 
 
 def _filterObject(obj):


-- 
To view, visit https://gerrit.ovirt.org/50945
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic7990dbe9787089b15f63e89de284c8695472b66
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list