diff --git a/msgpack/unpack_template.h b/msgpack/unpack_template.h index 797a2f91..cb4daa6a 100644 --- a/msgpack/unpack_template.h +++ b/msgpack/unpack_template.h @@ -35,6 +35,8 @@ struct unpack_context { unsigned int cs; unsigned int trail; unsigned int top; + /* mode that started the object on the stack. See unpack_check_mode(). */ + bool construct; unpack_stack stack[MSGPACK_EMBED_STACK_SIZE]; }; @@ -44,6 +46,7 @@ static inline void unpack_init(unpack_context* ctx) ctx->cs = CS_HEADER; ctx->trail = 0; ctx->top = 0; + ctx->construct = true; ctx->stack[0].obj = NULL; } @@ -386,7 +389,27 @@ static inline int unpack_execute(bool construct, unpack_context* ctx, const char #undef again_fixed_trail_if_zero #undef start_container +/* + * skip mode does not build the objects on the stack, so stack[].obj stays + * uninitialized. A later construct pass writes an item through that pointer. + * The reverse order drops the reference that the construct pass took. + * Reject the mode change while an object is still open. + */ +static inline int unpack_check_mode(unpack_context *ctx, bool construct) +{ + if (ctx->top != 0 && ctx->construct != construct) { + PyErr_SetString(PyExc_ValueError, + "cannot switch between unpack and skip while an object is incomplete"); + return -1; + } + ctx->construct = construct; + return 0; +} + static int unpack_construct(unpack_context *ctx, const char *data, Py_ssize_t len, Py_ssize_t *off) { + if (unpack_check_mode(ctx, true) < 0) { + return -1; + } int ret = unpack_execute(1, ctx, data, len, off); if (ret == -1) { unpack_clear(ctx); @@ -394,6 +417,9 @@ static int unpack_construct(unpack_context *ctx, const char *data, Py_ssize_t le return ret; } static int unpack_skip(unpack_context *ctx, const char *data, Py_ssize_t len, Py_ssize_t *off) { + if (unpack_check_mode(ctx, false) < 0) { + return -1; + } int ret = unpack_execute(0, ctx, data, len, off); if (ret == -1) { unpack_clear(ctx); diff --git a/test/test_sequnpack.py b/test/test_sequnpack.py index 0f895d7d..d9d56a83 100644 --- a/test/test_sequnpack.py +++ b/test/test_sequnpack.py @@ -1,7 +1,7 @@ #!/usr/bin/env python import io -from pytest import raises +from pytest import mark, raises from msgpack import BufferFull, Unpacker, pack, packb from msgpack.exceptions import OutOfData @@ -146,3 +146,30 @@ def test_unpack_tell(): m2 = next(unpacker) assert m == m2 assert o == unpacker.tell() + + +@mark.skipif( + Unpacker.__module__ == "msgpack.fallback", + reason="only the C extension keeps parser state between calls", +) +def test_mode_switch_while_incomplete(): + # skip() does not build the objects that unpack() needs, so the parser + # state of one mode is not valid for the other. The unpacker must reject + # the change instead of a crash. The original mode still finishes. + unpacker = Unpacker() + unpacker.feed(b"\x91") + with raises(OutOfData): + unpacker.skip() + unpacker.feed(b"\x00") + with raises(ValueError): + unpacker.unpack() + assert unpacker.skip() is None + + unpacker = Unpacker() + unpacker.feed(b"\x91") + with raises(OutOfData): + unpacker.unpack() + unpacker.feed(b"\x00") + with raises(ValueError): + unpacker.skip() + assert unpacker.unpack() == [0]