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

"invalid type: sequence" when deserializing type with custom serialization #276

Closed
djc opened this issue Aug 15, 2019 · 5 comments
Closed

Comments

@djc
Copy link

djc commented Aug 15, 2019

In my project this error came up: Custom("invalid type: sequence, expected Secp256k1Point").

Secp256k1Point in this context is from the curv library, with Deserialize and Serialize implementations defined here: https://github.com/KZen-networks/curv/blob/master/src/elliptic/curves/secp256_k1.rs#L521.

(De)serialization with serde_cbor seems to work okay, so maybe there's a bug in bincode? I don't know the subtleties of serde very well, but if someone has some pointers I might be able to contribute a fix for this.

@jdm
Copy link
Collaborator

jdm commented Aug 15, 2019

A minimum code sample to reproduce the problem would help with providing pointers.

@djc
Copy link
Author

djc commented Aug 19, 2019

Here's a minimal example, which I did not find much more enlightening than the code I pointed at earlier:

se bincode;
use std::fmt;
use serde::{Deserialize, Serialize, Serializer};
use serde::ser::SerializeStruct;
use serde::de::{Deserializer, MapAccess, Visitor};

fn main() {
    let p = Point {
        x: b"foo".to_vec(),
        y: b"bar".to_vec(),
    };

    let serialized = serde_cbor::to_vec(&p).unwrap();
    let deserialized: Point = serde_cbor::from_slice(&serialized).unwrap();
    assert_eq!(p, deserialized);

    let serialized = bincode::serialize(&p).unwrap();
    let _: Point = bincode::deserialize(&serialized).expect("bincode deserialization failed");
}

#[derive(Debug, PartialEq)]
struct Point {
    x: Vec<u8>,
    y: Vec<u8>,
}

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

impl<'de> Deserialize<'de> for Point {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: Deserializer<'de>,
    {
        let fields = &["x", "y"];
        deserializer.deserialize_struct("Point", fields, PointVisitor)
    }
}

struct PointVisitor;

impl<'de> Visitor<'de> for PointVisitor {
    type Value = Point;

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

    fn visit_map<E: MapAccess<'de>>(self, mut map: E) -> Result<Point, E::Error> {
        let mut x = vec![];
        let mut y = vec![];

        while let Some(ref key) = map.next_key::<String>()? {
            let v = map.next_value::<Vec<u8>>()?;
            if key == "x" {
                x = v
            } else if key == "y" {
                y = v
            } else {
                panic!("Serialization failed!")
            }
        }

        Ok(Point { x, y })
    }
}

From staring at the code a bit, I think the issue is that bincode always wants to serialize structs as tuples, but this custom serializer expected a map. As such, the deserializer is getting a (tuple) sequence (since deserialize_struct() is called) even though it expects to get a map. I'm not sure how this is supposed to work. Perhaps serialization should add some kind of tag to indicate that there's a custom implementation in play, and then the deserializer can build a MapAccess representation instead of a SeqAccess one?

@jdm
Copy link
Collaborator

jdm commented Aug 19, 2019

Is there a reason that Serialize and Deserialize need to be defined manually rather than derived?

@djc
Copy link
Author

djc commented Aug 19, 2019

I suppose they use it because they want hex-encoded strings in their JSON rather than lists of numbers; but this is in a crate upstream from my project, so I don't necessarily have control over it.

@djc
Copy link
Author

djc commented Sep 5, 2019

I found this documentation in the serde book: "The implementation supports two possible ways that a struct may be represented by a data format: as a seq like in Bincode, and as a map like in JSON." So it looks like Deserialize impls that want to be generic over Deserializer implementations should just implement both to be compatible with bincode.

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

2 participants