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

#[serde(flatten)] causes error SequenceMustHaveLength #245

Closed
jasonwhite opened this issue Aug 28, 2018 · 7 comments
Closed

#[serde(flatten)] causes error SequenceMustHaveLength #245

jasonwhite opened this issue Aug 28, 2018 · 7 comments

Comments

@jasonwhite
Copy link

This example has a runtime error upon deserializing:

#[macro_use]
extern crate serde_derive;
extern crate bincode;

use bincode::{serialize, deserialize};

#[derive(Serialize, Deserialize, PartialEq, Debug)]
struct Bar {
    x: usize,
    y: usize,
}

#[derive(Serialize, Deserialize, PartialEq, Debug)]
struct Foo {
    #[serde(flatten)]
    bar: Bar,

    z: usize,
}

fn main() {
    let foo = Foo {
        bar: Bar {
            x: 1,
            y: 2,
        },
        z: 3,
    };

    let encoded: Vec<u8> = serialize(&foo).unwrap();

    // Fails here with error:
    // `bincode::ErrorKind::SequenceMustHaveLength`
    //
    // Removing `#[serde(flatten)]` fixes it.
    let decoded: Foo = deserialize(&encoded[..]).unwrap();

    assert_eq!(foo, decoded);
}

Cargo.toml dependencies:

[dependencies]
serde = "1"
serde_derive = "1"
bincode = "1"

Flatten seems to be incompatible with bincode. Is this a known issue? If I were only deserializing from bincode, then I wouldn't use flatten at all, but I also need to deserialize from JSON thus the requirement.

My current workaround is to manually implement Serialize and Deserialize like so:

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

use std::fmt;

use serde::de::{self, Deserialize, Deserializer, MapAccess, SeqAccess, Visitor};
use serde::ser::{Serialize, SerializeStruct, Serializer};

use bincode::{deserialize, serialize};

#[derive(PartialEq, Debug)]
struct Bar {
    x: usize,
    y: usize,
}

#[derive(PartialEq, Debug)]
struct Foo {
    bar: Bar,

    z: usize,
}

impl Serialize for Foo {
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
    where
        S: Serializer,
    {
        let mut state = serializer.serialize_struct("foo", 3)?;
        state.serialize_field("x", &self.bar.x)?;
        state.serialize_field("y", &self.bar.y)?;
        state.serialize_field("z", &self.z)?;
        state.end()
    }
}

impl<'de> Deserialize<'de> for Foo {
    fn deserialize<D>(deserializer: D) -> Result<Foo, D::Error>
    where
        D: Deserializer<'de>,
    {
        #[derive(Deserialize)]
        #[serde(field_identifier, rename_all = "lowercase")]
        enum Field {
            X,
            Y,
            Z,
        }

        const FIELDS: &[&str] = &["x", "y", "z"];

        struct FooVisitor;

        impl<'de> Visitor<'de> for FooVisitor {
            type Value = Foo;

            fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
                formatter.write_str("struct Foo")
            }

            fn visit_seq<V>(self, mut seq: V) -> Result<Self::Value, V::Error>
            where
                V: SeqAccess<'de>,
            {
                let x = seq
                    .next_element()?
                    .ok_or_else(|| de::Error::invalid_length(0, &self))?;
                let y = seq
                    .next_element()?
                    .ok_or_else(|| de::Error::invalid_length(1, &self))?;
                let z = seq
                    .next_element()?
                    .ok_or_else(|| de::Error::invalid_length(2, &self))?;

                Ok(Foo {
                    bar: Bar { x, y },
                    z,
                })
            }

            // Must implement `visit_map` too for JSON deserialization.
            fn visit_map<V>(self, mut map: V) -> Result<Self::Value, V::Error>
            where
                V: MapAccess<'de>,
            {
                let mut x = None;
                let mut y = None;
                let mut z = None;

                while let Some(key) = map.next_key()? {
                    match key {
                        Field::X => {
                            if x.is_some() {
                                return Err(de::Error::duplicate_field("x"));
                            }

                            x = Some(map.next_value()?);
                        }
                        Field::Y => {
                            if y.is_some() {
                                return Err(de::Error::duplicate_field("y"));
                            }

                            y = Some(map.next_value()?);
                        }
                        Field::Z => {
                            if z.is_some() {
                                return Err(de::Error::duplicate_field("z"));
                            }

                            z = Some(map.next_value()?);
                        }
                    }
                }

                let x = x.ok_or_else(|| de::Error::missing_field("x"))?;
                let y = y.ok_or_else(|| de::Error::missing_field("y"))?;
                let z = z.ok_or_else(|| de::Error::missing_field("z"))?;

                Ok(Foo {
                    bar: Bar { x, y },
                    z,
                })
            }
        }

        deserializer.deserialize_struct("foo", FIELDS, FooVisitor)
    }
}

fn main() {
    let foo = Foo {
        bar: Bar { x: 1, y: 2 },
        z: 3,
    };

    let encoded: Vec<u8> = serialize(&foo).unwrap();

    // Works with custom serialization/deserialization.
    let decoded: Foo = deserialize(&encoded[..]).unwrap();

    assert_eq!(foo, decoded);
}
@zimond
Copy link

zimond commented Apr 2, 2019

This is still happening

@JoshMcguigan
Copy link
Contributor

JoshMcguigan commented May 2, 2019

I've confirmed this behavior, with one minor difference. It actually panics when serializing the data, not when deserializing. One additional note, running serialized_size returns the same error for this struct.

I'll review the code a bit and see if I can understand why this is happening.

@JoshMcguigan
Copy link
Contributor

This is caused by the way serde implements flatten. When you flatten, serde uses the map type rather than the struct type. See this serde issue for details. This explains why your custom serialize impl works, because you still serialize the object as a struct.

The remaining question is whether it would be possible for serde to provide length information to bincode when serializing this "map", because in that case I believe bincode would still work. Currently, fn serialize_map(self, len: Option<usize>) -> Result<Self::SerializeMap> in bincode is being passed a None for len, which is the direct cause of this error.

@JoshMcguigan
Copy link
Contributor

Finally wrapping up this research. The reason serde cannot provide size information to bincode when flattening a struct is related to the implementation of the macro system in Rust. There is no way for the derive(Serialize) on Foo to know anything about Bar. This means the knowledge of how many fields are on Bar, which is needed to determine the overall size of Foo, is not available to the derive macro.

@mitsuhiko
Copy link

This seems impossible to fix in serde itself. When I contributed the flatten support I did not find a way to make it work with protocols that do not have maps without length information.

I'm not sure how to fix this but with bincode being used to transfer data for ipc it's not uncommon now to have objects that are internally using flatten. It would be nice to be able to support such objects in a less efficient way. Right now the workaround is to go through serde_json (annoyingly it can't be serde_json::Value either because "any serialization" is also not supported by bincode).

@ZoeyR
Copy link
Collaborator

ZoeyR commented Mar 15, 2020

Bincode has one more issue that makes it difficult to fix this. On deserialize, structs annotated with flatten use deserialize_identifier, which bincode does not support.

@ZoeyR
Copy link
Collaborator

ZoeyR commented May 19, 2020

I'm going to track this as part of #167

@ZoeyR ZoeyR closed this as completed May 19, 2020
phodal added a commit to datum-lang/scie that referenced this issue Oct 12, 2020
vbarrielle added a commit to sparsemat/sprs that referenced this issue Dec 20, 2020
This works around a bug encountered with the combination of serde and
bincode when there is a flatten attribute, see
bincode-org/bincode#245
vbarrielle added a commit to sparsemat/sprs that referenced this issue Dec 21, 2020
This works around a bug encountered with the combination of serde and
bincode when there is a flatten attribute, see
bincode-org/bincode#245
vbarrielle added a commit to sparsemat/sprs that referenced this issue Dec 21, 2020
This works around a bug encountered with the combination of serde and
bincode when there is a flatten attribute, see
bincode-org/bincode#245
noib3 added a commit to nomad/cola that referenced this issue Jul 16, 2023
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

No branches or pull requests

5 participants