From dbe238e338084041bce774ba1d7b0a9c43e2a089 Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Sat, 18 Jul 2026 18:13:13 +0300 Subject: [PATCH 1/2] gh-154002: Fix exception chaining in pickle._Unpickler._instantiate When a class constructor raised TypeError during old-style (INST/OBJ) unpickling, the pure-Python unpickler did `raise TypeError(msg, err.__traceback__)`, which stored the traceback object in the exception's args and performed no real chaining (__cause__ stayed None). Use `raise TypeError(msg) from err` instead. The C implementation is unaffected; it lets the original error propagate. --- Lib/pickle.py | 2 +- Lib/test/picklecommon.py | 7 ++++++ Lib/test/pickletester.py | 24 +++++++++++++++++++ ...-07-18-18-12-44.gh-issue-154002.Qw9zTn.rst | 4 ++++ 4 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2026-07-18-18-12-44.gh-issue-154002.Qw9zTn.rst diff --git a/Lib/pickle.py b/Lib/pickle.py index f92b1fde768fc7d..26c31895de8cf0f 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -1627,7 +1627,7 @@ def _instantiate(self, klass, args): value = klass(*args) except TypeError as err: raise TypeError("in constructor for %s: %s" % - (klass.__name__, str(err)), err.__traceback__) + (klass.__name__, str(err))) from err else: value = klass.__new__(klass) self.append(value) diff --git a/Lib/test/picklecommon.py b/Lib/test/picklecommon.py index bb8e41b01492ead..5dd56c4fbf9ec81 100644 --- a/Lib/test/picklecommon.py +++ b/Lib/test/picklecommon.py @@ -17,6 +17,11 @@ class E(C): def __getinitargs__(self): return () +# For test_load_bad_constructor +class BadConstructor: + def __init__(self, *args): + raise TypeError("bad constructor") + import __main__ __main__.C = C C.__module__ = "__main__" @@ -24,6 +29,8 @@ def __getinitargs__(self): D.__module__ = "__main__" __main__.E = E E.__module__ = "__main__" +__main__.BadConstructor = BadConstructor +BadConstructor.__module__ = "__main__" # Simple mutable object. class Object(object): diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index 9ba498ce8f575de..13eac4f543fb2fe 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -846,6 +846,30 @@ def test_load_classic_instance(self): b'q\x00oq\x01}q\x02b.').replace(b'X', xname) self.assert_is_copy(X(*args), self.loads(pickle2)) + def test_load_bad_constructor(self): + # gh-154002: a TypeError raised by an old-style instance constructor + # during INST/OBJ unpickling must be reported with proper exception + # chaining, not with the traceback object stored in the exception's + # args (which used to happen in the pure-Python unpickler). + # 0: ( MARK + # 1: I INT 1 + # 4: i INST '__main__ BadConstructor' (MARK at 0) + # 28: . STOP + data = b'(I1\ni__main__\nBadConstructor\n.' + with self.assertRaises(TypeError) as cm: + self.loads(data) + exc = cm.exception + # A traceback object must never leak into the exception's args. + self.assertNotIn(types.TracebackType, [type(a) for a in exc.args]) + # Only the pure-Python unpickler wraps the failure; the C one lets the + # original TypeError propagate. When it wraps, it must chain the cause. + if str(exc).startswith("in constructor for "): + self.assertEqual( + exc.args, + ("in constructor for BadConstructor: bad constructor",)) + self.assertIsInstance(exc.__cause__, TypeError) + self.assertEqual(str(exc.__cause__), "bad constructor") + def test_maxint64(self): maxint64 = (1 << 63) - 1 data = b'I' + str(maxint64).encode("ascii") + b'\n.' diff --git a/Misc/NEWS.d/next/Library/2026-07-18-18-12-44.gh-issue-154002.Qw9zTn.rst b/Misc/NEWS.d/next/Library/2026-07-18-18-12-44.gh-issue-154002.Qw9zTn.rst new file mode 100644 index 000000000000000..fd4d65edf33ae4b --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-07-18-18-12-44.gh-issue-154002.Qw9zTn.rst @@ -0,0 +1,4 @@ +Fix exception handling in the pure-Python :mod:`pickle` unpickler when a +class constructor raises :exc:`TypeError` while unpickling an old-style +instance. The original error is now chained with ``from`` instead of having +its traceback object stored in the raised exception's ``args``. From c90a27b3ac512897fadc1ae316cfdb2edcdff136 Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Thu, 6 Aug 2026 00:55:07 +0300 Subject: [PATCH 2/2] Drop the wrapper instead of chaining it The prefix repeats the class name that the constructor TypeError already carries, and the C unpickler never wrapped the error, so letting the original propagate removes the traceback-in-args bug and the divergence at once. --- Lib/pickle.py | 6 +----- Lib/test/pickletester.py | 18 ++++-------------- ...6-07-18-18-12-44.gh-issue-154002.Qw9zTn.rst | 8 ++++---- 3 files changed, 9 insertions(+), 23 deletions(-) diff --git a/Lib/pickle.py b/Lib/pickle.py index 26c31895de8cf0f..b475d96dd6576d9 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -1623,11 +1623,7 @@ def load_dict(self): def _instantiate(self, klass, args): if (args or not isinstance(klass, type) or hasattr(klass, "__getinitargs__")): - try: - value = klass(*args) - except TypeError as err: - raise TypeError("in constructor for %s: %s" % - (klass.__name__, str(err))) from err + value = klass(*args) else: value = klass.__new__(klass) self.append(value) diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index 13eac4f543fb2fe..c9b72cf5a1ca837 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -848,9 +848,9 @@ def test_load_classic_instance(self): def test_load_bad_constructor(self): # gh-154002: a TypeError raised by an old-style instance constructor - # during INST/OBJ unpickling must be reported with proper exception - # chaining, not with the traceback object stored in the exception's - # args (which used to happen in the pure-Python unpickler). + # during INST/OBJ unpickling propagates unchanged. The pure-Python + # unpickler used to replace it with one that carried the traceback + # object in its args. # 0: ( MARK # 1: I INT 1 # 4: i INST '__main__ BadConstructor' (MARK at 0) @@ -858,17 +858,7 @@ def test_load_bad_constructor(self): data = b'(I1\ni__main__\nBadConstructor\n.' with self.assertRaises(TypeError) as cm: self.loads(data) - exc = cm.exception - # A traceback object must never leak into the exception's args. - self.assertNotIn(types.TracebackType, [type(a) for a in exc.args]) - # Only the pure-Python unpickler wraps the failure; the C one lets the - # original TypeError propagate. When it wraps, it must chain the cause. - if str(exc).startswith("in constructor for "): - self.assertEqual( - exc.args, - ("in constructor for BadConstructor: bad constructor",)) - self.assertIsInstance(exc.__cause__, TypeError) - self.assertEqual(str(exc.__cause__), "bad constructor") + self.assertEqual(cm.exception.args, ("bad constructor",)) def test_maxint64(self): maxint64 = (1 << 63) - 1 diff --git a/Misc/NEWS.d/next/Library/2026-07-18-18-12-44.gh-issue-154002.Qw9zTn.rst b/Misc/NEWS.d/next/Library/2026-07-18-18-12-44.gh-issue-154002.Qw9zTn.rst index fd4d65edf33ae4b..0475ce7330b6fa8 100644 --- a/Misc/NEWS.d/next/Library/2026-07-18-18-12-44.gh-issue-154002.Qw9zTn.rst +++ b/Misc/NEWS.d/next/Library/2026-07-18-18-12-44.gh-issue-154002.Qw9zTn.rst @@ -1,4 +1,4 @@ -Fix exception handling in the pure-Python :mod:`pickle` unpickler when a -class constructor raises :exc:`TypeError` while unpickling an old-style -instance. The original error is now chained with ``from`` instead of having -its traceback object stored in the raised exception's ``args``. +The pure-Python :mod:`pickle` unpickler no longer replaces a :exc:`TypeError` +raised by an old-style instance constructor with a new one carrying the +traceback object in its ``args``. The original error now propagates, as it +already did in the C implementation.