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

TimeStamp type uses incorrect endianness? #98

Merged
merged 1 commit into from
Jul 11, 2018
Merged

TimeStamp type uses incorrect endianness? #98

merged 1 commit into from
Jul 11, 2018

Conversation

stefano-pogliani
Copy link
Contributor

Hello,

I have recently attempted to use the new bson::TimeStamp type to decode an optime returned by the replSetGetStatus MongoDB command.
The value that I got was different from the result of rs.status().

After a bit of investigation I noticed the timestamp is converted to big endian before it is masked and shifted: https://github.com/zonyitoo/bson-rs/blob/master/src/decoder/serde.rs#L595

I am not familiar with the protocol but other clients (including the bson package in the mongodb repo) seem to use little endian encoding for the values of the timestamp:

Is this indeed a bug or am I doing something wrong?

@saghm
Copy link
Contributor

saghm commented Jul 10, 2018

This looks like a bug to me; the BSON spec indicates that the value of a Timestamp is the "basic type" uint64, which is described as little endian: http://bsonspec.org/spec.html

@stefano-pogliani
Copy link
Contributor Author

Unrelated to this behaviour I also have a commit to implement serde Serialise on the new bson::TimeStamp that I would like to submit for consideration.

Should that be a separate PR?

@zonyitoo
Copy link
Contributor

That must be a bug. Thanks for your contribution.

@zonyitoo zonyitoo merged commit bf3c702 into mongodb:master Jul 11, 2018
@zonyitoo
Copy link
Contributor

@stefano-pogliani Implement Serialize for bson::TimeStamp separately? How? You should open a PR if you want to.

@stefano-pogliani stefano-pogliani deleted the le-timestamp branch July 11, 2018 07:30
lrlna pushed a commit to lrlna/bson-rs that referenced this pull request Feb 27, 2019
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

Successfully merging this pull request may close these issues.

3 participants