3Hren / msgpack-rust

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

Impossible to decode rmp-serde Ext type even with own trait implemented for rmp_serde::decode::Deserializer #164

Open royaltm opened 6 years ago

royaltm commented 6 years ago

There is no way (or it would be very hacky) to read the Ext meta data from the inner rd reader (the get_ref method only exposes the internal implementation of rd but not the rd itself).

My approach was to implement my own trait for rmp_serde::decode::Deserializer like this:

impl<'de, 'a, R> MyDeserializer<'de> for &'a mut rmp_serde::decode::Deserializer<R>
where R: rmp_serde::decode::Read<'de>
{
    fn deserialize_foo<V: Visitor<'de>>(self, visitor: V) -> Result<V::Value, Self::Error> {
        self.deserialize_any(visitor).or_else(|err| {
            if let rmp_serde::decode::Error::TypeMismatch(marker) = err {
                let meta: ExtMeta = self.somehow_get_ext_meta()?;
                // do something with meta, .e.g. call self.deserialize_any again if type matches
                return Ok(value);
            }
            Err(err)
        })
    }
}

But, there is no way to read the ExtMeta from the internal rd reader of Deserializer. I think the ExtMeta could be easily returned from this place: https://github.com/3Hren/msgpack-rust/blob/master/rmp-serde/src/decode.rs#L347 like this:

marker => {
    let size = match marker {
        Marker::FixExt1 => 1,
        Marker::FixExt2 => 2,
        Marker::FixExt4 => 4,
        Marker::FixExt8 => 8,
        Marker::FixExt16 => 16,
        Marker::Ext8 => read_u8(&mut self.rd)? as u32,
        Marker::Ext16 => read_u16(&mut self.rd)? as u32,
        Marker::Ext32 => read_u32(&mut self.rd)?,
        marker => return Err(Error::TypeMismatch(marker)),
    };
    let ty = rmp::decode::read_data_i8(&mut self.rd)?;
    let meta = rmp::decode::ExtMeta {
        typeid: ty,
        size: size,
    };
    Err(Error::ExtensionFound(meta)) // Would require an ExtensionFound(ExtMeta) variant in Error enum
}

Adding a method to get access to the Deserializer self.rd would also solve the problem anyway.

royaltm commented 6 years ago

with #![feature(specialization)] it would be easy for rmp-serde to make VisitorExt trait in this way (maybe via features):

pub trait VisitorExt<'de>: Visitor<'de> {
    fn visit_ext(self, meta: ExtMeta, _v: &[u8]) -> Result<Self::Value, Error> {
        Err(Error::ExtensionFound(meta))
    }

    fn visit_borrowed_ext(self, meta: ExtMeta, _v: &'de [u8]) -> Result<Self::Value, Error> {
        Err(Error::ExtensionFound(meta))
    }
}

impl<'de,V> VisitorExt<'de> for V where V: Visitor<'de> {}

and then in rmp_serde::decode::Deserialize::deserialize_any in place of https://github.com/3Hren/msgpack-rust/blob/master/rmp-serde/src/decode.rs#L347

            marker => {
                let size = match marker {
                    Marker::FixExt1 => 1,
                    Marker::FixExt2 => 2,
                    Marker::FixExt4 => 4,
                    Marker::FixExt8 => 8,
                    Marker::FixExt16 => 16,
                    Marker::Ext8 => read_u8(&mut self.rd)? as u32,
                    Marker::Ext16 => read_u16(&mut self.rd)? as u32,
                    Marker::Ext32 => read_u32(&mut self.rd)?,
                    marker => return Err(Error::TypeMismatch(marker)),
                };
                let typeid = rmp::decode::read_data_i8(&mut self.rd)?;
                let meta = ExtMeta {
                    typeid,
                    size,
                };
                match self.read_bin_data(size)? {
                    Reference::Borrowed(buf) => VisitorExt::visit_borrowed_ext(visitor, meta, buf),
                    Reference::Copied(buf) => VisitorExt::visit_ext(visitor, meta, buf),
                }
            }

so one could implement optionally (via specialization ability) VisitorExt for MyVisitor

it would be even better if instead of buf the visit_*_ext method would receive instance of deserializer so it would call deserialize.deserialize_whatever on it.