| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
skip() was the only unpack entry point in fallback.py that let a RecursionError escape on deeply nested input; unpack() and __next__() both re-raise it as StackError, and the C extension raises StackError on this path too. Callers guarding against adversarial nesting with except StackError/ValueError were unprotected when skipping. Wrap the body the same way unpack() does, leaving _consume() outside the try so the post-failure buffer state matches.
| Back | FazBrowse Home | New Git URL |
Finding #6 of #683.
Unpacker.skip() is the one unpack entry point in fallback.py that lets a RecursionError escape. unpack() and __next__() both translate it, skip() does not:
so on deeply nested input the two disagree:
The C extension has no such gap: its skip() goes through the same Unpacker._unpack as unpack(), which raises StackError on ret == -3 (_unpacker.pyx:503). So what a caller sees on this path depends on which implementation is loaded.
StackError subclasses ValueError, so code that guards against adversarial nesting with except StackError or except ValueError is unprotected on the skip path under the pure-Python implementation, which is the configuration where the recursion is reachable in the first place.
The fix
Wrap the body the way unpack() already does, four lines away:
self._consume() stays outside the try, matching unpack(), so the buffer state after a failure is the same on both paths. I checked that: after the failure tell() is 0 and a retry raises StackError again, identical to unpack() on the same input.
The except cannot mask a user error here. Under EX_SKIP, _unpack calls no user-supplied hook: the array and map branches recurse with EX_SKIP without touching list_hook / object_hook / object_pairs_hook, and the if execute == EX_SKIP: return short-circuits before ext_hook. The only recursion inside the try is msgpack's own, which makes this guard strictly narrower than the two it copies.
read_array_header() / read_map_header() do not need the same treatment: _unpack returns the header count before it reaches the recursion, so they are unaffected by nesting depth.
Tests
The assertion sits next to the existing one-shot StackError case in test_invalidvalue, since it closes the one-shot vs. streaming gap on the same payload. It fails on main with RecursionError: maximum recursion depth exceeded and passes with the change. It is green under the C extension too, where the same input trips the embed stack limit, so it also pins the parity rather than just the fallback.
Verified on CPython 3.14: full suite green in both the C-extension and MSGPACK_PUREPYTHON=1 configurations, ruff check and ruff format clean.
Disclosure: I used an AI assistant while investigating and writing this patch. I reproduced the divergence myself, confirmed the fix against both builds, and I stand behind the change.