msgpack-c icon indicating copy to clipboard operation
msgpack-c copied to clipboard

Unable to unpack extension with `UINT32_MAX` bytes of data

Open fumoboy007 opened this issue 2 years ago • 4 comments

Describe the bug According to the specification, an extension can have up to UINT32_MAX bytes of data. However, attempting to unpack such an extension using msgpack-c 6.0.0 fails with MSGPACK_UNPACK_PARSE_ERROR.

To Reproduce

TEST(MSGPACKC, simple_buffer_ext_maxlen)
{
    const size_t size = UINT32_MAX;
    void *buf = calloc(size, 1);

    msgpack_sbuffer sbuf;
    msgpack_sbuffer_init(&sbuf);
    msgpack_packer pk;
    msgpack_packer_init(&pk, &sbuf, msgpack_sbuffer_write);

    msgpack_pack_ext(&pk, size, 82);
    msgpack_pack_ext_body(&pk, buf, size);
    msgpack_zone z;
    msgpack_zone_init(&z, 2048);
    msgpack_object obj;
    msgpack_unpack_return ret =
        msgpack_unpack(sbuf.data, sbuf.size, NULL, &z, &obj);

    ASSERT_EQ(MSGPACK_UNPACK_SUCCESS, ret);
    ASSERT_EQ(MSGPACK_OBJECT_EXT, obj.type);
    EXPECT_EQ(82, obj.via.ext.type);
    ASSERT_EQ(size, obj.via.ext.size);
    EXPECT_EQ(0, memcmp(buf, obj.via.ext.ptr, size));

    msgpack_zone_destroy(&z);
    msgpack_sbuffer_destroy(&sbuf);
    free(buf);
}

Expected behavior The test should pass.

fumoboy007 avatar Aug 26 '23 07:08 fumoboy007

Thank you for reporting the issue.

For C++

When I implemented some C++ feature related to EXT32, I did #175 design choice. So C++ version should work well on 64bit environment, but doesn't work 32bit environment. It is intentional design choice. But the document should be added.

For C

It seems that the EXT familiy support is introduced by the following commits:

https://github.com/msgpack/msgpack-c/commit/dfa277a9e019fc9e55549410f43f24f54a8e995b https://github.com/msgpack/msgpack-c/commit/d6122b4a18cbc9829cc1dbebf72890fcab311ff8

If you post a PR to fix the issue, I would merge it.

redboltz avatar Aug 26 '23 08:08 redboltz

@tarruda any ideas?

redboltz avatar Aug 26 '23 08:08 redboltz

No idea, though it has been 9 years since I did those changes so I barely remember anything about this.

@fumoboy007 can you check if this test fails on commits https://github.com/msgpack/msgpack-c/commit/d6122b4a18cbc9829cc1dbebf72890fcab311ff8 and https://github.com/msgpack/msgpack-c/commit/0335df55e1a408c0d56d43e46253c952fb8a7f04 ?

If not, then I suggest using git bisect to find the commit which introduced the problem.

tarruda avatar Aug 27 '23 03:08 tarruda

(I’m trying to run make at https://github.com/msgpack/msgpack-c/commit/d6122b4a18cbc9829cc1dbebf72890fcab311ff8 but there are so many build errors. 😭)

fumoboy007 avatar Aug 29 '23 00:08 fumoboy007