- Added unpack limit. #175
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Added new exceptions to explain limit over errors. - Fixed ext maximum size problem. Problem: The type of m_trail was uint32_t but when parsing ext, it could be 0xffffffff + 1. +1 means type. See https://github.com/msgpack/msgpack/blob/master/spec.md#ext-format-family Solution: Modified the type of m_trail as std::size_t. If sizeof(std::size_t) == 4, 0xffffffff size of ext is an error. If sizeof(std::size_t) == 8, 0xffffffff size of ext is not an error. m_trail is 0xffffffff + 1. Design cohice: I chose std::size_t as the m_trail's type instead of uint64_t intentionally. On 64 addressing bit environment, there is no problem. On 32 bit environment, there is no problem except ext with maximum size. There is only one exception in the all msgpack format. Using uint64_t to support that, it's very expensive. On 32 addressing bit environment, allocating 0xffffffff + 1 bytes of memory couldn't succeed, so I believe that throwing an exception is a reasonable design choice in the case.
Contributor
Author
|
This PR for #159 |
include/msgpack/unpack.hpp
Outdated
Contributor
There was a problem hiding this comment.
I'd think it'd be more convenient to supply both overloads.
Contributor
Author
There was a problem hiding this comment.
Thanks for the comment. I agree with you.
… std::string& are supported. Updated all exceptions relate to unpack take a message parameter. It is better to explain the exceptional situation in detail. So far, just passing the exceptions name.
Member
|
LGTM. By the way, we can use |
redboltz
added a commit
to redboltz/msgpack-c
that referenced
this pull request
Jan 5, 2019
Fixed `msgpack::object` packing visitor and equal comparison visitor. NOTE: In the function `visit_ext(const char* v, uint32_t size)`, v contains type and size means buffer `v` size. See msgpack#175.
Merged
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem:
The type of m_trail was uint32_t but when parsing ext, it could be 0xffffffff + 1. +1 means type. See https://github.com/msgpack/msgpack/blob/master/spec.md#ext-format-family
Solution:
Modified the type of m_trail as std::size_t. If sizeof(std::size_t) == 4, 0xffffffff size of ext is an error. If sizeof(std::size_t) == 8, 0xffffffff size of ext is not an error. m_trail is 0xffffffff + 1.
Design cohice:
I chose std::size_t as the m_trail's type instead of uint64_t intentionally. On 64 addressing bit environment, there is no problem. On 32 bit environment, there is no problem except ext with maximum size. There is only one exception in the all msgpack format. Using uint64_t to support that, it's very expensive. On 32 addressing bit environment, allocating 0xffffffff + 1 bytes of memory couldn't succeed, so I believe that throwing an exception is a reasonable design choice in the case.