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

mspack::pack of msgpack::object containing time is wrong #754

Closed
grmcdorman opened this issue Jan 4, 2019 · 5 comments
Closed

mspack::pack of msgpack::object containing time is wrong #754

grmcdorman opened this issue Jan 4, 2019 · 5 comments

Comments

@grmcdorman
Copy link

The following code:

	auto time_32bit = std::chrono::system_clock::from_time_t(0x32BCFEDA);

	msgpack::zone zone;
	msgpack::object time_object(time_32bit, zone);
	std::stringstream buffer;
	msgpack::pack(buffer, time_object);

results in incorrect output; specifically, the output is:

  • EXT 8 (0xC7) instead of FIXEXT 4 (0xD6)
  • 8 characters instead of six
  • the type byte (0xFF or -1) is repeated
    The actual data returned:
    C7 05 FF FF 32 BC FE DA
    It appears the following code in object.hpp is at fault (line 354 in the current source):
    bool visit_ext(const char* v, uint32_t size) {
        m_packer.pack_ext(size, *v);
        m_packer.pack_ext_body(v, size);
        return true;
    }

The supplied size is the size including the type byte, but pack_ext and pack_ext_body expect the data-only size. In addition, pack_ext_body is passed the data starting with the type byte.

To correct:

    bool visit_ext(const char* v, uint32_t size) {
        m_packer.pack_ext(size - 1, *v);
        m_packer.pack_ext_body(v + 1, size - 1);
        return true;
    }

Generic ext-object packing in this fashion will of course also be affected.

@grmcdorman
Copy link
Author

There may also be an error in comparison:

    bool visit_ext(const char* v, uint32_t size) {
        if (m_ptr->type != msgpack::type::EXT ||
            m_ptr->via.ext.size != size ||
            std::memcmp(m_ptr->via.ext.ptr, v, size) != 0) {
            m_result = false;
            return false;
        }
        return true;
}

size does not count the type byte, so the memcmp is one byte short.

@redboltz
Copy link
Contributor

redboltz commented Jan 5, 2019

@grmcdorman , thank you for reporting the issue. As you mentioned, it is not only time issue but also EXT issue. I will fix it.

@redboltz
Copy link
Contributor

redboltz commented Jan 5, 2019

I sent the PR #755 to fix the issue.
@grmcdorman , could you try this?

@grmcdorman
Copy link
Author

Verified that it works for me. Thank you for the quick fix.

redboltz added a commit that referenced this issue Jan 8, 2019
@redboltz
Copy link
Contributor

redboltz commented Jan 8, 2019

Thank you for checking. Merged.

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

2 participants