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

Fix borrowed Cow<'_, [u8]> deserializing as str. #1175

Merged
merged 1 commit into from Mar 12, 2018
Merged

Fix borrowed Cow<'_, [u8]> deserializing as str. #1175

merged 1 commit into from Mar 12, 2018

Conversation

daboross
Copy link
Contributor

This changes the deserialize implementation for a borrowed Cow<[u8]> to specifically request a byte slice, rather than a borrowed string.

The old behavior breaks any program which relies on data being deserialized the same way as it was serialized and uses Cow<[u8]>. In serde_json, it just wouldn't deserialize. In bincode, it deserialized normally unless the bytes were invalid UTF8.

See bincode-org/bincode#231 for error description, and test program.

I'm not sure where a test for this behavior would fit in, since it's a "what hint to provide the deserializer" failure, but if needed I can try and make one.

Thanks!

This changes the deserialize implementation for a borrowed Cow<[u8]>
to specifically request a byte slice, rather than a borrowed string.

The old behavior breaks any program which relies on data being
deserialized the same way as it was serialized and uses Cow<[u8]>.
In serde_json, it just wouldn't deserialize. In bincode, it
deserialized normally unless the bytes were invalid UTF8.

Fixes bincode-org/bincode#231.
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dtolnay dtolnay merged commit a7a7a31 into serde-rs:master Mar 12, 2018
@daboross daboross deleted the fix-borrowed-cow-bytes branch March 12, 2018 18:45
@dtolnay
Copy link
Member

dtolnay commented Mar 12, 2018

I published this fix in serde 1.0.30.

@daboross
Copy link
Contributor Author

Thanks! Glad to have this working.

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

Successfully merging this pull request may close these issues.

None yet

2 participants