diff --git a/rmp-serde/src/decode.rs b/rmp-serde/src/decode.rs index fe30c574..1e3efe23 100644 --- a/rmp-serde/src/decode.rs +++ b/rmp-serde/src/decode.rs @@ -1,8 +1,10 @@ //! Generic MessagePack deserialization. +use std::convert::TryInto; use std::error; use std::fmt::{self, Display, Formatter}; use std::io::{self, Cursor, ErrorKind, Read}; +use std::num::TryFromIntError; use std::str::{self, Utf8Error}; use byteorder::{self, ReadBytesExt}; @@ -157,6 +159,13 @@ impl<'a> From> for Error { } } +impl From for Error { + #[cold] + fn from(_: TryFromIntError) -> Self { + Error::OutOfRange + } +} + /// A Deserializer that reads bytes from a buffer. /// /// # Note @@ -327,8 +336,6 @@ impl<'de, R: ReadSlice<'de>, C: SerializerConfig> Deserializer { } fn read_128(&mut self) -> Result<[u8; 16], Error> { - use std::convert::TryInto; - let marker = self.take_or_read_marker()?; if marker != Marker::Bin8 { @@ -508,7 +515,8 @@ impl<'de, 'a, R: ReadSlice<'de>, C: SerializerConfig> serde::Deserializer<'de> f Marker::FixStr(len) => Ok(len.into()), Marker::Str8 => read_u8(&mut self.rd).map(u32::from), Marker::Str16 => read_u16(&mut self.rd).map(u32::from), - Marker::Str32 | _ => read_u32(&mut self.rd).map(u32::from), + Marker::Str32 => read_u32(&mut self.rd).map(u32::from), + _ => unreachable!() }?; self.read_str_data(len, visitor) } @@ -516,27 +524,46 @@ impl<'de, 'a, R: ReadSlice<'de>, C: SerializerConfig> serde::Deserializer<'de> f Marker::Array16 | Marker::Array32 => { let len = match marker { - Marker::FixArray(len) => Ok(len.into()), - Marker::Array16 => read_u16(&mut self.rd).map(u32::from), - Marker::Array32 | _ => read_u32(&mut self.rd).map(u32::from), - }?; - depth_count!(self.depth, visitor.visit_seq(SeqAccess::new(self, len as usize))) + Marker::FixArray(len) => len.into(), + Marker::Array16 => read_u16(&mut self.rd)?.into(), + Marker::Array32 => read_u32(&mut self.rd)?, + _ => unreachable!(), + }; + + depth_count!(self.depth, { + let mut seq = SeqAccess::new(self, len); + let res = visitor.visit_seq(&mut seq)?; + match seq.left { + 0 => Ok(res), + excess => Err(Error::LengthMismatch(len - excess)), + } + }) } Marker::FixMap(_) | Marker::Map16 | Marker::Map32 => { let len = match marker { - Marker::FixMap(len) => Ok(len.into()), - Marker::Map16 => read_u16(&mut self.rd).map(u32::from), - Marker::Map32 | _ => read_u32(&mut self.rd).map(u32::from), - }?; - depth_count!(self.depth, visitor.visit_map(MapAccess::new(self, len as usize))) + Marker::FixMap(len) => len.into(), + Marker::Map16 => read_u16(&mut self.rd)?.into(), + Marker::Map32 => read_u32(&mut self.rd)?, + _ => unreachable!() + }; + + depth_count!(self.depth, { + let mut seq = MapAccess::new(self, len); + let res = visitor.visit_map(&mut seq)?; + match seq.left { + 0 => Ok(res), + excess => Err(Error::LengthMismatch(len - excess)), + } + }) } - Marker::Bin8 | Marker::Bin16| Marker::Bin32 => { + Marker::Bin8 | Marker::Bin16 | Marker::Bin32 => { let len = match marker { Marker::Bin8 => read_u8(&mut self.rd).map(u32::from), Marker::Bin16 => read_u16(&mut self.rd).map(u32::from), - Marker::Bin32 | _ => read_u32(&mut self.rd).map(u32::from), + Marker::Bin32 => read_u32(&mut self.rd).map(u32::from), + _ => unreachable!() }?; match read_bin_data(&mut self.rd, len)? { Reference::Borrowed(buf) => visitor.visit_borrowed_bytes(buf), @@ -652,66 +679,22 @@ impl<'de, 'a, R: ReadSlice<'de>, C: SerializerConfig> serde::Deserializer<'de> f visitor.visit_u128(u128::from_be_bytes(buf)) } - fn deserialize_f32(self, visitor: V) -> Result - where V: Visitor<'de> - { - // This special case allows us to decode some integer types as floats when safe, and - // asked for. - // - // This allows interoperability with msgpack-lite, which performs an optimization of - // storing rounded floats as integers. - let f = match self.take_or_read_marker()? { - Marker::U8 => self.rd.read_data_u8().map(f32::from), - Marker::U16 => self.rd.read_data_u16().map(f32::from), - Marker::I8 => self.rd.read_data_i8().map(f32::from), - Marker::I16 => self.rd.read_data_i16().map(f32::from), - Marker::F32 => self.rd.read_data_f32(), - marker => { - self.marker = Some(marker); - return self.deserialize_any(visitor); - } - }?; - visitor.visit_f32(f) - } - - fn deserialize_f64(self, visitor: V) -> Result - where V: Visitor<'de> - { - // This special case allows us to decode some integer types as floats when safe, and - // asked for. This is here to be consistent with 'f32'. - let f = match self.take_or_read_marker()? { - Marker::U8 => self.rd.read_data_u8().map(f64::from), - Marker::U16 => self.rd.read_data_u16().map(f64::from), - Marker::U32 => self.rd.read_data_u32().map(f64::from), - Marker::I8 => self.rd.read_data_i8().map(f64::from), - Marker::I16 => self.rd.read_data_i16().map(f64::from), - Marker::I32 => self.rd.read_data_i32().map(f64::from), - Marker::F32 => self.rd.read_data_f32().map(f64::from), - Marker::F64 => self.rd.read_data_f64(), - marker => { - self.marker = Some(marker); - return self.deserialize_any(visitor); - } - }?; - visitor.visit_f64(f) - } - forward_to_deserialize_any! { - bool u8 u16 u32 u64 i8 i16 i32 i64 char - str string bytes byte_buf unit seq map - struct identifier tuple tuple_struct - ignored_any + bool u8 u16 u32 u64 i8 i16 i32 i64 f32 + f64 char str string bytes byte_buf unit + seq map struct identifier tuple + tuple_struct ignored_any } } struct SeqAccess<'a, R, C> { de: &'a mut Deserializer, - left: usize, + left: u32, } impl<'a, R: 'a, C> SeqAccess<'a, R, C> { #[inline] - fn new(de: &'a mut Deserializer, len: usize) -> Self { + fn new(de: &'a mut Deserializer, len: u32) -> Self { SeqAccess { de, left: len, @@ -736,17 +719,17 @@ impl<'de, 'a, R: ReadSlice<'de> + 'a, C: SerializerConfig> de::SeqAccess<'de> fo #[inline(always)] fn size_hint(&self) -> Option { - Some(self.left) + self.left.try_into().ok() } } struct MapAccess<'a, R, C> { de: &'a mut Deserializer, - left: usize, + left: u32, } impl<'a, R: 'a, C> MapAccess<'a, R, C> { - fn new(de: &'a mut Deserializer, len: usize) -> Self { + fn new(de: &'a mut Deserializer, len: u32) -> Self { MapAccess { de, left: len, @@ -763,7 +746,7 @@ impl<'de, 'a, R: ReadSlice<'de> + 'a, C: SerializerConfig> de::MapAccess<'de> fo { if self.left > 0 { self.left -= 1; - Ok(Some(seed.deserialize(&mut *self.de)?)) + seed.deserialize(&mut *self.de).map(Some) } else { Ok(None) } @@ -778,7 +761,7 @@ impl<'de, 'a, R: ReadSlice<'de> + 'a, C: SerializerConfig> de::MapAccess<'de> fo #[inline(always)] fn size_hint(&self) -> Option { - Some(self.left) + self.left.try_into().ok() } } diff --git a/rmp-serde/src/lib.rs b/rmp-serde/src/lib.rs index 54cce8ae..acd69371 100644 --- a/rmp-serde/src/lib.rs +++ b/rmp-serde/src/lib.rs @@ -320,7 +320,7 @@ impl<'a> RawRef<'a> { pub fn as_bytes(&self) -> &[u8] { match self.s { Ok(s) => s.as_bytes(), - Err(ref err) => err.0, + Err((bytes, _err)) => bytes, } } } diff --git a/rmp-serde/tests/round.rs b/rmp-serde/tests/round.rs index 75ef451c..026c5bb5 100644 --- a/rmp-serde/tests/round.rs +++ b/rmp-serde/tests/round.rs @@ -565,6 +565,50 @@ fn roundtrip_some() { assert_roundtrips(Some("hi".to_string())); } +/// Some types don't fully consume their input SeqAccess, leading to incorrect +/// deserializes. +/// +/// https://github.com/3Hren/msgpack-rust/issues/287 +#[test] +fn checked_seq_access_len() +{ + #[derive(Serialize)] + struct Input { + a: [&'static str; 4], + d: &'static str, + } + + #[allow(dead_code)] + #[derive(Deserialize, Debug)] + struct Output { + a: [String; 2], + c: String, + } + + let mut buffer = Vec::new(); + let mut serializer = Serializer::new(&mut buffer) + .with_binary() + .with_struct_map(); + + // The bug is basically that Output will successfully deserialize from input + // because the [String; 0] deserializer doesn't drain the SeqAccess, and + // the two fields it leaves behind can then be deserialized into `v` + + let data = Input { + a: ["b", "b", "c", "c"], + d: "d", + }; + + data.serialize(&mut serializer).expect("failed to serialize"); + + let mut deserializer = rmp_serde::Deserializer::new( + Cursor::new(&buffer) + ).with_binary(); + + Output::deserialize(&mut deserializer) + .expect_err("Input round tripped into Output; this shouldn't happen"); +} + #[ignore] #[test] fn roundtrip_some_failures() {