3Hren / msgpack-rust

MessagePack implementation for Rust / msgpack.org[Rust]
MIT License
1.15k stars 131 forks source link

Error when deserialize enum with rmp serde #153

Open LooMaclin opened 6 years ago

LooMaclin commented 6 years ago

Repo to reproduce error: https://github.com/LooMaclin/reproduce_rmp_serde_deserialize_enum_error

Code example:

extern crate rmp_serde;
extern crate serde;
#[macro_use]
extern crate serde_derive;

#[derive(Debug, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
struct  A {
    b: B,
}

#[derive(Debug, Serialize, Deserialize)]
#[serde(tag = "kind", content = "data")]
#[serde(rename_all = "snake_case")]
pub enum B {
    First,
    Second
}

fn main() {
    println!("Hello, world!");
    let a = A {
        b: B::First,
    };
    let a = rmp_serde::to_vec(&a).unwrap();
    let a: A = rmp_serde::from_slice(&a).unwrap();
    println!("a: {:?}", a);
}

Error:

[loomaclin@loomaclin reproduce_rmp_serde_tagged_enums_error]$ cargo run
   Compiling reproduce_rmp_serde_tagged_enums_error v0.1.0 (file:///home/loomaclin/reproduce_rmp_serde_tagged_enums_error)
    Finished dev [unoptimized + debuginfo] target(s) in 1.7 secs
     Running `target/debug/reproduce_rmp_serde_tagged_enums_error`
Hello, world!
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Syntax("invalid length 1, expected adjacently tagged enum B")', /checkout/src/libcore/result.rs:916:4
note: Run with `RUST_BACKTRACE=1` for a backtrace.

rustup show:

[loomaclin@loomaclin reproduce_rmp_serde_tagged_enums_error]$ rustup show
Default host: x86_64-unknown-linux-gnu

installed toolchains
--------------------

stable-x86_64-unknown-linux-gnu
nightly-2017-09-07-x86_64-unknown-linux-gnu
nightly-x86_64-unknown-linux-gnu (default)

active toolchain
----------------

nightly-x86_64-unknown-linux-gnu (default)
rustc 1.24.0-nightly (cddc4a62d 2017-12-26)
hjiayz commented 6 years ago

tried

[derive(Debug, Serialize, Deserialize)]

[serde(tag = "kind", content = "data")]

[serde(rename_all = "snake_case")]

pub enum B { First(i64), Second(i64) }

or

[derive(Debug, Serialize, Deserialize)]

[serde(tag = "kind")]

[serde(rename_all = "snake_case")]

pub enum B { First, Second }

passed

LooMaclin commented 6 years ago

@hjiayz can you run code from this repo https://github.com/LooMaclin/reproduce_rmp_serde_deserialize_enum_error and show me output?

also rustup show output plz

royaltm commented 6 years ago

serde_json serializes simple enum variants (no wrapped value) as strings:

extern crate serde;
extern crate serde_json;
#[macro_use] extern crate serde_derive;

#[derive(Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum B {
    First,
    Second
}

fn main() {
    assert_eq!("\"second\"", serde_json::to_string(&B::Second).unwrap());
}

Frankly I have been expecting the same behavior from rmp-serde. After serializing with_names with rmp_serde we get Array([Integer(PosInt(1)), Array([])]) instead of MsgPack string.

royaltm commented 6 years ago

IMHO encoding enum as some internal variant value is inherently wrong. MsgPack is a serialization format and as such, it should not give away the internal implementation of the enum value. What if I want to use the serialized data in a program written in another language? Or why we should even care about the order of the enum variants? There is no specific "enum" datatype in MsgPack spec, so I think we should prefer the most portable way (default) of serializing data over the efficient one in this case. Or at least give a way to configure this behavior. If we want to be efficient we should probably use some Ext type anyway.

tmpolaczyk commented 5 years ago

Currently it is impossible to store anything more than a single enum:

#[test]
fn option_option() {
    let a = Some(Some(1u8));
    let msp = rmp_serde::to_vec(&a).unwrap();
    assert_eq!(rmp_serde::from_slice::<Option<Option<u8>>>(&msp).unwrap(), a);
}