Skip to content

Conversation

@djoume
Copy link

@djoume djoume commented Nov 19, 2025

Disclaimer: I used Claude Code to help me make this change

This fix addresses error handling inconsistencies in CPython's pure Python pickle.py implementation to match the behavior of the C _pickle module. The changes make the pure Python implementation raise proper UnpicklingError exceptions for invalid pickle data instead of low-level KeyError and IndexError exceptions.

  • Invalid opcodes now raise UnpicklingError instead of KeyError
  • Operations requiring MARK (TUPLE, LIST, DICT, etc.) now raise UnpicklingError: could not find MARK instead of IndexError: pop from empty list

Note: the C implementation (_pickle) always raises UnpicklingError for all error conditions because it has explicit error checking. The pure Python implementation can't catch all cases without significant performance overhead, so there are still some cases where the pure Python implementation will raise IndexError (e.g., self.stack.pop() on empty stack, self.stack[-1] on empty stack)

This approach:

  • ✅ Improves error messages for common cases
  • ✅ Maintains performance (no try-except around every stack operation)
  • ✅ Passes all existing tests
  • ✅ Makes pure Python implementation more consistent with C implementation
  • ✅ Follows the precedent set in the code (e.g., load_get, load_binget catch KeyError for memo access)

What Gets Fixed

Before the fix:

>>> pickle.loads(b'\xff\xff\xff')
KeyError: 255

>>> pickle.loads(b'this is not valid pickle data')
IndexError: pop from empty list

After the fix:

>>> pickle.loads(b'\xff\xff\xff')
pickle.UnpicklingError: invalid load key, '\xff'.

>>> pickle.loads(b'this is not valid pickle data')
pickle.UnpicklingError: could not find MARK

Compatibility

  • ✅ Backward compatible: Code catching UnpicklingError continues to work
  • ✅ Forward compatible: Code catching IndexError continues to work (for other stack operations)
  • ✅ All existing CPython tests pass
  • ✅ Behavior is closer to C implementation

Impact

This fix benefits:

  • Brython: Primary use case - needs pure Python pickle
  • PyPy: When using pure Python mode
  • MicroPython: If it includes pickle
  • Jython: If using CPython stdlib
  • Debugging: Better error messages for developers
  • Security tools: Proper exception types for malformed pickle validation

This fix addresses error handling inconsistencies in CPython's pure Python `pickle.py` implementation to match the behavior of the C `_pickle` module. The changes make the pure Python implementation raise proper `UnpicklingError` exceptions for invalid pickle data instead of low-level `KeyError` and `IndexError` exceptions.
@python-cla-bot
Copy link

python-cla-bot bot commented Nov 19, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Nov 19, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@djoume djoume changed the title gh-141749 make pure python pickle.py error handling more concistent with the c implementation gh-141749 make pure python pickle.py error handling more consistent with the c implementation Nov 19, 2025
assert isinstance(key, bytes_types)
dispatch[key[0]](self)
try:
dispatch[key[0]](self)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not catch the KeyError for the call. Store the dispatcher separately.


unpickler = pickle._Unpickler
bad_stack_errors = (IndexError,)
bad_stack_errors = (pickle.UnpicklingError, IndexError)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need to catch IndexError?

@@ -0,0 +1 @@
Pure python pickle.py error handling is more consistent with the c implementation, raising UnpicklingError exceptions for invalid pickle data instead of KeyError or IndexError.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Pure python pickle.py error handling is more consistent with the c implementation, raising UnpicklingError exceptions for invalid pickle data instead of KeyError or IndexError.
: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`.

You can also add "Patch by [your name]" at the end.

picnixz

This comment was marked as duplicate.

@picnixz picnixz changed the title gh-141749 make pure python pickle.py error handling more consistent with the c implementation gh-141749: align exceptions raised by pickle.load with _pickle.load Nov 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants