diff --git a/src/serde/value/borrowed/de.rs b/src/serde/value/borrowed/de.rs index d282df3b..7ff50749 100644 --- a/src/serde/value/borrowed/de.rs +++ b/src/serde/value/borrowed/de.rs @@ -51,7 +51,7 @@ impl<'de> de::Deserializer<'de> for Value<'de> { Cow::Owned(s) => visitor.visit_string(s), }, - Value::Array(a) => visitor.visit_seq(Array(a.into_iter())), + Value::Array(a) => visit_array(Array(a.into_iter()), visitor), Value::Object(o) => visitor.visit_map(ObjectAccess::new(o.into_iter())), } } @@ -126,7 +126,7 @@ impl<'de> de::Deserializer<'de> for Value<'de> { { match self { // Give the visitor access to each element of the sequence. - Value::Array(a) => visitor.visit_seq(Array(a.into_iter())), + Value::Array(a) => visit_array(Array(a.into_iter()), visitor), Value::Object(o) => visitor.visit_map(ObjectAccess::new(o.into_iter())), other => Err(crate::Deserializer::error(ErrorType::Unexpected( Some(ValueType::Object), @@ -142,7 +142,33 @@ impl<'de> de::Deserializer<'de> for Value<'de> { } } +/// Number of array elements not yet handed out. +trait Remaining { + fn remaining(&self) -> usize; +} + +/// Hands the array to `visitor`; elements a fixed-length visitor (tuple, array, tuple struct) did +/// not read are an error, as with the text deserializer and serde's `SeqDeserializer`. +fn visit_array<'de, A, V>(mut seq: A, visitor: V) -> Result +where + A: SeqAccess<'de, Error = Error> + Remaining, + V: Visitor<'de>, +{ + let len = seq.remaining(); + let value = visitor.visit_seq(&mut seq)?; + if seq.remaining() == 0 { + Ok(value) + } else { + Err(de::Error::invalid_length(len, &"fewer elements in array")) + } +} + struct Array<'de>(std::vec::IntoIter>); +impl Remaining for Array<'_> { + fn remaining(&self) -> usize { + self.0.len() + } +} // `SeqAccess` is provided to the `Visitor` to give it the ability to iterate // through elements of the sequence. @@ -160,6 +186,11 @@ impl<'de> SeqAccess<'de> for Array<'de> { } struct ArrayRef<'de>(std::slice::Iter<'de, Value<'de>>); +impl Remaining for ArrayRef<'_> { + fn remaining(&self) -> usize { + self.0.len() + } +} // `SeqAccess` is provided to the `Visitor` to give it the ability to iterate // through elements of the sequence. @@ -565,7 +596,7 @@ impl<'de> VariantAccess<'de> for VariantDeserializer<'de> { if v.is_empty() { visitor.visit_unit() } else { - visitor.visit_seq(Array(v.into_iter())) + visit_array(Array(v.into_iter()), visitor) } } Some(other) => Err(crate::Deserializer::error(ErrorType::Unexpected( @@ -589,6 +620,8 @@ impl<'de> VariantAccess<'de> for VariantDeserializer<'de> { { match self.value { Some(Value::Object(o)) => visitor.visit_map(ObjectAccess::new(o.into_iter())), + // a struct variant may be written as a sequence, as when deserializing from text + Some(Value::Array(a)) => visit_array(Array(a.into_iter()), visitor), Some(other) => Err(crate::Deserializer::error(ErrorType::Unexpected( Some(ValueType::Object), Some(other.value_type()), @@ -623,7 +656,7 @@ impl<'de> de::Deserializer<'de> for &'de Value<'de> { #[allow(clippy::useless_conversion)] // .into() required by ordered-float Value::Static(StaticNode::F64(n)) => visitor.visit_f64((*n).into()), Value::String(s) => visitor.visit_borrowed_str(s), - Value::Array(a) => visitor.visit_seq(ArrayRef(a.as_slice().iter())), + Value::Array(a) => visit_array(ArrayRef(a.as_slice().iter()), visitor), Value::Object(o) => visitor.visit_map(ObjectRefAccess::new(o.iter())), } } @@ -663,7 +696,7 @@ impl<'de> de::Deserializer<'de> for &'de Value<'de> { { match self { // Give the visitor access to each element of the sequence. - Value::Array(a) => visitor.visit_seq(ArrayRef(a.as_slice().iter())), + Value::Array(a) => visit_array(ArrayRef(a.as_slice().iter()), visitor), Value::Object(o) => visitor.visit_map(ObjectRefAccess::new(o.iter())), other => Err(crate::Deserializer::error(ErrorType::Unexpected( Some(ValueType::Object), @@ -778,7 +811,7 @@ impl<'de> VariantAccess<'de> for VariantRefDeserializer<'de> { if v.is_empty() { visitor.visit_unit() } else { - visitor.visit_seq(ArrayRef(v.as_slice().iter())) + visit_array(ArrayRef(v.as_slice().iter()), visitor) } } Some(other) => Err(crate::Deserializer::error(ErrorType::Unexpected( @@ -802,6 +835,8 @@ impl<'de> VariantAccess<'de> for VariantRefDeserializer<'de> { { match self.value { Some(Value::Object(o)) => visitor.visit_map(ObjectRefAccess::new(o.iter())), + // a struct variant may be written as a sequence, as when deserializing from text + Some(Value::Array(a)) => visit_array(ArrayRef(a.as_slice().iter()), visitor), Some(other) => Err(crate::Deserializer::error(ErrorType::Unexpected( Some(ValueType::Object), Some(other.value_type()), diff --git a/src/serde/value/owned/de.rs b/src/serde/value/owned/de.rs index ca92c759..10e907e7 100644 --- a/src/serde/value/owned/de.rs +++ b/src/serde/value/owned/de.rs @@ -37,7 +37,7 @@ impl<'de> de::Deserializer<'de> for Value { #[allow(clippy::useless_conversion)] // .into() required by ordered-float Value::Static(StaticNode::F64(n)) => visitor.visit_f64(n.into()), Value::String(s) => visitor.visit_string(s), - Value::Array(a) => visitor.visit_seq(Array(a.into_iter())), + Value::Array(a) => visit_array(Array(a.into_iter()), visitor), Value::Object(o) => visitor.visit_map(ObjectAccess { i: o.into_iter(), v: None, @@ -114,7 +114,7 @@ impl<'de> de::Deserializer<'de> for Value { { match self { // Give the visitor access to each element of the sequence. - Value::Array(a) => visitor.visit_seq(Array(a.into_iter())), + Value::Array(a) => visit_array(Array(a.into_iter()), visitor), Value::Object(o) => visitor.visit_map(ObjectAccess::new(o.into_iter())), other => Err(crate::Deserializer::error(ErrorType::Unexpected( Some(ValueType::Object), @@ -130,7 +130,33 @@ impl<'de> de::Deserializer<'de> for Value { } } +/// Number of array elements not yet handed out. +trait Remaining { + fn remaining(&self) -> usize; +} + +/// Hands the array to `visitor`; elements a fixed-length visitor (tuple, array, tuple struct) did +/// not read are an error, as with the text deserializer and serde's `SeqDeserializer`. +fn visit_array<'de, A, V>(mut seq: A, visitor: V) -> Result +where + A: SeqAccess<'de, Error = Error> + Remaining, + V: Visitor<'de>, +{ + let len = seq.remaining(); + let value = visitor.visit_seq(&mut seq)?; + if seq.remaining() == 0 { + Ok(value) + } else { + Err(de::Error::invalid_length(len, &"fewer elements in array")) + } +} + struct Array(std::vec::IntoIter); +impl Remaining for Array { + fn remaining(&self) -> usize { + self.0.len() + } +} // `SeqAccess` is provided to the `Visitor` to give it the ability to iterate // through elements of the sequence. @@ -148,6 +174,11 @@ impl<'de> SeqAccess<'de> for Array { } struct ArrayRef<'de>(std::slice::Iter<'de, Value>); +impl Remaining for ArrayRef<'_> { + fn remaining(&self) -> usize { + self.0.len() + } +} // `SeqAccess` is provided to the `Visitor` to give it the ability to iterate // through elements of the sequence. @@ -552,7 +583,7 @@ impl<'de> VariantAccess<'de> for VariantDeserializer { if v.is_empty() { visitor.visit_unit() } else { - visitor.visit_seq(Array(v.into_iter())) + visit_array(Array(v.into_iter()), visitor) } } Some(other) => Err(crate::Deserializer::error(ErrorType::Unexpected( @@ -576,6 +607,8 @@ impl<'de> VariantAccess<'de> for VariantDeserializer { { match self.value { Some(Value::Object(o)) => visitor.visit_map(ObjectAccess::new(o.into_iter())), + // a struct variant may be written as a sequence, as when deserializing from text + Some(Value::Array(a)) => visit_array(Array(a.into_iter()), visitor), Some(other) => Err(crate::Deserializer::error(ErrorType::Unexpected( Some(ValueType::Object), Some(other.value_type()), @@ -610,7 +643,7 @@ impl<'de> de::Deserializer<'de> for &'de Value { #[allow(clippy::useless_conversion)] // .into() required by ordered-float Value::Static(StaticNode::F64(n)) => visitor.visit_f64((*n).into()), Value::String(s) => visitor.visit_borrowed_str(s), - Value::Array(a) => visitor.visit_seq(ArrayRef(a.as_slice().iter())), + Value::Array(a) => visit_array(ArrayRef(a.as_slice().iter()), visitor), Value::Object(o) => visitor.visit_map(ObjectRefAccess::new(o.iter())), } } @@ -651,7 +684,7 @@ impl<'de> de::Deserializer<'de> for &'de Value { { match self { // Give the visitor access to each element of the sequence. - Value::Array(a) => visitor.visit_seq(ArrayRef(a.as_slice().iter())), + Value::Array(a) => visit_array(ArrayRef(a.as_slice().iter()), visitor), Value::Object(o) => visitor.visit_map(ObjectRefAccess::new(o.iter())), other => Err(crate::Deserializer::error(ErrorType::Unexpected( Some(ValueType::Object), @@ -755,7 +788,7 @@ impl<'de> VariantAccess<'de> for VariantRefDeserializer<'de> { if v.is_empty() { visitor.visit_unit() } else { - visitor.visit_seq(ArrayRef(v.as_slice().iter())) + visit_array(ArrayRef(v.as_slice().iter()), visitor) } } Some(other) => Err(crate::Deserializer::error(ErrorType::Unexpected( @@ -779,6 +812,8 @@ impl<'de> VariantAccess<'de> for VariantRefDeserializer<'de> { { match self.value { Some(Value::Object(o)) => visitor.visit_map(ObjectRefAccess::new(o.iter())), + // a struct variant may be written as a sequence, as when deserializing from text + Some(Value::Array(a)) => visit_array(ArrayRef(a.as_slice().iter()), visitor), Some(other) => Err(crate::Deserializer::error(ErrorType::Unexpected( Some(ValueType::Object), Some(other.value_type()), diff --git a/src/tests/serde.rs b/src/tests/serde.rs index e6a6e36f..7d3feed7 100644 --- a/src/tests/serde.rs +++ b/src/tests/serde.rs @@ -1212,3 +1212,118 @@ fn overdriven_next_key_seed_errors_instead_of_oob() { let mut input = br#"{"a":1}"#.to_vec(); assert!(from_slice::(&mut input).is_err()); } + +#[test] +fn value_fixed_length_sequences_reject_extra_elements() { + // From a Value, a tuple/array/struct visitor that stops early used to leave the rest of the + // array unread without error: `[1,2,3]` as `(u8, u8)` gave `(1, 2)`. + use crate::serde::{ + from_borrowed_value, from_owned_value, from_refborrowed_value, from_refowned_value, + }; + #[derive(Deserialize, Debug, PartialEq)] + struct Pair(u8, u8); + #[derive(Deserialize, Debug, PartialEq)] + struct Named { + a: u8, + } + let mut d = b"[1,2,3]".to_vec(); + let o = to_owned_value(&mut d).expect("valid"); + let mut d2 = b"[1,2,3]".to_vec(); + let b = to_borrowed_value(&mut d2).expect("valid"); + assert!(from_owned_value::<(u8, u8)>(o.clone()).is_err()); + assert!(from_refowned_value::<[u8; 2]>(&o).is_err()); + assert!(from_borrowed_value::(b.clone()).is_err()); + assert!(from_refborrowed_value::(&b).is_err()); + assert!(from_owned_value::(o.clone()).is_err()); + // exact length and Vec are unchanged; fewer elements still an error + assert_eq!(from_refowned_value::>(&o).ok(), Some(vec![1, 2, 3])); + assert_eq!( + from_owned_value::<(u8, u8, u8)>(o.clone()).ok(), + Some((1, 2, 3)) + ); + assert!(from_owned_value::<(u8, u8, u8, u8)>(o).is_err()); +} + +#[test] +fn value_struct_variant_from_sequence() { + // A struct variant written as a sequence deserializes from text; from a Value it failed with + // Unexpected(Object, Array). + use crate::serde::{ + from_borrowed_value, from_owned_value, from_refborrowed_value, from_refowned_value, + }; + #[derive(Deserialize, Debug, PartialEq)] + enum E { + S { a: u8, b: String }, + } + let expected = E::S { + a: 5, + b: "k".to_string(), + }; + let mut d = br#"{"S":[5,"k"]}"#.to_vec(); + assert_eq!( + from_slice::(&mut d).ok(), + Some(E::S { + a: 5, + b: "k".to_string() + }) + ); + let mut d = br#"{"S":[5,"k"]}"#.to_vec(); + let o = to_owned_value(&mut d).expect("valid"); + assert_eq!( + from_owned_value::(o.clone()).ok(), + Some(E::S { + a: 5, + b: "k".to_string() + }) + ); + assert_eq!( + from_refowned_value::(&o).ok(), + Some(E::S { + a: 5, + b: "k".to_string() + }) + ); + let mut d = br#"{"S":[5,"k"]}"#.to_vec(); + let b = to_borrowed_value(&mut d).expect("valid"); + assert_eq!( + from_borrowed_value::(b.clone()).ok(), + Some(E::S { + a: 5, + b: "k".to_string() + }) + ); + assert_eq!(from_refborrowed_value::(&b).ok(), Some(expected)); + for bad in [&br#"{"S":[5]}"#[..], br#"{"S":[5,"k",1]}"#] { + let mut d = bad.to_vec(); + let o = to_owned_value(&mut d).expect("valid"); + assert!(from_owned_value::(o.clone()).is_err()); + assert!(from_refowned_value::(&o).is_err()); + } +} + +#[test] +fn value_tuple_variant_from_sequence() { + // A tuple variant from a Value reads exactly its elements: extra elements are an error. + use crate::serde::{ + from_borrowed_value, from_owned_value, from_refborrowed_value, from_refowned_value, + }; + #[derive(Deserialize, Debug, PartialEq)] + enum E { + Tpl(u8, u8), + } + let mut d = br#"{"Tpl":[1,2]}"#.to_vec(); + assert_eq!(from_slice::(&mut d).ok(), Some(E::Tpl(1, 2))); + let mut d = br#"{"Tpl":[1,2]}"#.to_vec(); + let o = to_owned_value(&mut d).expect("valid"); + assert_eq!(from_refowned_value::(&o).ok(), Some(E::Tpl(1, 2))); + assert_eq!(from_owned_value::(o).ok(), Some(E::Tpl(1, 2))); + + let mut d = br#"{"Tpl":[1,2,3]}"#.to_vec(); + let o = to_owned_value(&mut d).expect("valid"); + let mut d2 = br#"{"Tpl":[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()); +}