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

Slight encoding error in the BSON encoder #243

Closed
PapeCoding opened this issue Jun 3, 2020 · 3 comments
Closed

Slight encoding error in the BSON encoder #243

PapeCoding opened this issue Jun 3, 2020 · 3 comments

Comments

@PapeCoding
Copy link

Hello Daniel,
First: Great work with this library, exactly what I needed! Keep it up! 🎉

Today I wanted to export some BSON with the extension and noticed, that the other side (reading the bson) ran into trouble. After drilling down on the issue, I found a slight mistake in the encoder's visit_int64 method.

In the original one, it selects based on the tag, if it puts the identifier of jsoncons::bson::detail::bson_format::int64_cd or jsoncons::bson::detail::bson_format::datetime_cd into the buffer and then decides if the number fits into 32 or 64 bits. If it then decides to write only 32 bits, the identifier is already written and the other side expects 64 bits.

I would suggest altering the code like so: (This is only tested in my own project so far, please verify!)

if (tag == semantic_tag::timestamp)
{
	before_value(jsoncons::bson::detail::bson_format::datetime_cd);
	jsoncons::detail::native_to_little(static_cast<int64_t>(val),std::back_inserter(buffer_));
}
else if (val >= (std::numeric_limits<int32_t>::lowest)() && val <= (std::numeric_limits<int32_t>::max)())
{
	before_value(jsoncons::bson::detail::bson_format::int32_cd);
	jsoncons::detail::native_to_little(static_cast<uint32_t>(val),std::back_inserter(buffer_));
}
else if (val >= (std::numeric_limits<int64_t>::lowest)() && val <= (std::numeric_limits<int64_t>::max)())
{
	before_value(jsoncons::bson::detail::bson_format::int64_cd);
	jsoncons::detail::native_to_little(static_cast<int64_t>(val),std::back_inserter(buffer_));
}
else
{
	// error
}

return true;

The same problem also occurs with the visit_uint64 method, which can be fixed in the same manner:

if (tag == semantic_tag::timestamp)
{
	before_value(jsoncons::bson::detail::bson_format::datetime_cd);
	jsoncons::detail::native_to_little(static_cast<uint64_t>(val),std::back_inserter(buffer_));
}		
else if (val <= (std::numeric_limits<int32_t>::max)())
{
	before_value(jsoncons::bson::detail::bson_format::int32_cd);
	jsoncons::detail::native_to_little(static_cast<uint32_t>(val),std::back_inserter(buffer_));
}
else if (val <= (uint64_t)(std::numeric_limits<int64_t>::max)())
{
	before_value(jsoncons::bson::detail::bson_format::int64_cd);
	jsoncons::detail::native_to_little(static_cast<uint64_t>(val),std::back_inserter(buffer_));
}
else
{
	// error
}

return true;
@danielaparker
Copy link
Owner

danielaparker commented Jun 4, 2020

Thanks for reporting this, I've incorporated your proposed changes into master. Clearly we didn't have enough test cases, no coverage for int32. I've added some tests from the mongodb/libbson test suite that cover null, bool, int32, int64, double, utf8 string, binary, document, and array, which all pass.

@PapeCoding
Copy link
Author

Nice, thank you!

@danielaparker
Copy link
Owner

Fix is in v0.153.1

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

No branches or pull requests

2 participants