From 9939a1c0c57b94b687dd5def5adfb5459a4eb8f3 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sat, 25 Mar 2023 21:29:57 +0100 Subject: [PATCH 01/24] align python implementation of pickle, c implementation of pickle and copy implementation --- Lib/copy.py | 33 +++++++++++++++++++-------------- Lib/pickle.py | 20 ++++++++++++-------- Lib/test/test_copy.py | 14 ++++++++++++++ 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/Lib/copy.py b/Lib/copy.py index da2908ef623d8c..e6f3decadfcf6d 100644 --- a/Lib/copy.py +++ b/Lib/copy.py @@ -56,6 +56,11 @@ class Error(Exception): pass error = Error # backward compatibility +class _NoValue: + pass + +NoValue = _NoValue() + __all__ = ["Error", "copy", "deepcopy"] def copy(x): @@ -74,20 +79,20 @@ def copy(x): # treat it as a regular class: return _copy_immutable(x) - copier = getattr(cls, "__copy__", None) - if copier is not None: + copier = getattr(cls, "__copy__", NoValue) + if copier is not NoValue: return copier(x) reductor = dispatch_table.get(cls) if reductor is not None: rv = reductor(x) else: - reductor = getattr(x, "__reduce_ex__", None) - if reductor is not None: + reductor = getattr(x, "__reduce_ex__", NoValue) + if reductor is not NoValue: rv = reductor(4) else: - reductor = getattr(x, "__reduce__", None) - if reductor: + reductor = getattr(x, "__reduce__", NoValue) + if reductor is not NoValue: rv = reductor() else: raise Error("un(shallow)copyable object of type %s" % cls) @@ -131,27 +136,27 @@ def deepcopy(x, memo=None, _nil=[]): cls = type(x) - copier = _deepcopy_dispatch.get(cls) - if copier is not None: + copier = _deepcopy_dispatch.get(cls, NoValue) + if copier is not NoValue: y = copier(x, memo) else: if issubclass(cls, type): y = _deepcopy_atomic(x, memo) else: - copier = getattr(x, "__deepcopy__", None) - if copier is not None: + copier = getattr(x, "__deepcopy__", NoValue) + if copier is not NoValue: y = copier(memo) else: reductor = dispatch_table.get(cls) if reductor: rv = reductor(x) else: - reductor = getattr(x, "__reduce_ex__", None) - if reductor is not None: + reductor = getattr(x, "__reduce_ex__", NoValue) + if reductor is not NoValue: rv = reductor(4) else: - reductor = getattr(x, "__reduce__", None) - if reductor: + reductor = getattr(x, "__reduce__", NoValue) + if reductor is not NoValue: rv = reductor() else: raise Error( diff --git a/Lib/pickle.py b/Lib/pickle.py index 15fa5f6e579932..8da71eced3def5 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -395,6 +395,10 @@ def decode_long(data): """ return int.from_bytes(data, byteorder='little', signed=True) +class _NoValue: + pass + +NoValue = _NoValue() # Pickling machinery @@ -542,8 +546,8 @@ def save(self, obj, save_persistent_id=True): return rv = NotImplemented - reduce = getattr(self, "reducer_override", None) - if reduce is not None: + reduce = getattr(self, "reducer_override", NoValue) + if reduce is not NoValue: rv = reduce(obj) if rv is NotImplemented: @@ -567,12 +571,12 @@ def save(self, obj, save_persistent_id=True): return # Check for a __reduce_ex__ method, fall back to __reduce__ - reduce = getattr(obj, "__reduce_ex__", None) - if reduce is not None: + reduce = getattr(obj, "__reduce_ex__", NoValue) + if reduce is not NoValue: rv = reduce(self.proto) else: - reduce = getattr(obj, "__reduce__", None) - if reduce is not None: + reduce = getattr(obj, "__reduce__", NoValue) + if reduce is not NoValue: rv = reduce() else: raise PicklingError("Can't pickle %r object: %r" % @@ -1705,8 +1709,8 @@ def load_build(self): stack = self.stack state = stack.pop() inst = stack[-1] - setstate = getattr(inst, "__setstate__", None) - if setstate is not None: + setstate = getattr(inst, "__setstate__", NoValue) + if setstate is not NoValue: setstate(state) return slotstate = None diff --git a/Lib/test/test_copy.py b/Lib/test/test_copy.py index 826e46824e004c..834570df1975a9 100644 --- a/Lib/test/test_copy.py +++ b/Lib/test/test_copy.py @@ -262,6 +262,20 @@ def __eq__(self, other): x = C(0.0) self.assertEqual(copy.copy(x), x) + def test_copy_invalid_copy_method(self): + class C: + __copy__ = None + c = C() + with self.assertRaises(TypeError): + copy.copy(c) + + class C: + __reduce_ex__ = None + c = C() + with self.assertRaises(TypeError): + copy.copy(c) + + # The deepcopy() method def test_deepcopy_basic(self): From 29445b911ad159da2b31958499df212d6c180975 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sun, 26 Mar 2023 21:03:19 +0200 Subject: [PATCH 02/24] review comments --- Lib/copy.py | 5 +---- Lib/pickle.py | 4 +--- Lib/test/test_pickle.py | 16 ++++++++++++++++ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/Lib/copy.py b/Lib/copy.py index e6f3decadfcf6d..68b676c2b18e32 100644 --- a/Lib/copy.py +++ b/Lib/copy.py @@ -56,10 +56,7 @@ class Error(Exception): pass error = Error # backward compatibility -class _NoValue: - pass - -NoValue = _NoValue() +NoValue = object() __all__ = ["Error", "copy", "deepcopy"] diff --git a/Lib/pickle.py b/Lib/pickle.py index 8da71eced3def5..e43ef402aae8e5 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -395,10 +395,8 @@ def decode_long(data): """ return int.from_bytes(data, byteorder='little', signed=True) -class _NoValue: - pass -NoValue = _NoValue() +NoValue = object() # Pickling machinery diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 80e7a4d23a4ba8..02c005419d860c 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -542,6 +542,22 @@ def test_multiprocessing_exceptions(self): ('multiprocessing.context', name)) +class PickleReductionMethodsTests(unittest.TestCase): + + def test_pickle_invalid_reduction_method(self): + class C: + __reduce_ex__ = None + c = C() + with self.assertRaises(TypeError): + pickle.dumps(c) + + class C: + __reduce__ = None + c = C() + with self.assertRaises(TypeError): + pickle.dumps(c) + + def load_tests(loader, tests, pattern): tests.addTest(doctest.DocTestSuite()) return tests From 95cbf7788ce7900ec5dd49ea9bda3137716eed31 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sun, 26 Mar 2023 21:06:44 +0200 Subject: [PATCH 03/24] review comments --- Lib/pickle.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/pickle.py b/Lib/pickle.py index e43ef402aae8e5..4102b50a781487 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -558,8 +558,8 @@ def save(self, obj, save_persistent_id=True): # Check private dispatch table if any, or else # copyreg.dispatch_table - reduce = getattr(self, 'dispatch_table', dispatch_table).get(t) - if reduce is not None: + reduce = getattr(self, 'dispatch_table', dispatch_table).get(t, NoValue) + if reduce is not NoValue: rv = reduce(obj) else: # Check for a class with a custom metaclass; treat as regular From fa2a7414269cd8128fc65676c9fd8859ea4d8368 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sun, 26 Mar 2023 19:11:13 +0000 Subject: [PATCH 04/24] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst b/Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst new file mode 100644 index 00000000000000..639b69c6d00ed9 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst @@ -0,0 +1 @@ +Update the Python pickle module implementation to match the C implementation of the pickle module and update the implemetation of the copy module to match the pickle module conventions. For all modules reduction settings methods like `__reduce_ex__` or `__reduce__` to `None` will result in a `TypeError` with pickling or copying an object. From 3e6e4ebb36fe656b94bc61325941b627424c9ad3 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sun, 26 Mar 2023 21:23:25 +0200 Subject: [PATCH 05/24] format news entry --- .../2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst b/Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst index 639b69c6d00ed9..826b51c73398e0 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst @@ -1 +1 @@ -Update the Python pickle module implementation to match the C implementation of the pickle module and update the implemetation of the copy module to match the pickle module conventions. For all modules reduction settings methods like `__reduce_ex__` or `__reduce__` to `None` will result in a `TypeError` with pickling or copying an object. +Update the Python pickle module implementation to match the C implementation of the pickle module and update the implementation of the copy module to match the pickle module conventions. For all modules setting reduction methods like ``__reduce_ex__`` or ``__reduce__`` to ``None`` will result in a ``TypeError`` with pickling or copying an object. From 92362c126eedd41e83fac0cc24f0d6d3677702af Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sun, 26 Mar 2023 21:34:22 +0200 Subject: [PATCH 06/24] remove unused exception --- Lib/pickle.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/pickle.py b/Lib/pickle.py index 4102b50a781487..6bfdc24141312f 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -1612,7 +1612,7 @@ def load_binget(self): i = self.read(1)[0] try: self.append(self.memo[i]) - except KeyError as exc: + except KeyError: msg = f'Memo value not found at index {i}' raise UnpicklingError(msg) from None dispatch[BINGET[0]] = load_binget @@ -1621,7 +1621,7 @@ def load_long_binget(self): i, = unpack(' Date: Sun, 2 Apr 2023 21:32:34 +0200 Subject: [PATCH 07/24] align C and python pickle implementations --- Modules/_pickle.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Modules/_pickle.c b/Modules/_pickle.c index a26732af8ba2a1..26f926b1791e35 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3953,7 +3953,7 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) size = PyTuple_Size(args); if (size < 2 || size > 6) { PyErr_SetString(st->PicklingError, "tuple returned by " - "__reduce__ must contain 2 through 6 elements"); + "__reduce__ must contain 2 to 6 elements"); return -1; } @@ -4383,6 +4383,7 @@ save(PicklerObject *self, PyObject *obj, int pers_save) Py_INCREF(reduce_func); } } else { + // check private dispatch table reduce_func = PyObject_GetItem(self->dispatch_table, (PyObject *)type); if (reduce_func == NULL) { @@ -4413,8 +4414,7 @@ save(PicklerObject *self, PyObject *obj, int pers_save) goto error; } if (reduce_func != NULL) { - PyObject *proto; - proto = PyLong_FromLong(self->proto); + PyObject *proto = PyLong_FromLong(self->proto); if (proto != NULL) { reduce_value = _Pickle_FastCall(reduce_func, proto); } From 1900578c5fd7d0f95b5d8ffbd9aa87ce1ad30f8b Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Thu, 25 May 2023 10:49:57 +0200 Subject: [PATCH 08/24] rename NoValue to private _NoValue --- Lib/pickle.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Lib/pickle.py b/Lib/pickle.py index 14e49e8e0d925e..8e9f68860a05b2 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -396,7 +396,7 @@ def decode_long(data): return int.from_bytes(data, byteorder='little', signed=True) -NoValue = object() +_NoValue = object() # Pickling machinery @@ -544,8 +544,8 @@ def save(self, obj, save_persistent_id=True): return rv = NotImplemented - reduce = getattr(self, "reducer_override", NoValue) - if reduce is not NoValue: + reduce = getattr(self, "reducer_override", _NoValue) + if reduce is not _NoValue: rv = reduce(obj) if rv is NotImplemented: @@ -558,8 +558,8 @@ def save(self, obj, save_persistent_id=True): # Check private dispatch table if any, or else # copyreg.dispatch_table - reduce = getattr(self, 'dispatch_table', dispatch_table).get(t, NoValue) - if reduce is not NoValue: + reduce = getattr(self, 'dispatch_table', dispatch_table).get(t, _NoValue) + if reduce is not _NoValue: rv = reduce(obj) else: # Check for a class with a custom metaclass; treat as regular @@ -569,12 +569,12 @@ def save(self, obj, save_persistent_id=True): return # Check for a __reduce_ex__ method, fall back to __reduce__ - reduce = getattr(obj, "__reduce_ex__", NoValue) - if reduce is not NoValue: + reduce = getattr(obj, "__reduce_ex__", _NoValue) + if reduce is not _NoValue: rv = reduce(self.proto) else: - reduce = getattr(obj, "__reduce__", NoValue) - if reduce is not NoValue: + reduce = getattr(obj, "__reduce__", _NoValue) + if reduce is not _NoValue: rv = reduce() else: raise PicklingError("Can't pickle %r object: %r" % @@ -1707,8 +1707,8 @@ def load_build(self): stack = self.stack state = stack.pop() inst = stack[-1] - setstate = getattr(inst, "__setstate__", NoValue) - if setstate is not NoValue: + setstate = getattr(inst, "__setstate__", _NoValue) + if setstate is not _NoValue: setstate(state) return slotstate = None From d822705fa0194d8de287e5bbaa92000855bbc9ca Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Thu, 25 May 2023 15:20:49 +0200 Subject: [PATCH 09/24] reverse changes to copy.py --- Lib/copy.py | 30 ++++++++++++++---------------- Lib/test/test_copy.py | 14 -------------- 2 files changed, 14 insertions(+), 30 deletions(-) diff --git a/Lib/copy.py b/Lib/copy.py index 68b676c2b18e32..da2908ef623d8c 100644 --- a/Lib/copy.py +++ b/Lib/copy.py @@ -56,8 +56,6 @@ class Error(Exception): pass error = Error # backward compatibility -NoValue = object() - __all__ = ["Error", "copy", "deepcopy"] def copy(x): @@ -76,20 +74,20 @@ def copy(x): # treat it as a regular class: return _copy_immutable(x) - copier = getattr(cls, "__copy__", NoValue) - if copier is not NoValue: + copier = getattr(cls, "__copy__", None) + if copier is not None: return copier(x) reductor = dispatch_table.get(cls) if reductor is not None: rv = reductor(x) else: - reductor = getattr(x, "__reduce_ex__", NoValue) - if reductor is not NoValue: + reductor = getattr(x, "__reduce_ex__", None) + if reductor is not None: rv = reductor(4) else: - reductor = getattr(x, "__reduce__", NoValue) - if reductor is not NoValue: + reductor = getattr(x, "__reduce__", None) + if reductor: rv = reductor() else: raise Error("un(shallow)copyable object of type %s" % cls) @@ -133,27 +131,27 @@ def deepcopy(x, memo=None, _nil=[]): cls = type(x) - copier = _deepcopy_dispatch.get(cls, NoValue) - if copier is not NoValue: + copier = _deepcopy_dispatch.get(cls) + if copier is not None: y = copier(x, memo) else: if issubclass(cls, type): y = _deepcopy_atomic(x, memo) else: - copier = getattr(x, "__deepcopy__", NoValue) - if copier is not NoValue: + copier = getattr(x, "__deepcopy__", None) + if copier is not None: y = copier(memo) else: reductor = dispatch_table.get(cls) if reductor: rv = reductor(x) else: - reductor = getattr(x, "__reduce_ex__", NoValue) - if reductor is not NoValue: + reductor = getattr(x, "__reduce_ex__", None) + if reductor is not None: rv = reductor(4) else: - reductor = getattr(x, "__reduce__", NoValue) - if reductor is not NoValue: + reductor = getattr(x, "__reduce__", None) + if reductor: rv = reductor() else: raise Error( diff --git a/Lib/test/test_copy.py b/Lib/test/test_copy.py index 834570df1975a9..826e46824e004c 100644 --- a/Lib/test/test_copy.py +++ b/Lib/test/test_copy.py @@ -262,20 +262,6 @@ def __eq__(self, other): x = C(0.0) self.assertEqual(copy.copy(x), x) - def test_copy_invalid_copy_method(self): - class C: - __copy__ = None - c = C() - with self.assertRaises(TypeError): - copy.copy(c) - - class C: - __reduce_ex__ = None - c = C() - with self.assertRaises(TypeError): - copy.copy(c) - - # The deepcopy() method def test_deepcopy_basic(self): From a366dc50a5a661d955b415a5c0e24381978fdb4e Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Fri, 9 Jun 2023 21:13:34 +0200 Subject: [PATCH 10/24] revert style changes --- Lib/pickle.py | 4 ++-- Modules/_pickle.c | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Lib/pickle.py b/Lib/pickle.py index 14e49e8e0d925e..3c240e767cc8ce 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -1612,7 +1612,7 @@ def load_binget(self): i = self.read(1)[0] try: self.append(self.memo[i]) - except KeyError: + except KeyError as exc: msg = f'Memo value not found at index {i}' raise UnpicklingError(msg) from None dispatch[BINGET[0]] = load_binget @@ -1621,7 +1621,7 @@ def load_long_binget(self): i, = unpack('proto); + PyObject *proto; + proto = PyLong_FromLong(self->proto); if (proto != NULL) { reduce_value = _Pickle_FastCall(reduce_func, proto); } From 1e79c83a4cd340a598a2334b85a70a0675dc743a Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Fri, 9 Jun 2023 21:17:41 +0200 Subject: [PATCH 11/24] update news entry --- .../2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst b/Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst index 826b51c73398e0..3788fb8395816b 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst @@ -1 +1 @@ -Update the Python pickle module implementation to match the C implementation of the pickle module and update the implementation of the copy module to match the pickle module conventions. For all modules setting reduction methods like ``__reduce_ex__`` or ``__reduce__`` to ``None`` will result in a ``TypeError`` with pickling or copying an object. +Update the Python pickle module implementation to match the C implementation of the pickle module. For objects setting reduction methods like ``__reduce_ex__`` or ``__reduce__`` to ``None``, pickling will result in a ``TypeError``. From d365a4415b68b2885352d14fea7fdae6cf857d6f Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 21 Aug 2023 13:37:32 +0200 Subject: [PATCH 12/24] revert spelling mistake in exception message as it is part of the public api --- Modules/_pickle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 5e56f87066f110..d42351e2a8c7b7 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -4010,7 +4010,7 @@ save_reduce(PickleState *st, PicklerObject *self, PyObject *args, size = PyTuple_Size(args); if (size < 2 || size > 6) { PyErr_SetString(st->PicklingError, "tuple returned by " - "__reduce__ must contain 2 to 6 elements"); + "__reduce__ must contain 2 through 6 elements"); return -1; } From 1bf64c0cda8cdefa25ffc9180c60e9f5b1943c8d Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 21 Aug 2023 13:38:37 +0200 Subject: [PATCH 13/24] revert adding comment to pickle c implementation --- Modules/_pickle.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Modules/_pickle.c b/Modules/_pickle.c index d42351e2a8c7b7..c2b04cc513a664 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -4440,7 +4440,6 @@ save(PickleState *st, PicklerObject *self, PyObject *obj, int pers_save) Py_INCREF(reduce_func); } } - // check private dispatch table else if (PyMapping_GetOptionalItem(self->dispatch_table, (PyObject *)type, &reduce_func) < 0) { From 3548dc3169a726054594587c60c3e29ee0ae639b Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 22 Aug 2023 08:59:04 +0200 Subject: [PATCH 14/24] Update Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst Co-authored-by: Erlend E. Aasland --- .../2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst b/Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst index 3788fb8395816b..854da44b560b21 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-03-26-19-11-10.gh-issue-93627.0UgwBL.rst @@ -1 +1 @@ -Update the Python pickle module implementation to match the C implementation of the pickle module. For objects setting reduction methods like ``__reduce_ex__`` or ``__reduce__`` to ``None``, pickling will result in a ``TypeError``. +Update the Python pickle module implementation to match the C implementation of the pickle module. For objects setting reduction methods like :meth:`~object.__reduce_ex__` or :meth:`~object.__reduce__` to ``None``, pickling will result in a :exc:`TypeError`. From 250acccaef78aa96e83e8d35688b10841bd5a088 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 22 Aug 2023 10:35:29 +0200 Subject: [PATCH 15/24] add unit test for None value of __setstate__ --- Lib/test/test_pickle.py | 42 +++++++++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 4ea75d729a4798..0f5cac2193063e 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -570,21 +570,51 @@ def test_multiprocessing_exceptions(self): ('multiprocessing.context', name)) +class C_None_reduce_ex: + __reduce_ex__ = None + +class C_None_reduce: + __reduce_ex__ = None + +class C_None_setstate: + __setstate__ = None + +class C_setstate: + def __init__(self): + self.value = 0 + + def __getstate__(self): + return self.value + + def __setstate__(self, state): + print(state) + self.value = state + 1 + class PickleReductionMethodsTests(unittest.TestCase): def test_pickle_invalid_reduction_method(self): - class C: - __reduce_ex__ = None - c = C() + c = C_None_reduce_ex() with self.assertRaises(TypeError): pickle.dumps(c) - class C: - __reduce__ = None - c = C() + c = C_None_reduce() with self.assertRaises(TypeError): pickle.dumps(c) +class PickleSetstateTests(unittest.TestCase): + + def test_pickle_setstate(self): + c = C_setstate() + s = pickle.dumps(c) + c2 = pickle.loads(s) + self.assertEqual(c2.value, c.value + 1) + + c = C_None_setstate() + # See gh-103035 + p = pickle.dumps(c) + with self.assertRaises(TypeError): + pickle.loads(p) + def load_tests(loader, tests, pattern): tests.addTest(doctest.DocTestSuite()) From e58f4a1ca08b2908e580b33d07946c3053c77fca Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 22 Aug 2023 20:54:52 +0200 Subject: [PATCH 16/24] fix tests --- Lib/test/test_pickle.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 0f5cac2193063e..3fbc2b9a774cb4 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -574,9 +574,12 @@ class C_None_reduce_ex: __reduce_ex__ = None class C_None_reduce: - __reduce_ex__ = None + __reduce__ = None class C_None_setstate: + def __getstate__(self): + return 1 + __setstate__ = None class C_setstate: From b1bd83c3fe1d9badfdbbf9e37f2bd0513a1b4b15 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 23 Aug 2023 22:20:25 +0200 Subject: [PATCH 17/24] add test for reduced_override --- Lib/test/test_pickle.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 3fbc2b9a774cb4..6e21fff2a0cf55 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -570,6 +570,10 @@ def test_multiprocessing_exceptions(self): ('multiprocessing.context', name)) +class C_with_reduce_ex: + def __reduce_ex__(self): + return 10 + class C_None_reduce_ex: __reduce_ex__ = None @@ -604,6 +608,20 @@ def test_pickle_invalid_reduction_method(self): with self.assertRaises(TypeError): pickle.dumps(c) + def test_pickle_invalid_reducer_override(self): + obj=C_with_reduce_ex() + + bio = io.BytesIO() + pickler = pickle._Pickler(bio) + pickler.reducer_override = None + with self.assertRaises(TypeError): + pickler.dump(obj) + + pickler.reducer_override = 10 + with self.assertRaises(TypeError): + pickler.dump(obj) + + class PickleSetstateTests(unittest.TestCase): def test_pickle_setstate(self): From f56551334267a15dc5819055d1286e8fa8dd82ff Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 23 Aug 2023 22:36:39 +0200 Subject: [PATCH 18/24] add test for dispatch table --- Lib/test/test_pickle.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 6e21fff2a0cf55..5127d22afaaede 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -571,8 +571,9 @@ def test_multiprocessing_exceptions(self): class C_with_reduce_ex: - def __reduce_ex__(self): - return 10 + + def __reduce_ex__(self, proto): + return object.__reduce_ex__(self, proto) class C_None_reduce_ex: __reduce_ex__ = None @@ -613,6 +614,8 @@ def test_pickle_invalid_reducer_override(self): bio = io.BytesIO() pickler = pickle._Pickler(bio) + pickler.dump(obj) + pickler.reducer_override = None with self.assertRaises(TypeError): pickler.dump(obj) @@ -621,6 +624,16 @@ def test_pickle_invalid_reducer_override(self): with self.assertRaises(TypeError): pickler.dump(obj) + def test_pickle_invalid_dispatch_table(self): + # gh-93627 + obj=C_with_reduce_ex() + + bio = io.BytesIO() + pickler = pickle._Pickler(bio) + pickler.dispatch_table = {type(obj): None} + with self.assertRaises(TypeError): + pickler.dump(obj) + class PickleSetstateTests(unittest.TestCase): From 4a928aa7f441a9ec2299eb07ec97c1465669ae10 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 23 Aug 2023 22:59:20 +0200 Subject: [PATCH 19/24] whitespace --- Lib/test/test_pickle.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 5127d22afaaede..ea325df07ae55a 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -571,7 +571,6 @@ def test_multiprocessing_exceptions(self): class C_with_reduce_ex: - def __reduce_ex__(self, proto): return object.__reduce_ex__(self, proto) From 39ab1ed1c566d208e6c75f51334e89f210822754 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Thu, 24 Aug 2023 11:34:21 +0200 Subject: [PATCH 20/24] fix test --- Lib/test/test_pickle.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index ea325df07ae55a..d3289ce068f057 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -615,10 +615,12 @@ def test_pickle_invalid_reducer_override(self): pickler = pickle._Pickler(bio) pickler.dump(obj) + pickler.clear_memo() pickler.reducer_override = None with self.assertRaises(TypeError): pickler.dump(obj) + pickler.clear_memo() pickler.reducer_override = 10 with self.assertRaises(TypeError): pickler.dump(obj) From f694005181c10bc49f0e3c40c2245f3a57130d82 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Thu, 24 Aug 2023 14:07:43 +0200 Subject: [PATCH 21/24] move docstrings --- Lib/test/test_pickle.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index d3289ce068f057..07cd850ad10850 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -598,6 +598,7 @@ def __setstate__(self, state): self.value = state + 1 class PickleReductionMethodsTests(unittest.TestCase): + # see gh-93627 def test_pickle_invalid_reduction_method(self): c = C_None_reduce_ex() @@ -626,7 +627,6 @@ def test_pickle_invalid_reducer_override(self): pickler.dump(obj) def test_pickle_invalid_dispatch_table(self): - # gh-93627 obj=C_with_reduce_ex() bio = io.BytesIO() @@ -637,6 +637,7 @@ def test_pickle_invalid_dispatch_table(self): class PickleSetstateTests(unittest.TestCase): + # See gh-103035 def test_pickle_setstate(self): c = C_setstate() @@ -645,7 +646,6 @@ def test_pickle_setstate(self): self.assertEqual(c2.value, c.value + 1) c = C_None_setstate() - # See gh-103035 p = pickle.dumps(c) with self.assertRaises(TypeError): pickle.loads(p) From c803480bfb60f8b5e91220982af63a3154deae80 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 28 Aug 2023 13:31:20 +0200 Subject: [PATCH 22/24] refactor tests --- Lib/test/pickletester.py | 58 ++++++++++++++++++++++++++++ Lib/test/test_pickle.py | 81 ---------------------------------------- 2 files changed, 58 insertions(+), 81 deletions(-) diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index 6e87370c2065ba..316da00bab0283 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -2408,6 +2408,22 @@ def test_reduce_calls_base(self): y = self.loads(s) self.assertEqual(y._reduce_called, 1) + def test_reduce_ex_None(self): + c = REX_None() + with self.assertRaises(TypeError): + self.dumps(c) + + def test_reduce_None(self): + c = R_None() + with self.assertRaises(TypeError): + pickle.dumps(c) + + def test_pickle_setstate_None(self): + c = C_None_setstate() + p = self.dumps(c) + with self.assertRaises(TypeError): + self.loads(p) + @no_tracing def test_bad_getattr(self): # Issue #3514: crash when there is an infinite loop in __getattr__ @@ -3348,6 +3364,20 @@ def __setstate__(self, state): def __reduce__(self): return type(self), (), self.state +class REX_None: + """ Setting __reduce_ex__ to None should fail """ + __reduce_ex__ = None + +class R_None: + """ Setting __reduce__ to None should fail """ + __reduce__ = None + +class C_None_setstate: + def __getstate__(self): + return 1 + + __setstate__ = None + # Test classes for newobj @@ -3751,6 +3781,25 @@ def test_unpickling_buffering_readline(self): unpickler = self.unpickler_class(f) self.assertEqual(unpickler.load(), data) + def test_pickle_invalid_reducer_override(self): + # gh-103035 + obj=object() + + f = io.BytesIO() + class MyPickler(self.pickler_class): + pass + pickler = MyPickler(f) + pickler.dump(obj) + + pickler.clear_memo() + pickler.reducer_override = None + with self.assertRaises(TypeError): + pickler.dump(obj) + + pickler.clear_memo() + pickler.reducer_override = 10 + with self.assertRaises(TypeError): + pickler.dump(obj) # Tests for dispatch_table attribute @@ -3913,6 +3962,15 @@ def dumps(obj, protocol=None): self._test_dispatch_table(dumps, dt) + def test_dispatch_table_None_item(self): + # gh-93627 + obj=object() + f = io.BytesIO() + pickler = self.pickler_class(f) + pickler.dispatch_table = {type(obj): None} + with self.assertRaises(TypeError): + pickler.dump(obj) + def _test_dispatch_table(self, dumps, dispatch_table): def custom_load_dump(obj): return pickle.loads(dumps(obj, 0)) diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 07cd850ad10850..1a55da39bdc58d 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -570,87 +570,6 @@ def test_multiprocessing_exceptions(self): ('multiprocessing.context', name)) -class C_with_reduce_ex: - def __reduce_ex__(self, proto): - return object.__reduce_ex__(self, proto) - -class C_None_reduce_ex: - __reduce_ex__ = None - -class C_None_reduce: - __reduce__ = None - -class C_None_setstate: - def __getstate__(self): - return 1 - - __setstate__ = None - -class C_setstate: - def __init__(self): - self.value = 0 - - def __getstate__(self): - return self.value - - def __setstate__(self, state): - print(state) - self.value = state + 1 - -class PickleReductionMethodsTests(unittest.TestCase): - # see gh-93627 - - def test_pickle_invalid_reduction_method(self): - c = C_None_reduce_ex() - with self.assertRaises(TypeError): - pickle.dumps(c) - - c = C_None_reduce() - with self.assertRaises(TypeError): - pickle.dumps(c) - - def test_pickle_invalid_reducer_override(self): - obj=C_with_reduce_ex() - - bio = io.BytesIO() - pickler = pickle._Pickler(bio) - pickler.dump(obj) - - pickler.clear_memo() - pickler.reducer_override = None - with self.assertRaises(TypeError): - pickler.dump(obj) - - pickler.clear_memo() - pickler.reducer_override = 10 - with self.assertRaises(TypeError): - pickler.dump(obj) - - def test_pickle_invalid_dispatch_table(self): - obj=C_with_reduce_ex() - - bio = io.BytesIO() - pickler = pickle._Pickler(bio) - pickler.dispatch_table = {type(obj): None} - with self.assertRaises(TypeError): - pickler.dump(obj) - - -class PickleSetstateTests(unittest.TestCase): - # See gh-103035 - - def test_pickle_setstate(self): - c = C_setstate() - s = pickle.dumps(c) - c2 = pickle.loads(s) - self.assertEqual(c2.value, c.value + 1) - - c = C_None_setstate() - p = pickle.dumps(c) - with self.assertRaises(TypeError): - pickle.loads(p) - - def load_tests(loader, tests, pattern): tests.addTest(doctest.DocTestSuite()) return tests From 7f3ee96201773bd383aa1dc47539c1a8646eb1ff Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 28 Aug 2023 13:33:55 +0200 Subject: [PATCH 23/24] fix typo --- Lib/test/pickletester.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index 316da00bab0283..db5e173e4ec8e3 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -2416,7 +2416,7 @@ def test_reduce_ex_None(self): def test_reduce_None(self): c = R_None() with self.assertRaises(TypeError): - pickle.dumps(c) + self.dumps(c) def test_pickle_setstate_None(self): c = C_None_setstate() @@ -3373,6 +3373,7 @@ class R_None: __reduce__ = None class C_None_setstate: + """ Setting __setstate__ to None should fail """ def __getstate__(self): return 1 From 1f02ab2aa44d7250dcd443128a8b753bfb69357c Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 28 Aug 2023 13:35:22 +0200 Subject: [PATCH 24/24] whitespace --- Lib/test/pickletester.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index db5e173e4ec8e3..5fbd1468eff631 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -3784,7 +3784,7 @@ def test_unpickling_buffering_readline(self): def test_pickle_invalid_reducer_override(self): # gh-103035 - obj=object() + obj = object() f = io.BytesIO() class MyPickler(self.pickler_class): @@ -3965,7 +3965,7 @@ def dumps(obj, protocol=None): def test_dispatch_table_None_item(self): # gh-93627 - obj=object() + obj = object() f = io.BytesIO() pickler = self.pickler_class(f) pickler.dispatch_table = {type(obj): None}