Add function ZPublisher.utils.fix_properties. (#993)

* Add function ``ZPublisher.utils.fix_properties``.

You can call this to fix lines properties to only contain strings, not bytes.
It also replaces the deprecated property types ulines, utext, utoken, and ustring with their non-unicode variants.
See #987 for why this is needed.

Note: the code is not called by Zope itself.
The idea is that integrators who think they need this in their database, can call it.
Sample use:

    app.ZopeFindAndApply(app, apply_func=fix_properties)

I intend to use this (or a temporary copy) in the Plone 6 upgrade code.
See plone/Products.CMFPlone#3305

* Update src/ZPublisher/utils.py

Co-authored-by: Jens Vagelpohl <[email protected]>

* Update src/ZPublisher/utils.py

Co-authored-by: Jens Vagelpohl <[email protected]>

* Fix unicode properties: mark object as changed when _properties is changed.

Co-authored-by: Jens Vagelpohl <[email protected]>

CHANGES.rst

@@ -11,6 +11,12 @@ https://github.com/zopefoundation/Zope/blob/4.x/CHANGES.rst
 5.4 (unreleased)
 ----------------
 
+- Add function ``ZPublisher.utils.fix_properties``.
+  You can call this to fix lines properties to only contain strings, not bytes.
+  It also replaces the deprecated property types ulines, utext, utoken, and ustring
+  with their non-unicode variants.
+  See `issue 987 <https://github.com/zopefoundation/Zope/issues/987>`_.
+
 - Add support for Python 3.10.
 
 - Update to newest compatible versions of dependencies.

src/ZPublisher/tests/test_utils.py

@@ -34,3 +34,71 @@ def test_unicode(self):
     def test_utf_8(self):
         self.assertEqual(self._makeOne('test\xc2\xae'), 'test\xc2\xae')
         self.assertEqual(self._makeOne(b'test\xc2\xae'), 'test\xae')
+
+
+class FixPropertiesTests(unittest.TestCase):
+
+    def _makeOne(self):
+        from OFS.PropertyManager import PropertyManager
+
+        return PropertyManager()
+
+    def test_lines(self):
+        from ZPublisher.utils import fix_properties
+
+        obj = self._makeOne()
+        obj._setProperty("mixed", ["text and", b"bytes"], "lines")
+        self.assertEqual(obj.getProperty("mixed"), ("text and", b"bytes"))
+        self.assertEqual(obj.getPropertyType("mixed"), "lines")
+
+        fix_properties(obj)
+        self.assertEqual(obj.getProperty("mixed"), ("text and", "bytes"))
+        self.assertEqual(obj.getPropertyType("mixed"), "lines")
+
+    def test_ulines(self):
+        from ZPublisher.utils import fix_properties
+
+        obj = self._makeOne()
+        obj._setProperty("mixed", ["text and", b"bytes"], "ulines")
+        self.assertEqual(obj.getProperty("mixed"), ("text and", b"bytes"))
+        self.assertEqual(obj.getPropertyType("mixed"), "ulines")
+
+        fix_properties(obj)
+        self.assertEqual(obj.getProperty("mixed"), ("text and", "bytes"))
+        self.assertEqual(obj.getPropertyType("mixed"), "lines")
+
+    def test_utokens(self):
+        from ZPublisher.utils import fix_properties
+
+        obj = self._makeOne()
+        obj._setProperty("mixed", ["text", "and", b"bytes"], "utokens")
+        self.assertEqual(obj.getProperty("mixed"), ("text", "and", b"bytes"))
+        self.assertEqual(obj.getPropertyType("mixed"), "utokens")
+
+        fix_properties(obj)
+        self.assertEqual(obj.getProperty("mixed"), ("text", "and", "bytes"))
+        self.assertEqual(obj.getPropertyType("mixed"), "tokens")
+
+    def test_utext(self):
+        from ZPublisher.utils import fix_properties
+
+        obj = self._makeOne()
+        obj._setProperty("prop1", "multiple\nlines", "utext")
+        self.assertEqual(obj.getProperty("prop1"), "multiple\nlines")
+        self.assertEqual(obj.getPropertyType("prop1"), "utext")
+
+        fix_properties(obj)
+        self.assertEqual(obj.getProperty("prop1"), "multiple\nlines")
+        self.assertEqual(obj.getPropertyType("prop1"), "text")
+
+    def test_ustring(self):
+        from ZPublisher.utils import fix_properties
+
+        obj = self._makeOne()
+        obj._setProperty("prop1", "single line", "ustring")
+        self.assertEqual(obj.getProperty("prop1"), "single line")
+        self.assertEqual(obj.getPropertyType("prop1"), "ustring")
+
+        fix_properties(obj)
+        self.assertEqual(obj.getProperty("prop1"), "single line")
+        self.assertEqual(obj.getPropertyType("prop1"), "string")

src/ZPublisher/utils.py

@@ -19,6 +19,7 @@
 from Acquisition import aq_parent
 
 
+logger = logging.getLogger('ZPublisher')
 AC_LOGGER = logging.getLogger('event.AccessControl')
 
 
@@ -100,3 +101,118 @@ def basic_auth_decode(token):
     plain = plain.decode('latin-1')
     user, password = plain.split(':', 1)  # Split at most once
     return (user, password)
+
+
+def _string_tuple(value):
+    if not value:
+        return ()
+    return tuple([safe_unicode(element) for element in value])
+
+
+def fix_properties(obj, path=None):
+    """Fix properties on object.
+
+    This does two things:
+
+    1. Make sure lines contain only strings, instead of bytes,
+       or worse: a combination of strings and bytes.
+    2. Replace deprecated ulines, utext, utoken, and ustring properties
+       with their non-unicode variant, using native strings.
+
+    See https://github.com/zopefoundation/Zope/issues/987
+
+    Since Zope 5.3, a lines property stores strings instead of bytes.
+    But there is no migration yet.  (We do that here.)
+    Result is that getProperty on an already created lines property
+    will return the old value with bytes, but a newly created lines property
+    will return strings.  And you might get combinations.
+
+    Also since Zope 5.3, the ulines property type is deprecated.
+    You should use a lines property instead.
+    Same for a few others: utext, utoken, ustring.
+    The unicode variants are planned to be removed in Zope 6.
+
+    Intended usage:
+    app.ZopeFindAndApply(app, apply_func=fix_properties)
+    """
+    if path is None:
+        # When using ZopeFindAndApply, path is always given.
+        # But we may be called by other code.
+        if hasattr(object, 'getPhysicalPath'):
+            path = '/'.join(object.getPhysicalPath())
+        else:
+            # Some simple object, for example in tests.
+            # We don't care about the path then, it is only shown in logs.
+            path = "/dummy"
+
+    try:
+        prop_map = obj.propertyMap()
+    except AttributeError:
+        # Object does not inherit from PropertyManager.
+        # For example 'MountedObject'.
+        return
+
+    for prop_info in prop_map:
+        # Example: {'id': 'title', 'type': 'string', 'mode': 'w'}
+        prop_id = prop_info.get("id")
+        current = obj.getProperty(prop_id)
+        if current is None:
+            continue
+        new_type = prop_type = prop_info.get("type")
+        if prop_type == "lines":
+            new = _string_tuple(current)
+        elif prop_type == "ulines":
+            new_type = "lines"
+            new = _string_tuple(current)
+        elif prop_type == "utokens":
+            new_type = "tokens"
+            new = _string_tuple(current)
+        elif prop_type == "utext":
+            new_type = "text"
+            new = safe_unicode(current)
+        elif prop_type == "ustring":
+            new_type = "string"
+            new = safe_unicode(current)
+        else:
+            continue
+        if prop_type != new_type:
+            # Replace with non-unicode variant.
+            # This could easily lead to:
+            # Exceptions.BadRequest: Invalid or duplicate property id.
+            #   obj._delProperty(prop_id)
+            #   obj._setProperty(prop_id, new, new_type)
+            # So fix it by using internal details.
+            for prop in obj._properties:
+                if prop.get("id") == prop_id:
+                    prop["type"] = new_type
+                    obj._p_changed = True
+                    break
+            else:
+                # This probably cannot happen.
+                # If it does, we want to know.
+                logger.warning(
+                    "Could not change property %s from %s to %s for %s",
+                    prop_id,
+                    prop_type,
+                    new_type,
+                    path,
+                )
+                continue
+            obj._updateProperty(prop_id, new)
+            logger.info(
+                "Changed property %s from %s to %s for %s",
+                prop_id,
+                prop_type,
+                new_type,
+                path,
+            )
+            continue
+        if current != new:
+            obj._updateProperty(prop_id, new)
+            logger.info(
+                "Changed property %s at %s so value fits the type %s: %r",
+                prop_id,
+                path,
+                prop_type,
+                new,
+            )