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

Conversions between float and int are not type-checked. #1139

Open
mayuehit opened this issue Aug 22, 2024 · 2 comments
Open

Conversions between float and int are not type-checked. #1139

mayuehit opened this issue Aug 22, 2024 · 2 comments

Comments

@mayuehit
Copy link

Describe the bug
I've been using msgpack-c 4.0.0, which does type checking for conversions between float and int.But when I updated to 4.1.3, I found that the same code produced a different behavior.

To Reproduce

Code:

  float original_value = 1.0;

  msgpack::sbuffer buffer;
  msgpack::pack(buffer, original_value);

  msgpack::object_handle oh = msgpack::unpack(buffer.data(), buffer.size());

  try {
      int unpacked_value = oh.get().as<int>();
      std::cout << "Unpacked value: " << unpacked_value << std::endl;
  } catch (const std::exception& e) {
      std::cerr << "Failed to unpack as int " << e.what() << std::endl;
  }

Output:
When I run this code with msgpack-c 4.0.0

Failed to unpack as int std::bad_cast

But when I run this code with msgpack-c 4.1.3

Unpacked value: 1

Please prepare https://stackoverflow.com/help/minimal-reproducible-example

Expected behavior
I expect it to report an error, because this kind of unchecked type conversion tends to be problematic.

@redboltz
Copy link
Contributor

redboltz commented Aug 22, 2024

Thank you for reporting this issue.
The change was introduced in pull request #1018, which addresses issue #1017 that explains the rationale behind this update.

This update was implemented in version 4.1.2. You can refer to the changelog here: https://github.com/msgpack/msgpack-c/blob/cpp_master/CHANGELOG.md#2022-09-07-version-412-for-c.

At that time, I considered it a minor refinement, so I only incremented the patch version. However, in hindsight, it might have been more appropriate to update the major version.
Anyway, this is an expected behavior.

@mayuehit
Copy link
Author

mayuehit commented Aug 22, 2024 via email

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