diff --git a/msgpack/_unpacker.pyx b/msgpack/_unpacker.pyx index 7e406622..f444472d 100644 --- a/msgpack/_unpacker.pyx +++ b/msgpack/_unpacker.pyx @@ -44,6 +44,7 @@ cdef extern from "unpack.h": msgpack_user user PyObject* obj Py_ssize_t count + unsigned int top ctypedef int (*execute_fn)(unpack_context* ctx, const char* data, Py_ssize_t len, Py_ssize_t* off) except -1 @@ -320,6 +321,10 @@ cdef class Unpacker: cdef Py_ssize_t max_buffer_size cdef uint64_t stream_offset cdef bint _unpacking + # Which of unpack_construct/unpack_skip left an array or map open on + # the last call, so a later call can refuse to resume it with the + # other one instead of touching stack entries it never populated. + cdef execute_fn _resume_execute def __dealloc__(self): unpack_clear(&self.ctx) @@ -473,6 +478,14 @@ cdef class Unpacker: cdef object obj cdef Py_ssize_t prev_head + if (self.ctx.top != 0 and self._resume_execute != NULL + and self._resume_execute != execute): + raise ValueError( + "unpack() and skip() cannot be mixed while an array or " + "map is still incomplete; finish it with the same method " + "that started it" + ) + self._unpacking = True try: while 1: @@ -486,8 +499,10 @@ cdef class Unpacker: if ret == 1: obj = unpack_data(&self.ctx) unpack_init(&self.ctx) + self._resume_execute = NULL return obj if ret == 0: + self._resume_execute = execute if self.file_like is not None: self.read_from_file() continue @@ -497,6 +512,7 @@ cdef class Unpacker: raise OutOfData("No more data to unpack.") unpack_clear(&self.ctx) + self._resume_execute = NULL if ret == -2: raise FormatError elif ret == -3: diff --git a/test/test_sequnpack.py b/test/test_sequnpack.py index 0f895d7d..e0613939 100644 --- a/test/test_sequnpack.py +++ b/test/test_sequnpack.py @@ -1,6 +1,7 @@ #!/usr/bin/env python import io +import pytest from pytest import raises from msgpack import BufferFull, Unpacker, pack, packb @@ -63,6 +64,87 @@ def test_foobar_skip(): unpacker.unpack() +@pytest.mark.skipif( + Unpacker.__module__ == "msgpack.fallback", + reason="only the C extension keeps a stack frame across an incomplete read", +) +def test_skip_then_unpack_across_incomplete_container(): + # skip() opens the array's stack frame without ever populating its + # object slot (it has nothing to build), so resuming with unpack() + # used to reuse that slot as if it held a real list and crash. See + # GH #734. + unpacker = Unpacker() + unpacker.feed(b"\x91") + with raises(OutOfData): + unpacker.skip() + unpacker.feed(b"\x00") + with raises(ValueError): + unpacker.unpack() + + +@pytest.mark.skipif( + Unpacker.__module__ == "msgpack.fallback", + reason="only the C extension keeps a stack frame across an incomplete read", +) +def test_unpack_then_skip_across_incomplete_container(): + unpacker = Unpacker() + unpacker.feed(b"\x91") + with raises(OutOfData): + unpacker.unpack() + unpacker.feed(b"\x00") + with raises(ValueError): + unpacker.skip() + + +def test_skip_then_skip_across_incomplete_container_still_works(): + unpacker = Unpacker() + unpacker.feed(b"\x91") + with raises(OutOfData): + unpacker.skip() + unpacker.feed(b"\x00") + assert unpacker.skip() is None + + +def test_unpack_then_unpack_across_incomplete_container_still_works(): + unpacker = Unpacker() + unpacker.feed(b"\x91") + with raises(OutOfData): + unpacker.unpack() + unpacker.feed(b"\x00") + assert unpacker.unpack() == [0] + + +@pytest.mark.skipif( + Unpacker.__module__ != "msgpack.fallback", + reason="the C extension is the one that needs to reject this mix, see the tests above", +) +def test_fallback_skip_then_unpack_across_incomplete_container_still_works(): + # The fallback never keeps a stack frame across an OutOfData; an + # incomplete read rolls the buffer position back to where the call + # started, so the next call just reparses the array header from + # scratch regardless of which method it uses. No corruption risk here, + # so unlike the C extension it doesn't need to reject the mix. + unpacker = Unpacker() + unpacker.feed(b"\x91") + with raises(OutOfData): + unpacker.skip() + unpacker.feed(b"\x00") + assert unpacker.unpack() == [0] + + +@pytest.mark.skipif( + Unpacker.__module__ != "msgpack.fallback", + reason="the C extension is the one that needs to reject this mix, see the tests above", +) +def test_fallback_unpack_then_skip_across_incomplete_container_still_works(): + unpacker = Unpacker() + unpacker.feed(b"\x91") + with raises(OutOfData): + unpacker.unpack() + unpacker.feed(b"\x00") + assert unpacker.skip() is None + + def test_maxbuffersize(): with raises(ValueError): Unpacker(read_size=5, max_buffer_size=3)