From e0953ea88f54830ec2f195d13240d6aeda3b261c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 09:43:46 +0000 Subject: [PATCH 1/3] fix: reject extra array elements when deserializing from a Value The Value deserializers (`from_owned_value`, `from_borrowed_value` and the `&Value` variants) handed arrays to the visitor without checking afterwards that every element was consumed, so a fixed-length type silently dropped the rest: let v = to_owned_value(b"[1,2,3]")?; from_owned_value::<(u8, u8)>(v) Ok((1, 2)) from_refowned_value::<[u8; 2]>(&v) Ok([1, 2]) The text deserializer rejects these (`fix/seq-extra-elements`), as do serde_json's `from_value` and sonic-rs. Arrays now go through `visit_array`, which returns `invalid_length` when elements are left, like serde's `SeqDeserializer::end`. Test: `value_fixed_length_sequences_reject_extra_elements` (fails before this change). Found by a differential fuzzer (serde_json, simd-json, sonic-rs, jiter). --- src/serde/value/borrowed/de.rs | 43 +++++++++++++++++++++++++++++----- src/serde/value/owned/de.rs | 43 +++++++++++++++++++++++++++++----- src/tests/serde.rs | 31 ++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 12 deletions(-) diff --git a/src/serde/value/borrowed/de.rs b/src/serde/value/borrowed/de.rs index d282df3b..9b045147 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( @@ -623,7 +654,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 +694,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 +809,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( diff --git a/src/serde/value/owned/de.rs b/src/serde/value/owned/de.rs index ca92c759..1e015d54 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( @@ -610,7 +641,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 +682,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 +786,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( diff --git a/src/tests/serde.rs b/src/tests/serde.rs index e6a6e36f..3a7ed7c4 100644 --- a/src/tests/serde.rs +++ b/src/tests/serde.rs @@ -1212,3 +1212,34 @@ 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()); +} From 33509719998c9df6ca5a0243eb8fad184b512839 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 09:52:34 +0000 Subject: [PATCH 2/3] fix: deserialize struct variants from a sequence in Value A struct variant written as a JSON array deserializes from text, but from an owned or borrowed Value (`from_owned_value` etc.) it failed with `Unexpected(Some(Object), Some(Array))`: from_slice::(br#"{"S":[5,"k"]}"#) Ok(S { a: 5, .. }) from_owned_value::(to_owned_value(..)?) Err(Unexpected(..)) `struct_variant` now hands an array to the visitor through `visit_array`, which also rejects wrong lengths. serde_json (#1049) and sonic-rs had the same gap. Builds on `fix/value-seq-extra-elements` (uses its `visit_array`). Test: `value_struct_variant_from_sequence` (fails before this change). Found by a differential fuzzer (serde_json, simd-json, sonic-rs, jiter). --- src/serde/value/borrowed/de.rs | 4 +++ src/serde/value/owned/de.rs | 4 +++ src/tests/serde.rs | 57 ++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+) diff --git a/src/serde/value/borrowed/de.rs b/src/serde/value/borrowed/de.rs index 9b045147..7ff50749 100644 --- a/src/serde/value/borrowed/de.rs +++ b/src/serde/value/borrowed/de.rs @@ -620,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()), @@ -833,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 1e015d54..10e907e7 100644 --- a/src/serde/value/owned/de.rs +++ b/src/serde/value/owned/de.rs @@ -607,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()), @@ -810,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 3a7ed7c4..25e8fed1 100644 --- a/src/tests/serde.rs +++ b/src/tests/serde.rs @@ -1243,3 +1243,60 @@ fn value_fixed_length_sequences_reject_extra_elements() { ); 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()); + } +} From cf629f734c5a150810afaeaa7413aae076098506 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 07:52:32 +0000 Subject: [PATCH 3/3] test: tuple variants from a Value reject extra elements --- src/tests/serde.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/tests/serde.rs b/src/tests/serde.rs index 25e8fed1..7d3feed7 100644 --- a/src/tests/serde.rs +++ b/src/tests/serde.rs @@ -1300,3 +1300,30 @@ fn value_struct_variant_from_sequence() { 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()); +}