Message pack appears to calculate integer boundaries incorrectly

[brianbruggeman]

There are several instances of this specific pattern, though I only find it in the one file.

See: https://github.com/aerospike/aerospike-client-rust/blob/master/src/msgpack/encoder.rs#L257

And a playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=b2ebb5a04b0a8464862963e413f57155

In this line, it kind of looks like 2 ^ 16 implies 2 raised to the power of 16. However, Rust will calculate this as a bitwise operation and will result in 18. So I think in this specific instance, if a value is less than 16, then we'll pack a half byte. If it's between 16 and 18, then we'll pack an i16. Otherwise, we'll pack an i32. And my assumption here is that we're nearly always packing an i32. This isn't a "breaking" bug, but it definitely would lead to inefficiencies for some use-cases and it should be really easy to build a test-case.

I looked at git blame, and I think this has been a bug for several years.

[kportertx]

This mistake was repeated several times in the encoder. These values should have been 1 << 16. We do take pull requests if you'd like to submit one. I'll make sure our client team is aware of this issue, thanks for raising it.

[jhecking]

The inefficient msgpack packing can cause incorrect results for some HLL operations on server versions 5.3 or earlier: https://github.com/aerospike/aerospike-client-rust/pull/93#issuecomment-767373402

[kportertx]

@brianbruggeman, forgot to ask, have you filled out a contributor agreement?

In order to accept a code, Aerospike would need you to fill out a contributor agreement. The second link is a simple form you should fill and submit. https://www.aerospike.com/community/contributor/ https://www.aerospike.com/community/contributor-license-form/

[brianbruggeman]

Thanks!! I submitted the form.