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

Use of deserialize_any breaks deserialization with bincode #43

Closed
jekirl opened this issue Oct 1, 2017 · 4 comments · Fixed by #256
Closed

Use of deserialize_any breaks deserialization with bincode #43

jekirl opened this issue Oct 1, 2017 · 4 comments · Fixed by #256
Assignees

Comments

@jekirl
Copy link
Contributor

jekirl commented Oct 1, 2017

Currently the deserialize implementation calls the serde deserialize_any function, which breaks Decimal serialization with non-self describing formats like bincode [0].

When implementing Deserialize, you should avoid relying on Deserializer::deserialize_any unless you need to be told by the Deserializer what type is in the input. Know that relying on Deserializer::deserialize_any means your data type will be able to deserialize from self-describing formats only, ruling out Bincode and many others.

There are two possible easy fixes:

  • Replace deserialize_any with deserialize_str
  • Simply have Decimal derive Serialize, Deserialize

[0] https://serde.rs/impl-deserialize.html

@paupino paupino self-assigned this Oct 4, 2017
@paupino
Copy link
Owner

paupino commented Oct 4, 2017

Thanks for that write up; I'll take a look into it.

I'm a little reluctant to simply replace with deserialize_str at risk of breaking:

{
   "amount": 100
}

This may, however, be a compromise to support bincode etc. I'll do some further investigation.

@paupino
Copy link
Owner

paupino commented Oct 4, 2017

Interestingly, simply changing it to deserialize_str works for the existing tests. The only type it won't do is:

{
  "amount": 100.00
}

I'll push in a change for the above now and create a new ticket for float support since it was something that didn't work before.

@Palmik
Copy link

Palmik commented Jun 14, 2020

Hi. Thanks for this crate. The deserialize_str was swapped back for deserialize_any in this commit so bincode no longer works by default.

@paupino paupino reopened this Jul 4, 2020
@paupino
Copy link
Owner

paupino commented Jul 4, 2020

I'll look into this a bit some more. I may try and wrap either this or the any into a feature so that it gives library consumers the opportunity to select behavior as needed.

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 a pull request may close this issue.

3 participants