Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Unable to unpack extension with UINT32_MAX bytes of data #1086

Open
fumoboy007 opened this issue Aug 26, 2023 · 4 comments
Open

Unable to unpack extension with UINT32_MAX bytes of data #1086

fumoboy007 opened this issue Aug 26, 2023 · 4 comments

Comments

@fumoboy007
Copy link

fumoboy007 commented Aug 26, 2023

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.

@redboltz
Copy link
Contributor

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:

dfa277a
d6122b4

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

@redboltz
Copy link
Contributor

@tarruda any ideas?

@tarruda
Copy link
Contributor

tarruda commented Aug 27, 2023

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 d6122b4 and 0335df5 ?

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

@fumoboy007
Copy link
Author

(I’m trying to run make at d6122b4 but there are so many build errors. 😭)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants