From 701fcf101da6d0e9e1380a8e1c91cd64615c9bd7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 07:10:01 +0000 Subject: [PATCH 1/2] fix: reject extra array elements for fixed-length types A tuple, array, tuple struct or struct read from an array asks the `SeqAccess` for exactly as many elements as it has and then stops. The elements it left unread stayed on the tape and were read as the values that follow the array, so data silently shifted: from_slice::<((u8, u8), u8)>("[[1,2,3],4]") Ok(((1, 2), 3)) from_slice::<(u8, (u8,), u8)>("[1,[2,99,98],3]") Ok((1, (2,), 99)) from_slice::>("[[1,2,[9,9]],[4,5]]") Ok([(1, 2), (9, 9)]) and in other shapes failed with unrelated errors (`ExpectedArray`, `ExpectedString`). serde_json and sonic-rs reject all of these. (`from_owned_value` ignores the extra elements without shifting the following values; it is not changed here.) After the visitor returns, the remaining element count must be 0; otherwise it is an `invalid_length` error, as in `serde::de::value::SeqDeserializer::end` and serde_json's `visit_array`. The same check covers objects (a visitor that stops reading members early). Test: `fixed_length_sequences_reject_extra_elements` (fails before this change). Found by a three-way differential fuzzer (serde_json, simd-json, sonic-rs). --- src/serde/de.rs | 43 +++++++++++++++++++++++++++++++++++++------ src/tests/serde.rs | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 6 deletions(-) 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..d45dd5cc 100644 --- a/src/tests/serde.rs +++ b/src/tests/serde.rs @@ -1327,3 +1327,47 @@ 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()); +} From 7ce11714e7545f188aa3d8eb375ed711624d9aaa Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 07:51:39 +0000 Subject: [PATCH 2/2] test: tuple variants reject extra elements from text and every Value --- src/tests/serde.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/tests/serde.rs b/src/tests/serde.rs index d45dd5cc..dacd3973 100644 --- a/src/tests/serde.rs +++ b/src/tests/serde.rs @@ -1371,3 +1371,36 @@ fn fixed_length_sequences_reject_extra_elements() { 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))); +}