Repository: jsonpickle/jsonpickle
Pull request #514 (2024-06-29 08:13:36)

tags: use py/mod tags for modules instead of py/repr
A continuation of the ideas from #513 which we can refine after that PR is merged.

davvid: I squashed in the typofix you committed and also squashed in a further update to the changelog that calls out the `safe=...` change as a breaking change.

Since I think this might warrant a major version bump we can hold onto these changes here and defer merging them until after the next v3.x release. We should probably let #515 play out and once once that's merged we'll tag the next v3.x release. These changes can follow behind that merge as the starting point for v4.x.
davvid: @Theelx I almost forgot about this one ;-)

I just rebased this against the latest and I think this is more or less about ready to go if you're still cool with the caveats we discussed in this PR.

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 23c8410..304e816 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -1,3 +1,16 @@
+Upcoming
+========
+
+    * **Breaking Change**: Support for pre-0.7.0 ``repr``-serialized objects is no
+      longer enabled by default. The ``safe`` option to ``decode()`` was changed from
+      ``False`` to ``True``. Users can still pass ``safe=False`` to ``decode()`` in order
+      to enable this feature for the purposes of loading older files, but beware that
+      this feature relies on unsafe behavior through its use of ``eval()``. Users are
+      encouraged to re-pickle old data in order to migrate away from the the unsafe loading
+      feature. (+514)
+    * The pickler no longer produces ``py/repr`` tags when pickling modules.
+      ``py/mod`` is used instead, as it is clearer and uses one less byte. (+514)
+
 v3.3.0
 ======
     * The unpickler was updated to avoid using ``eval``, which helps improve its
diff --git a/jsonpickle/pickler.py b/jsonpickle/pickler.py
index 75a8e92..9e74e50 100644
--- a/jsonpickle/pickler.py
+++ b/jsonpickle/pickler.py
@@ -670,7 +670,7 @@ class Pickler(object):
 
         if util.is_module(obj):
             if self.unpicklable:
-                data[tags.REPR] = '{name}/{name}'.format(name=obj.__name__)
+                data[tags.MODULE] = '{name}/{name}'.format(name=obj.__name__)
             else:
                 data = compat.ustr(obj)
             return data
diff --git a/jsonpickle/tags.py b/jsonpickle/tags.py
index 9f2bb22..d6b346f 100644
--- a/jsonpickle/tags.py
+++ b/jsonpickle/tags.py
@@ -15,6 +15,7 @@ ID = 'py/id'
 INITARGS = 'py/initargs'
 ITERATOR = 'py/iterator'
 JSON_KEY = 'json://'
+MODULE = 'py/mod'
 NEWARGS = 'py/newargs'
 NEWARGSEX = 'py/newargsex'
 NEWOBJ = 'py/newobj'
@@ -36,6 +37,7 @@ RESERVED = {
     ID,
     INITARGS,
     ITERATOR,
+    MODULE,
     NEWARGS,
     NEWARGSEX,
     NEWOBJ,
diff --git a/jsonpickle/unpickler.py b/jsonpickle/unpickler.py
index ebbad8e..bdbb223 100644
--- a/jsonpickle/unpickler.py
+++ b/jsonpickle/unpickler.py
@@ -19,7 +19,7 @@ def decode(
     context=None,
     keys=False,
     reset=True,
-    safe=False,
+    safe=True,
     classes=None,
     v1_decode=False,
     on_missing='ignore',
@@ -45,8 +45,13 @@ def decode(
         This flag is not typically used outside of a custom handler or
         `__getstate__` implementation.
 
-    :param safe: If set to True, eval() is avoided, but backwards-compatible
-        (pre-0.7.0) deserialization of repr-serialized objects is disabled.
+    :param safe: If set to False, use of ``eval()`` for backwards-compatible (pre-0.7.0)
+        deserialization of repr-serialized objects is enabled. Defaults to True.
+
+        .. warning::
+
+            ``eval()`` is used when set to ``False`` and is not secure against
+            malicious inputs. You should avoid setting ``safe=False``.
 
     :param classes: If set to a single class, or a sequence (list, set, tuple) of
         classes, then the classes will be made available when constructing objects.
@@ -296,10 +301,10 @@ def loadrepr(reprstr):
     return eval(evalstr, mylocals)
 
 
-def loadmodule(module_str):
+def _loadmodule(module_str):
     """Returns a reference to a module.
 
-    >>> fn = loadmodule('datetime/datetime.datetime.fromtimestamp')
+    >>> fn = _loadmodule('datetime/datetime.datetime.fromtimestamp')
     >>> fn.__name__
     'fromtimestamp'
 
@@ -341,7 +346,7 @@ class Unpickler(object):
         self,
         backend=None,
         keys=False,
-        safe=False,
+        safe=True,
         v1_decode=False,
         on_missing='ignore',
         handle_readonly=False,
@@ -573,11 +578,16 @@ class Unpickler(object):
             return obj
         return typeref
 
+    def _restore_module(self, obj):
+        obj = _loadmodule(obj[tags.MODULE])
+        return self._mkref(obj)
+
+    def _restore_repr_safe(self, obj):
+        obj = _loadmodule(obj[tags.REPR])
+        return self._mkref(obj)
+
     def _restore_repr(self, obj):
-        if self.safe:
-            # eval() is not allowed in safe mode
-            return None
-        obj = loadmodule(obj[tags.REPR])
+        obj = loadrepr(obj[tags.REPR])
         return self._mkref(obj)
 
     def _loadfactory(self, obj):
@@ -919,8 +929,13 @@ class Unpickler(object):
                 restore = self._restore_reduce
             elif tags.FUNCTION in obj:
                 restore = self._restore_function
-            elif tags.REPR in obj:  # Backwards compatibility
-                restore = self._restore_repr
+            elif tags.MODULE in obj:
+                restore = self._restore_module
+            elif tags.REPR in obj:
+                if self.safe:
+                    restore = self._restore_repr_safe
+                else:
+                    restore = self._restore_repr
             else:
                 restore = self._restore_dict
         elif util.is_list(obj):


Test patch:
diff --git a/tests/datetime_test.py b/tests/datetime_test.py
index e51cc11..d6c5ef8 100644
--- a/tests/datetime_test.py
+++ b/tests/datetime_test.py
@@ -241,9 +241,10 @@ class DateTimeAdvancedTestCase(unittest.TestCase):
         obj = datetime.datetime.now()
         pickler = jsonpickle.pickler.Pickler(unpicklable=False)
         flattened = pickler.flatten(obj)
-        self.assertFalse(tags.REPR in flattened)
-        self.assertFalse(tags.OBJECT in flattened)
-        self.assertEqual(obj.isoformat(), flattened)
+        assert tags.REPR not in flattened
+        assert tags.MODULE not in flattened
+        assert tags.OBJECT not in flattened
+        assert obj.isoformat() == flattened
 
     def test_datetime_dict_keys_defaults(self):
         """Test that we handle datetime objects as keys."""
diff --git a/tests/jsonpickle_test.py b/tests/jsonpickle_test.py
index 01e417b..fccf99c 100644
--- a/tests/jsonpickle_test.py
+++ b/tests/jsonpickle_test.py
@@ -404,7 +404,7 @@ class PicklingTestCase(unittest.TestCase):
         flattened = self.pickler.flatten(obj)
         self.unpickler.safe = True
         inflated = self.unpickler.restore(flattened)
-        self.assertEqual(inflated.themodule, None)
+        assert inflated.themodule is os
 
     def test_thing_with_submodule(self):
         obj = Thing('with-submodule')