From 49e9574fe892cb097453e102c8f09ac7125598aa Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Thu, 2 Dec 2021 23:52:21 +0100 Subject: [PATCH] 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 https://github.com/zopefoundation/Zope/issues/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 https://github.com/plone/Products.CMFPlone/issues/3305 * Update src/ZPublisher/utils.py Co-authored-by: Jens Vagelpohl * Update src/ZPublisher/utils.py Co-authored-by: Jens Vagelpohl * Fix unicode properties: mark object as changed when _properties is changed. Co-authored-by: Jens Vagelpohl --- CHANGES.rst | 6 ++ src/ZPublisher/tests/test_utils.py | 68 +++++++++++++++++ src/ZPublisher/utils.py | 116 +++++++++++++++++++++++++++++ 3 files changed, 190 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 9bdbf0f6d0..328e7d981d 100644 --- a/CHANGES.rst +++ b/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 `_. + - Add support for Python 3.10. - Update to newest compatible versions of dependencies. diff --git a/src/ZPublisher/tests/test_utils.py b/src/ZPublisher/tests/test_utils.py index 91844d0be1..d6d6c5689d 100644 --- a/src/ZPublisher/tests/test_utils.py +++ b/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") diff --git a/src/ZPublisher/utils.py b/src/ZPublisher/utils.py index 76f23a426d..7174e6f3e2 100644 --- a/src/ZPublisher/utils.py +++ b/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, + )