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

Path serialization can panic #57

Closed
tbu- opened this issue Apr 15, 2015 · 5 comments
Closed

Path serialization can panic #57

tbu- opened this issue Apr 15, 2015 · 5 comments
Labels
Milestone

Comments

@tbu-
Copy link
Contributor

tbu- commented Apr 15, 2015

The Serialize implementation of Path incorrectly assumes that all paths can be represented as Unicode strings.

@erickt
Copy link
Member

erickt commented Apr 15, 2015

Good point! Should be simple enough to have it return a syntax error if it gets a non-unicode string.

@erickt
Copy link
Member

erickt commented Apr 15, 2015

err, rather tries to serialize to a non-unicode string. So it could either just be serialized as a byte string, or we first try to encode it as a string, then fail back to a byte string if it's non-unicode.

@tbu-
Copy link
Contributor Author

tbu- commented Jan 5, 2016

What should happen if you serialize the path \xff on Linux and try to deserialize that on Windows? Similarly, what happens if you serialize a lone surrogate on Windows and try to deserialize that on Linux?

Actually, what about path delimiters? :( Should serialization/deserialization even work across systems? What happens for usize-incompatiblities?

@erickt
Copy link
Member

erickt commented Jan 28, 2016

@tbu-: ah the madness of cross platform things. This is one area where I'm looking forward to #198, which eases overriding which serializer gets used for a field. That'd allow someone with more specific knowledge than serde about how something would get used.

For a default, I'd just rather have serde be able to serialize and deserialize the value and get the same results. It's more up to the serializers/deserializers/users to decide if they need to conform the serialized format to some well defined schema.

erickt added a commit to erickt/serde that referenced this issue Jan 28, 2016
The only way to safely serialize a `Path` is to use
`.to_string_lossy()`, which replaces invalid UTF-8 characters with
the U+FFFD replacement character. Unfortunately this would lose
information, so for our default implementations, it'd be better
to punt and report an error, and leave it up to the user to
decide if they want to use the lossy encoding.

Unfortunately, we had no way for `Serializer`s to require some methods
on `Serializer::Error`, so there was no way before this patch for
the `Path` implementation to generically report that it cannot encode
this value. This adds that implementation.

breaking-change

Closes serde-rs#57.
@erickt
Copy link
Member

erickt commented Feb 17, 2016

This was implemented in #224 for version 0.7.

@erickt erickt closed this as completed Feb 17, 2016
@dtolnay dtolnay added the bug label May 14, 2016
@dtolnay dtolnay added this to the v0.7.0 milestone May 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants