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

Custom deserialization - seq.next_element() not working as expected on last element of struct #2468

Closed
SiKreuz opened this issue Jun 5, 2023 · 5 comments
Labels

Comments

@SiKreuz
Copy link

SiKreuz commented Jun 5, 2023

Hello,

I have a problem with deserializing a struct (byte-array using bincode2 to struct). The struct looks like this:

#[derive(PartialEq, Debug)]
pub struct OnionTunnelBuild {
    pub reserved: u8,
    pub is_ipv6: bool,
    pub port: u16,
    pub ip: Either<Ipv4Addr, Ipv6Addr>,
    pub host_key: Vec<u8>,
}

I cannot use the derive attribute for deserializing because I need to deserialize the ip address depending on the version. Actually, that part works fine in my custom implementation. I went through the deserialization and everything worked out perfectly fine until I get to the host_key. When I want to fill the host_key of variable length with the rest of the byte array, I get into trouble. I get the following error message:

called `Result::unwrap()` on an `Err` value: Io(Error { kind: UnexpectedEof, message: "failed to fill whole buffer" })

Calling seq.next_element() on the host_key: Vec<u8> only gives me one u8. So seq seems not to be "empty" in the end and this error is thrown.

My implementation:

impl<'de> Deserialize<'de> for OnionTunnelBuild {
    fn deserialize<D>(deserializer: D) -> Result<OnionTunnelBuild, D::Error>
        where
            D: Deserializer<'de>
    {
        struct OnionTunnelBuildVisitor;

        impl<'de> Visitor<'de> for OnionTunnelBuildVisitor {
            type Value = OnionTunnelBuild;

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

            fn visit_seq<V>(self, mut seq: V) -> Result<OnionTunnelBuild, V::Error>
                where
                    V: SeqAccess<'de>,
            {
                let reserved = seq.next_element()?.ok_or_else(|| serde::de::Error::invalid_length(0, &self))?;
                let is_ipv6 = seq.next_element()?.ok_or_else(|| serde::de::Error::invalid_length(1, &self))?;
                let port = seq.next_element()?.ok_or_else(|| serde::de::Error::invalid_length(2, &self))?;

                let ip = if is_ipv6 {
                    Either::Right(seq.next_element()?.ok_or_else(|| serde::de::Error::invalid_length(3, &self))?)
                } else {
                    Either::Left(seq.next_element()?.ok_or_else(|| serde::de::Error::invalid_length(3, &self))?)
                };

                // crashing in the next line
                let host_key =  seq.next_element()?.ok_or_else(|| serde::de::Error::invalid_length(4, &self))?;

                Ok(OnionTunnelBuild {
                    reserved,
                    is_ipv6,
                    port,
                    ip,
                    host_key,
                })
            }
        }

        const FIELDS: &'static [&'static str] = &["reserved", "is_ipv6", "port", "ip", "host_key"];
        deserializer.deserialize_struct("OnionTunnelBuild", FIELDS, OnionTunnelBuildVisitor)
    }
}

Can someone help me on this? I was thinking that seq.next_element() is supposed to deserialize the rest into a Vec<u8>. Because that's what happens with derive attribute and a Vec<u8> in the end. Unfortunately, that's not the case here.

@oli-obk oli-obk added the support label Jun 5, 2023
@Mingun
Copy link
Contributor

Mingun commented Jun 5, 2023

Vec<u8> deserialized using deserialize_seq, but I think you want to read data using deserialize_bytes. Try to deserialize into serde_bytes::ByteBuf and unpack the result.

@Mingun
Copy link
Contributor

Mingun commented Jun 5, 2023

From your description I could assume that you expected that host_key is a trailing bytes after other fields of your struct, but both of deserialize_bytes and deserialize_seq in bincode expects some count of bytes with data length and the specified count of bytes after that. deserialize_tuple reads only data bytes, but doing that byte-per-byte (inefficiently).

So I think that you get error because data is interpreted as length which gives you very big size to read and trying to read that count of bytes exhausts the whole data stream.

@SiKreuz
Copy link
Author

SiKreuz commented Jun 6, 2023

@Mingun thanks for your quick response. That's a good hint. My program is actually crashing because deserialize_seq of bincode tries to get a u64 first to determine the length. In my test case I only put two bytes into the Vec. That can obviously not work.

However, I don't want to read the length from the Vec. So this problem persists. I do actually know the length before deserializing, but it can vary on every run. Can I somehow provide the expected length and use it to deserialize the Vec with it?

@Mingun
Copy link
Contributor

Mingun commented Jun 6, 2023

As I already noted, only deserialize_tuple does not read length, but it probably will be slow. You can try to implement you own bincode deserializer based on non-serde bincode API, but if you do that then probably it will be more simple just to use that API to read your struct without serde.

There was already an issue of requesting API for reading byte arrays with known length, which was closed as not needed (wrongly, I think): #2120. Actually, you need exactly that API

@SiKreuz
Copy link
Author

SiKreuz commented Jun 10, 2023

I just read through the other issue and yes, that seems to be exactly what I would need.
Well, it's not implemented so I cannot use it. The deserialize_tuple is also not an option as a requirement for me is performance. Seems that I have to just implement my (de-)serialization without serde.

Anyways, thanks for you help!

@SiKreuz SiKreuz closed this as completed Jun 10, 2023
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