diff --git a/src/serde/de.rs b/src/serde/de.rs index d819e822..ffdc24d2 100644 --- a/src/serde/de.rs +++ b/src/serde/de.rs @@ -30,8 +30,8 @@ where Node::Static(StaticNode::U64(n)) => visitor.visit_u64(n), #[cfg(feature = "128bit")] Node::Static(StaticNode::U128(n)) => visitor.visit_u128(n), - Node::Array { len, count: _ } => visitor.visit_seq(CommaSeparated::new(self, len)), - Node::Object { len, count: _ } => visitor.visit_map(CommaSeparated::new(self, len)), + Node::Array { len, count: _ } => visit_array(self, len, visitor), + Node::Object { len, count: _ } => visit_object(self, len, visitor), } } @@ -237,7 +237,7 @@ where // Parse the opening bracket of the sequence. if let Ok(Node::Array { len, count: _ }) = self.next() { // Give the visitor access to each element of the sequence. - visitor.visit_seq(CommaSeparated::new(self, len)) + visit_array(self, len, visitor) } else { Err(Deserializer::error(ErrorType::ExpectedArray)) } @@ -300,7 +300,7 @@ where // Parse the opening bracket of the sequence. if let Ok(Node::Object { len, count: _ }) = self.next() { // Give the visitor access to each element of the sequence. - visitor.visit_map(CommaSeparated::new(self, len)) + visit_object(self, len, visitor) } else { Err(Deserializer::error(ErrorType::ExpectedMap)) } @@ -318,8 +318,8 @@ where { match self.next() { // Give the visitor access to each element of the sequence. - Ok(Node::Object { len, count: _ }) => visitor.visit_map(CommaSeparated::new(self, len)), - Ok(Node::Array { len, count: _ }) => visitor.visit_seq(CommaSeparated::new(self, len)), + Ok(Node::Object { len, count: _ }) => visit_object(self, len, visitor), + Ok(Node::Array { len, count: _ }) => visit_array(self, len, visitor), _ => Err(Deserializer::error(ErrorType::ExpectedMap)), } } @@ -406,6 +406,37 @@ impl<'de> de::VariantAccess<'de> for VariantAccess<'_, 'de> { } } +/// Hands the `len` elements of an array to `visitor`. A visitor of fixed length (tuple, array, +/// tuple struct, struct from an array) stops asking for elements once it has enough; the elements +/// it left unread would stay on the tape and be read as the values that follow, so they are an +/// error, as in `serde_json` ("trailing characters") and `serde::de::value::SeqDeserializer`. +fn visit_array<'de, V>(de: &mut Deserializer<'de>, len: usize, visitor: V) -> Result +where + V: Visitor<'de>, +{ + let mut seq = CommaSeparated::new(de, len); + let value = stry!(visitor.visit_seq(&mut seq)); + if seq.len == 0 { + Ok(value) + } else { + Err(de::Error::invalid_length(len, &"fewer elements in array")) + } +} + +/// Like [`visit_array`], for the `len` members of an object. +fn visit_object<'de, V>(de: &mut Deserializer<'de>, len: usize, visitor: V) -> Result +where + V: Visitor<'de>, +{ + let mut map = CommaSeparated::new(de, len); + let value = stry!(visitor.visit_map(&mut map)); + if map.len == 0 { + Ok(value) + } else { + Err(de::Error::invalid_length(len, &"fewer elements in map")) + } +} + // In order to handle commas correctly when deserializing a JSON array or map, // we need to track whether we are on the first element or past the first // element. diff --git a/src/tests/serde.rs b/src/tests/serde.rs index 7d3feed7..dacd3973 100644 --- a/src/tests/serde.rs +++ b/src/tests/serde.rs @@ -1327,3 +1327,80 @@ fn value_tuple_variant_from_sequence() { assert!(from_borrowed_value::(b.clone()).is_err()); assert!(from_refborrowed_value::(&b).is_err()); } + +#[test] +fn fixed_length_sequences_reject_extra_elements() { + // A tuple, array or tuple struct reads only as many elements as it has. The rest of the + // array used to stay on the tape and be read as the values after it: `[[1,2,3],4]` as + // `((u8, u8), u8)` gave `((1, 2), 3)`. Extra elements are an error, as in serde_json. + #[derive(Deserialize, Debug, PartialEq)] + struct Pair(u8, u8); + #[derive(Deserialize, Debug, PartialEq)] + struct Named { + a: u8, + } + + let mut d = b"[[1,2,3],4]".to_vec(); + assert!(from_slice::<((u8, u8), u8)>(&mut d).is_err()); + let mut d = b"[1,[2,99,98],3]".to_vec(); + assert!(from_slice::<(u8, (u8,), u8)>(&mut d).is_err()); + let mut d = b"[[1,2,[9,9]],[4,5]]".to_vec(); + assert!(from_slice::>(&mut d).is_err()); + let mut d = b"[1,2,3]".to_vec(); + assert!(from_slice::<[u8; 2]>(&mut d).is_err()); + let mut d = b"[1,2,3]".to_vec(); + assert!(from_slice::(&mut d).is_err()); + let mut d = b"[1,2]".to_vec(); + assert!(from_slice::(&mut d).is_err()); + let mut d = br#"{"a":[1,2,3],"b":[4,5]}"#.to_vec(); + assert!(from_slice::>(&mut d).is_err()); + + // exact lengths, and sequences of any length, are unchanged + let mut d = b"[[1,2],3]".to_vec(); + assert_eq!(from_slice::<((u8, u8), u8)>(&mut d).ok(), Some(((1, 2), 3))); + let mut d = b"[1,2]".to_vec(); + assert_eq!(from_slice::(&mut d).ok(), Some(Pair(1, 2))); + let mut d = b"[1]".to_vec(); + assert_eq!(from_slice::(&mut d).ok(), Some(Named { a: 1 })); + let mut d = b"[[1,2,3],[4]]".to_vec(); + assert_eq!( + from_slice::>>(&mut d).ok(), + Some(vec![vec![1, 2, 3], vec![4]]) + ); + // fewer elements is still an error + let mut d = b"[1]".to_vec(); + assert!(from_slice::<(u8, u8)>(&mut d).is_err()); +} + +#[test] +fn tuple_variants_reject_extra_elements() { + // A tuple variant's array is a fixed-length sequence too, from text and from every Value. + use crate::serde::{ + from_borrowed_value, from_owned_value, from_refborrowed_value, from_refowned_value, + }; + #[derive(Deserialize, Debug, PartialEq)] + enum Enm { + Var(u8, u8), + } + let mut d = br#"{"Var":[1,2,3]}"#.to_vec(); + assert!(from_slice::(&mut d).is_err()); + let mut d = br#"[{"Var":[1,2,3]},{"Var":[4,5]}]"#.to_vec(); + assert!(from_slice::>(&mut d).is_err()); + + let mut d = br#"{"Var":[1,2,3]}"#.to_vec(); + let o = to_owned_value(&mut d).expect("valid"); + let mut d2 = br#"{"Var":[1,2,3]}"#.to_vec(); + let b = to_borrowed_value(&mut d2).expect("valid"); + assert!(from_owned_value::(o.clone()).is_err()); + assert!(from_refowned_value::(&o).is_err()); + assert!(from_borrowed_value::(b.clone()).is_err()); + assert!(from_refborrowed_value::(&b).is_err()); + + // exact length is unchanged + let mut d = br#"{"Var":[1,2]}"#.to_vec(); + assert_eq!(from_slice::(&mut d).ok(), Some(Enm::Var(1, 2))); + let mut d = br#"{"Var":[1,2]}"#.to_vec(); + let o = to_owned_value(&mut d).expect("valid"); + assert_eq!(from_refowned_value::(&o).ok(), Some(Enm::Var(1, 2))); + assert_eq!(from_owned_value::(o).ok(), Some(Enm::Var(1, 2))); +}