From ce4588a438553aacdeaae6eb750905e105caa5be Mon Sep 17 00:00:00 2001 From: Dmitry Voropaev Date: Thu, 24 Sep 2026 19:03:01 +0300 Subject: [PATCH] gh-141749: Align pure Python unpickler exceptions with the C one The pure Python ``_Unpickler`` raised ``KeyError`` for an unknown opcode and ``IndexError`` whenever an opcode reached past the end of the stack, while ``_pickle`` raises ``UnpicklingError`` with a descriptive message in both cases. Report the same errors, with the same messages, from the pure Python implementation, and narrow ``bad_stack_errors`` accordingly. --- Lib/pickle.py | 165 ++++++++++++++---- Lib/test/pickletester.py | 38 ++++ Lib/test/test_pickle.py | 4 +- ...-09-24-18-57-00.gh-issue-141749.b1O2AX.rst | 4 + 4 files changed, 177 insertions(+), 34 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-09-24-18-57-00.gh-issue-141749.b1O2AX.rst diff --git a/Lib/pickle.py b/Lib/pickle.py index b475d96dd6576d9..cd042acfcd100f4 100644 --- a/Lib/pickle.py +++ b/Lib/pickle.py @@ -1343,12 +1343,31 @@ def load(self): if not key: raise EOFError assert isinstance(key, bytes_types) - dispatch[key[0]](self) + try: + handler = dispatch[key[0]] + except KeyError: + self._invalid_opcode(key[0]) + handler(self) except _Stop as stopinst: return stopinst.value + def _invalid_opcode(self, code): + if 0x20 <= code <= 0x7e and code not in b"'\\": + char = "'%c'" % code + else: + char = "'\\x%02x'" % code + raise UnpicklingError("invalid load key, %s." % char) from None + + def _underflow(self): + # A pending MARK hides the rest of the stack, so an opcode reaching + # past it is reported differently, as in the C implementation. + raise UnpicklingError("unexpected MARK found" if self.metastack + else "unpickling stack underflow") from None + # Return a list of items pushed in the stack after last MARK instruction. def pop_mark(self): + if not self.metastack: + raise UnpicklingError("could not find MARK") items = self.stack self.stack = self.metastack.pop() self.append = self.stack.append @@ -1383,7 +1402,10 @@ def load_persid(self): dispatch[PERSID[0]] = load_persid def load_binpersid(self): - pid = self.stack.pop() + try: + pid = self.stack.pop() + except IndexError: + self._underflow() self.append(self.persistent_load(pid)) dispatch[BINPERSID[0]] = load_binpersid @@ -1543,7 +1565,10 @@ def load_next_buffer(self): dispatch[NEXT_BUFFER[0]] = load_next_buffer def load_readonly_buffer(self): - buf = self.stack[-1] + try: + buf = self.stack[-1] + except IndexError: + self._underflow() with memoryview(buf) as m: if not m.readonly: self.stack[-1] = m.toreadonly() @@ -1575,15 +1600,27 @@ def load_empty_tuple(self): dispatch[EMPTY_TUPLE[0]] = load_empty_tuple def load_tuple1(self): - self.stack[-1] = (self.stack[-1],) + stack = self.stack + try: + stack[-1] = (stack[-1],) + except IndexError: + self._underflow() dispatch[TUPLE1[0]] = load_tuple1 def load_tuple2(self): - self.stack[-2:] = [(self.stack[-2], self.stack[-1])] + stack = self.stack + try: + stack[-2:] = [(stack[-2], stack[-1])] + except IndexError: + self._underflow() dispatch[TUPLE2[0]] = load_tuple2 def load_tuple3(self): - self.stack[-3:] = [(self.stack[-3], self.stack[-2], self.stack[-1])] + stack = self.stack + try: + stack[-3:] = [(stack[-3], stack[-2], stack[-1])] + except IndexError: + self._underflow() dispatch[TUPLE3[0]] = load_tuple3 def load_empty_list(self): @@ -1610,6 +1647,8 @@ def load_list(self): def load_dict(self): items = self.pop_mark() + if len(items) % 2: + raise UnpicklingError("odd number of items for DICT") d = {items[i]: items[i+1] for i in range(0, len(items), 2)} self.append(d) @@ -1638,21 +1677,31 @@ def load_inst(self): def load_obj(self): # Stack is ... markobject classobject arg1 arg2 ... args = self.pop_mark() + if not args: + self._underflow() cls = args.pop(0) self._instantiate(cls, args) dispatch[OBJ[0]] = load_obj def load_newobj(self): - args = self.stack.pop() - cls = self.stack.pop() + stack = self.stack + try: + args = stack.pop() + cls = stack.pop() + except IndexError: + self._underflow() obj = cls.__new__(cls, *args) self.append(obj) dispatch[NEWOBJ[0]] = load_newobj def load_newobj_ex(self): - kwargs = self.stack.pop() - args = self.stack.pop() - cls = self.stack.pop() + stack = self.stack + try: + kwargs = stack.pop() + args = stack.pop() + cls = stack.pop() + except IndexError: + self._underflow() obj = cls.__new__(cls, *args, **kwargs) self.append(obj) dispatch[NEWOBJ_EX[0]] = load_newobj_ex @@ -1665,8 +1714,12 @@ def load_global(self): dispatch[GLOBAL[0]] = load_global def load_stack_global(self): - name = self.stack.pop() - module = self.stack.pop() + stack = self.stack + try: + name = stack.pop() + module = stack.pop() + except IndexError: + self._underflow() if type(name) is not str or type(module) is not str: raise UnpicklingError("STACK_GLOBAL requires str") self.append(self.find_class(module, name)) @@ -1723,16 +1776,21 @@ def find_class(self, module, name): def load_reduce(self): stack = self.stack - args = stack.pop() - func = stack[-1] + try: + args = stack.pop() + func = stack[-1] + except IndexError: + self._underflow() stack[-1] = func(*args) dispatch[REDUCE[0]] = load_reduce def load_pop(self): if self.stack: del self.stack[-1] - else: + elif self.metastack: self.pop_mark() + else: + self._underflow() dispatch[POP[0]] = load_pop def load_pop_mark(self): @@ -1740,7 +1798,11 @@ def load_pop_mark(self): dispatch[POP_MARK[0]] = load_pop_mark def load_dup(self): - self.append(self.stack[-1]) + try: + item = self.stack[-1] + except IndexError: + self._underflow() + self.append(item) dispatch[DUP[0]] = load_dup def load_get(self): @@ -1774,38 +1836,60 @@ def load_put(self): i = int(self.readline()[:-1]) if i < 0: raise ValueError("negative PUT argument") - self.memo[i] = self.stack[-1] + try: + value = self.stack[-1] + except IndexError: + self._underflow() + self.memo[i] = value dispatch[PUT[0]] = load_put def load_binput(self): i = self.read(1)[0] if i < 0: raise ValueError("negative BINPUT argument") - self.memo[i] = self.stack[-1] + try: + value = self.stack[-1] + except IndexError: + self._underflow() + self.memo[i] = value dispatch[BINPUT[0]] = load_binput def load_long_binput(self): i, = unpack(' maxsize: raise ValueError("negative LONG_BINPUT argument") - self.memo[i] = self.stack[-1] + try: + value = self.stack[-1] + except IndexError: + self._underflow() + self.memo[i] = value dispatch[LONG_BINPUT[0]] = load_long_binput def load_memoize(self): memo = self.memo - memo[len(memo)] = self.stack[-1] + try: + value = self.stack[-1] + except IndexError: + self._underflow() + memo[len(memo)] = value dispatch[MEMOIZE[0]] = load_memoize def load_append(self): stack = self.stack - value = stack.pop() - list = stack[-1] + try: + value = stack.pop() + list = stack[-1] + except IndexError: + self._underflow() list.append(value) dispatch[APPEND[0]] = load_append def load_appends(self): items = self.pop_mark() - list_obj = self.stack[-1] + try: + list_obj = self.stack[-1] + except IndexError: + self._underflow() try: extend = list_obj.extend except AttributeError: @@ -1823,22 +1907,33 @@ def load_appends(self): def load_setitem(self): stack = self.stack - value = stack.pop() - key = stack.pop() - dict = stack[-1] + try: + value = stack.pop() + key = stack.pop() + dict = stack[-1] + except IndexError: + self._underflow() dict[key] = value dispatch[SETITEM[0]] = load_setitem def load_setitems(self): items = self.pop_mark() - dict = self.stack[-1] + try: + dict = self.stack[-1] + except IndexError: + self._underflow() + if len(items) % 2: + raise UnpicklingError("odd number of items for SETITEMS") for i in range(0, len(items), 2): dict[items[i]] = items[i + 1] dispatch[SETITEMS[0]] = load_setitems def load_additems(self): items = self.pop_mark() - set_obj = self.stack[-1] + try: + set_obj = self.stack[-1] + except IndexError: + self._underflow() if isinstance(set_obj, set): set_obj.update(items) else: @@ -1849,8 +1944,11 @@ def load_additems(self): def load_build(self): stack = self.stack - state = stack.pop() - inst = stack[-1] + try: + state = stack.pop() + inst = stack[-1] + except IndexError: + self._underflow() setstate = getattr(inst, "__setstate__", _NoValue) if setstate is not _NoValue: setstate(state) @@ -1878,7 +1976,10 @@ def load_mark(self): dispatch[MARK[0]] = load_mark def load_stop(self): - value = self.stack.pop() + try: + value = self.stack.pop() + except IndexError: + self._underflow() raise _Stop(value) dispatch[STOP[0]] = load_stop diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index c53262e358b48ea..b2cb9fc95e05ae1 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -1565,6 +1565,40 @@ def test_bad_types(self): self.check_unpickling_error(AttributeError, b'N(\x90.') self.check_unpickling_error(TypeError, b'\x8f(]\x90.') # non-hashable element + def test_bad_opcode(self): + # A printable ASCII byte is reported as is, any other one is escaped. + badpickles = [ + (b'!', "invalid load key, '!'."), + (b"'", r"invalid load key, '\x27'."), + (b'\\', r"invalid load key, '\x5c'."), + (b'\x00', r"invalid load key, '\x00'."), + (b'\x7f', r"invalid load key, '\x7f'."), + (b'\xff', r"invalid load key, '\xff'."), + ] + for p, msg in badpickles: + with self.subTest(data=p): + with self.assertRaises(pickle.UnpicklingError) as cm: + self.loads(p) + self.assertEqual(str(cm.exception), msg) + + def test_bad_stack_messages(self): + # Reaching past a MARK is reported differently from exhausting the + # whole stack. + badpickles = [ + (b'.', 'unpickling stack underflow'), + (b'0', 'unpickling stack underflow'), + (b'N(.', 'unexpected MARK found'), + (b'1', 'could not find MARK'), + (b'(Nd', 'odd number of items for DICT'), + (b'}(Nu', 'odd number of items for SETITEMS'), + (b'((Nu', 'unexpected MARK found'), + ] + for p, msg in badpickles: + with self.subTest(data=p): + with self.assertRaises(pickle.UnpicklingError) as cm: + self.loads(p) + self.assertEqual(str(cm.exception), msg) + def test_bad_stack(self): badpickles = [ b'.', # STOP @@ -1572,6 +1606,7 @@ def test_bad_stack(self): b'1', # POP_MARK b'2', # DUP b'(2', + b'Q', # BINPERSID b'R', # REDUCE b')R', b'a', # APPEND @@ -1611,6 +1646,7 @@ def test_bad_stack(self): b'\x93', # STACK_GLOBAL b'Vlist\n\x93', b'\x94', # MEMOIZE + b'\x98', # READONLY_BUFFER ] for p in badpickles: self.check_unpickling_error(self.bad_stack_errors, p) @@ -1619,6 +1655,7 @@ def test_bad_mark(self): badpickles = [ b'N(.', # STOP b'N(2', # DUP + b'N(Q', # BINPERSID b'cbuiltins\nlist\n)(R', # REDUCE b'cbuiltins\nlist\n()R', b']N(a', # APPEND @@ -1650,6 +1687,7 @@ def test_bad_mark(self): b'Vbuiltins\n(Vlist\n\x93', b'Vbuiltins\nVlist\n(\x93', b'N(\x94', # MEMOIZE + b'N(\x98', # READONLY_BUFFER ] for p in badpickles: self.check_unpickling_error(self.bad_stack_errors, p) diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 5b0fd70105447f9..91181502f873a12 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -55,7 +55,7 @@ class PyPickleTests(AbstractPickleModuleTests, unittest.TestCase): class PyUnpicklerTests(AbstractUnpickleTests, unittest.TestCase): unpickler = pickle._Unpickler - bad_stack_errors = (IndexError,) + bad_stack_errors = (pickle.UnpicklingError,) truncated_errors = (pickle.UnpicklingError, EOFError, AttributeError, ValueError, struct.error, IndexError, ImportError) @@ -101,7 +101,7 @@ def loads(self, buf, **kwds): class InMemoryPickleTests(AbstractPickleTests, AbstractUnpickleTests, BigmemPickleTests, unittest.TestCase): - bad_stack_errors = (pickle.UnpicklingError, IndexError) + bad_stack_errors = (pickle.UnpicklingError,) truncated_errors = (pickle.UnpicklingError, EOFError, AttributeError, ValueError, struct.error, IndexError, ImportError) diff --git a/Misc/NEWS.d/next/Library/2026-09-24-18-57-00.gh-issue-141749.b1O2AX.rst b/Misc/NEWS.d/next/Library/2026-09-24-18-57-00.gh-issue-141749.b1O2AX.rst new file mode 100644 index 000000000000000..9ee4d1d459e2cc9 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-09-24-18-57-00.gh-issue-141749.b1O2AX.rst @@ -0,0 +1,4 @@ +:mod:`pickle`: align exceptions raised by the pure Python implementation of +:func:`pickle.load` with the C implementation. Previous cases raising +:exc:`KeyError` or :exc:`IndexError` now raise +:exc:`~pickle.UnpicklingError`. Patch by Dmitry Voropaev.