msgpack-rust icon indicating copy to clipboard operation
msgpack-rust copied to clipboard

Struct with named fields can be deserialized from sequence

Open ghost opened this issue 5 years ago • 2 comments

This issue in Serde affects Serde Msgpack.

Serde Msgpack unlike other format implementations has an option to the serializer to tell whether to serialize structs as a map with named fields or as a sequence without field names. The deserializer doesn't however have the corresponding option, but instead relies on this issue to accept both forms, which is usually not what is wanted, and comes with the unwanted consequences mentioned in the issue in Serde. Either the deserializer should have this option added, or even better, remove the option from the serializer and let the serialize implementation of the struct decide the format. The correct way to specify that a struct is going to be serialized without field names is in the serialize implementation of the struct. A container attribute can be added to Serde Derive for this purpose.

Example code adapted to Serde Msgpack:

use serde_derive::Deserialize;

#[derive(Debug, Deserialize)]
struct Person {
    first_name: String,
    last_name: String,
}

fn main() {
    let data = rmp_serde::to_vec(&("John", "Doe")).unwrap();
    eprintln!("{:?}", data);
    eprintln!("{:#?}", rmp_serde::from_slice::<Person>(&data));
}

ghost avatar Jul 27 '19 12:07 ghost

which is usually not what is wanted

Just speaking as another user, there are definitely use cases for loosely-validated deserialization. My use case isn't every use case of course, but being able to recover badly-serialized data can be more valuable than strict validation if fixing the application serializing stuff is out of the picture.

daboross avatar Aug 01 '19 00:08 daboross

Personally I find it weird that you decide when you serialize whether you want to encode struct values as tuples or maps. Seems like it would be much more consistent to do it on a struct-by-struct basis with macros. Eg:

#[derive(Debug, Deserialize)]
#[rmp_serde(map)]
struct Person {
    #[serde(rename("firstName"))]
    first_name: String,
    last_name: String,
}

Then it would be really easy to mix & match tuples and map based structs across an application as needed.

josephg avatar Nov 12 '19 22:11 josephg