Skip to content

Commit dbe238e

Browse files
committed
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.
1 parent 9adef68 commit dbe238e

4 files changed

Lines changed: 36 additions & 1 deletion

File tree

Lib/pickle.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1627,7 +1627,7 @@ def _instantiate(self, klass, args):
16271627
value = klass(*args)
16281628
except TypeError as err:
16291629
raise TypeError("in constructor for %s: %s" %
1630-
(klass.__name__, str(err)), err.__traceback__)
1630+
(klass.__name__, str(err))) from err
16311631
else:
16321632
value = klass.__new__(klass)
16331633
self.append(value)

Lib/test/picklecommon.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,20 @@ class E(C):
1717
def __getinitargs__(self):
1818
return ()
1919

20+
# For test_load_bad_constructor
21+
class BadConstructor:
22+
def __init__(self, *args):
23+
raise TypeError("bad constructor")
24+
2025
import __main__
2126
__main__.C = C
2227
C.__module__ = "__main__"
2328
__main__.D = D
2429
D.__module__ = "__main__"
2530
__main__.E = E
2631
E.__module__ = "__main__"
32+
__main__.BadConstructor = BadConstructor
33+
BadConstructor.__module__ = "__main__"
2734

2835
# Simple mutable object.
2936
class Object(object):

Lib/test/pickletester.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -846,6 +846,30 @@ def test_load_classic_instance(self):
846846
b'q\x00oq\x01}q\x02b.').replace(b'X', xname)
847847
self.assert_is_copy(X(*args), self.loads(pickle2))
848848

849+
def test_load_bad_constructor(self):
850+
# gh-154002: a TypeError raised by an old-style instance constructor
851+
# during INST/OBJ unpickling must be reported with proper exception
852+
# chaining, not with the traceback object stored in the exception's
853+
# args (which used to happen in the pure-Python unpickler).
854+
# 0: ( MARK
855+
# 1: I INT 1
856+
# 4: i INST '__main__ BadConstructor' (MARK at 0)
857+
# 28: . STOP
858+
data = b'(I1\ni__main__\nBadConstructor\n.'
859+
with self.assertRaises(TypeError) as cm:
860+
self.loads(data)
861+
exc = cm.exception
862+
# A traceback object must never leak into the exception's args.
863+
self.assertNotIn(types.TracebackType, [type(a) for a in exc.args])
864+
# Only the pure-Python unpickler wraps the failure; the C one lets the
865+
# original TypeError propagate. When it wraps, it must chain the cause.
866+
if str(exc).startswith("in constructor for "):
867+
self.assertEqual(
868+
exc.args,
869+
("in constructor for BadConstructor: bad constructor",))
870+
self.assertIsInstance(exc.__cause__, TypeError)
871+
self.assertEqual(str(exc.__cause__), "bad constructor")
872+
849873
def test_maxint64(self):
850874
maxint64 = (1 << 63) - 1
851875
data = b'I' + str(maxint64).encode("ascii") + b'\n.'
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix exception handling in the pure-Python :mod:`pickle` unpickler when a
2+
class constructor raises :exc:`TypeError` while unpickling an old-style
3+
instance. The original error is now chained with ``from`` instead of having
4+
its traceback object stored in the raised exception's ``args``.

0 commit comments

Comments
 (0)