Skip to content
New issue

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

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

Already on GitHub? Sign in 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. 😭)

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

No branches or pull requests

3 participants