From 49f29289b75dc5acf44041cf015185392aa8e408 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 03:11:27 +0000 Subject: [PATCH 1/3] fix: integer map keys must be JSON integers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integer map keys (i8..u64, and i128/u128 with `128bit`) were parsed with `str::parse`, which also accepts a leading `+` and leading zeros: from_slice::>(r#"{"+1":2}"#) Ok({1: 2}) from_slice::>(r#"{"01":2}"#) Ok({1: 2}) Neither is JSON number syntax (RFC 8259 ยง6), and serde_json and sonic-rs reject both as integer keys. `-0` is rejected as well: as a number it is the key `0`, as a string it isn't, so accepting it would let `"0"` and `"-0"` collide. One function, `serde::parse_integer_key`, now does this for both the text deserializer (`MapKey`) and the Value one (`MapKeyDeserializer`): it checks the leading characters and leaves the rest to `str::parse`. Test: `integer_keys_follow_json_number_syntax`, through `from_slice` and the Value functions (fails before this change). Found by a three-way differential fuzzer (serde_json, simd-json, sonic-rs). --- src/serde.rs | 11 ++++++++ src/serde/de.rs | 5 ++-- src/serde/value/shared.rs | 10 +++---- src/tests/serde.rs | 56 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 8 deletions(-) diff --git a/src/serde.rs b/src/serde.rs index a08000d9..014d4325 100644 --- a/src/serde.rs +++ b/src/serde.rs @@ -46,6 +46,17 @@ impl std::fmt::Display for SerdeConversionError { impl std::error::Error for SerdeConversionError {} +/// Parses an integer map key. A key is an integer only when written as a JSON integer +/// (`-?(0|[1-9][0-9]*)`), so `+1` and `01` (which `str::parse` alone accepts) are not; `-0` is +/// rejected too, since it would be the same number as the key `0` but a different string. +/// The leading characters are checked here, the rest by `str::parse`. +pub(crate) fn parse_integer_key(key: &str) -> Option { + match key.as_bytes() { + [b'0'] | [b'1'..=b'9', ..] | [b'-', b'1'..=b'9', ..] => key.parse().ok(), + _ => None, + } +} + /// parses a byte slice using a serde deserializer. /// note that the slice will be rewritten in the process. /// diff --git a/src/serde/de.rs b/src/serde/de.rs index ffdc24d2..36467ca4 100644 --- a/src/serde/de.rs +++ b/src/serde/de.rs @@ -523,9 +523,8 @@ macro_rules! deserialize_integer_key { V: de::Visitor<'de>, { visitor.$visit(stry!(match stry!(self.de.next()) { - Node::String(s) => s - .parse::<$type>() - .map_err(|_| Deserializer::error(ErrorType::InvalidNumber)), + Node::String(s) => crate::serde::parse_integer_key::<$type>(s) + .ok_or_else(|| Deserializer::error(ErrorType::InvalidNumber)), _ => Err(Deserializer::error(ErrorType::ExpectedString)), })) } diff --git a/src/serde/value/shared.rs b/src/serde/value/shared.rs index 690a47f9..9314a7af 100644 --- a/src/serde/value/shared.rs +++ b/src/serde/value/shared.rs @@ -10,10 +10,10 @@ macro_rules! deserialize_integer_key { where V: Visitor<'de>, { - match (self.key.parse(), self.key) { - (Ok(integer), _) => visitor.$visit(integer), + match (crate::serde::parse_integer_key(&self.key), self.key) { + (Some(integer), _) => visitor.$visit(integer), #[cfg(feature = "beef")] - (Err(_), s) => { + (None, s) => { if s.is_borrowed() { visitor.visit_borrowed_str(s.unwrap_borrowed()) } else { @@ -21,9 +21,9 @@ macro_rules! deserialize_integer_key { } } #[cfg(not(feature = "beef"))] - (Err(_), Cow::Borrowed(s)) => visitor.visit_borrowed_str(s), + (None, Cow::Borrowed(s)) => visitor.visit_borrowed_str(s), #[cfg(not(feature = "beef"))] - (Err(_), Cow::Owned(s)) => visitor.visit_string(s), + (None, Cow::Owned(s)) => visitor.visit_string(s), } } }; diff --git a/src/tests/serde.rs b/src/tests/serde.rs index dacd3973..3d673e1a 100644 --- a/src/tests/serde.rs +++ b/src/tests/serde.rs @@ -1404,3 +1404,59 @@ fn tuple_variants_reject_extra_elements() { assert_eq!(from_refowned_value::(&o).ok(), Some(Enm::Var(1, 2))); assert_eq!(from_owned_value::(o).ok(), Some(Enm::Var(1, 2))); } + +/// Deserializes the JSON object `json` as `T` through every path: `from_slice` and the four +/// Value functions. Returns one result per path, labelled. +fn map_key_results(json: &str) -> Vec<(&'static str, Option)> +where + T: serde::de::DeserializeOwned, +{ + use crate::serde::{ + from_borrowed_value, from_owned_value, from_refborrowed_value, from_refowned_value, + }; + let mut d = json.as_bytes().to_vec(); + let text = from_slice::(&mut d).ok(); + let mut d = json.as_bytes().to_vec(); + let o = to_owned_value(&mut d).expect("valid"); + let mut d2 = json.as_bytes().to_vec(); + let b = to_borrowed_value(&mut d2).expect("valid"); + vec![ + ("from_slice", text), + ("from_refowned_value", from_refowned_value::(&o).ok()), + ("from_owned_value", from_owned_value::(o).ok()), + ( + "from_refborrowed_value", + from_refborrowed_value::(&b).ok(), + ), + ("from_borrowed_value", from_borrowed_value::(b).ok()), + ] +} + +#[test] +fn integer_keys_follow_json_number_syntax() { + // Integer map keys used `str::parse`, which accepts `+1` and leading zeros. `-0` is + // rejected too: as a number it is the key `0`, as a string it isn't. + use std::collections::BTreeMap; + for key in [ + "+1", "01", "-01", "00", "+0", "-0", " 1", "1 ", "", "1x", "-", + ] { + let json = format!(r#"{{"{key}":2}}"#); + for (path, r) in map_key_results::>(&json) { + assert_eq!(r, None, "i64 key {key:?} via {path}"); + } + for (path, r) in map_key_results::>(&json) { + assert_eq!(r, None, "u32 key {key:?} via {path}"); + } + } + let ok = BTreeMap::from([(0_i64, 1_u8), (-7, 2), (120, 3)]); + for (path, r) in map_key_results::>(r#"{"0":1,"-7":2,"120":3}"#) { + if path == "from_slice" || path.starts_with("from_ref") { + assert_eq!(r.as_ref(), Some(&ok), "via {path}"); + } + } + // string keys are unchanged + let ok = BTreeMap::from([("01".to_string(), 2_u8), ("-0".to_string(), 3)]); + for (path, r) in map_key_results::>(r#"{"01":2,"-0":3}"#) { + assert_eq!(r.as_ref(), Some(&ok), "via {path}"); + } +} From d7c46bfdc6051a9707f05e155c6cf27bafe8e7b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 03:12:30 +0000 Subject: [PATCH 2/3] fix: i128/u128 map keys without the 128bit feature `i128` and `u128` values deserialize in the default build (through `as_i128`/`as_u128`), but the map-key implementations were behind `#[cfg(feature = "128bit")]`, in the text deserializer (`MapKey`) and in the Value one (`MapKeyDeserializer`), so without the feature every 128-bit key fell back to serde's default and failed: from_slice::>(r#"{"7":1}"#) Err(Serde("i128 is not supported")) Keys are parsed from the key text, so they need no tape support for 128-bit numbers; the cfg is dropped in both places. The 128-bit key round trip in `serde::test::maps` (also behind the feature, so the default run never exercised it) now runs in every build. serde_json and sonic-rs accept these keys. Test: `i128_map_keys`, through `from_slice` and the Value functions (fails before this change). Found by a three-way differential fuzzer (serde_json, simd-json, sonic-rs). --- src/serde.rs | 7 ++----- src/serde/de.rs | 3 +-- src/serde/value/shared.rs | 3 +-- src/tests/serde.rs | 25 +++++++++++++++++++++++++ 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/serde.rs b/src/serde.rs index 014d4325..de653d3a 100644 --- a/src/serde.rs +++ b/src/serde.rs @@ -1013,11 +1013,8 @@ mod test { ser_deser_map!(3_u16 => 3_i8, HashMap); ser_deser_map!(3_u32 => 3_i8, HashMap); ser_deser_map!(3_u64 => 3_i8, HashMap); - #[cfg(feature = "128bit")] - { - ser_deser_map!(3_i128 => 3_i8, HashMap); - ser_deser_map!(3_u128 => 3_i8, HashMap); - } + ser_deser_map!(3_i128 => 3_i8, HashMap); + ser_deser_map!(3_u128 => 3_i8, HashMap); ser_deser_map!(NewTypeStruct(1) => 3_i8, HashMap); ser_deser_map!(E::UnitVariant => 3_i8, HashMap); } diff --git a/src/serde/de.rs b/src/serde/de.rs index 36467ca4..9ca0b9a1 100644 --- a/src/serde/de.rs +++ b/src/serde/de.rs @@ -554,9 +554,8 @@ impl<'de> de::Deserializer<'de> for MapKey<'de, '_> { deserialize_integer_key!(deserialize_u32 => visit_u32; u32); deserialize_integer_key!(deserialize_u64 => visit_u64; u64); - #[cfg(feature = "128bit")] + // Like i128/u128 values, keys don't need the `128bit` feature: the key text is parsed directly. deserialize_integer_key!(deserialize_i128 => visit_i128; i128); - #[cfg(feature = "128bit")] deserialize_integer_key!(deserialize_u128 => visit_u128; u128); #[cfg_attr(not(feature = "no-inline"), inline)] diff --git a/src/serde/value/shared.rs b/src/serde/value/shared.rs index 9314a7af..83d17aa6 100644 --- a/src/serde/value/shared.rs +++ b/src/serde/value/shared.rs @@ -56,13 +56,12 @@ impl<'de> serde::Deserializer<'de> for MapKeyDeserializer<'de> { deserialize_integer_key!(deserialize_i16 => visit_i16); deserialize_integer_key!(deserialize_i32 => visit_i32); deserialize_integer_key!(deserialize_i64 => visit_i64); - #[cfg(feature = "128bit")] + // as for keys parsed from text, i128/u128 keys don't need the `128bit` feature deserialize_integer_key!(deserialize_i128 => visit_i128); deserialize_integer_key!(deserialize_u8 => visit_u8); deserialize_integer_key!(deserialize_u16 => visit_u16); deserialize_integer_key!(deserialize_u32 => visit_u32); deserialize_integer_key!(deserialize_u64 => visit_u64); - #[cfg(feature = "128bit")] deserialize_integer_key!(deserialize_u128 => visit_u128); #[cfg_attr(not(feature = "no-inline"), inline)] diff --git a/src/tests/serde.rs b/src/tests/serde.rs index 3d673e1a..85da4b97 100644 --- a/src/tests/serde.rs +++ b/src/tests/serde.rs @@ -1460,3 +1460,28 @@ fn integer_keys_follow_json_number_syntax() { assert_eq!(r.as_ref(), Some(&ok), "via {path}"); } } + +#[test] +fn i128_map_keys() { + // i128/u128 values deserialize without the `128bit` feature; keys used to fail with + // "i128 is not supported" unless it was enabled. + use std::collections::BTreeMap; + let ok = BTreeMap::from([(i128::MIN, 1_u8), (7, 2)]); + let json = r#"{"-170141183460469231731687303715884105728":1,"7":2}"#; + for (path, r) in map_key_results::>(json) { + if path == "from_slice" || path.starts_with("from_ref") { + assert_eq!(r.as_ref(), Some(&ok), "via {path}"); + } + } + let ok = BTreeMap::from([(u128::MAX, 1_u8)]); + let json = r#"{"340282366920938463463374607431768211455":1}"#; + for (path, r) in map_key_results::>(json) { + if path == "from_slice" || path.starts_with("from_ref") { + assert_eq!(r.as_ref(), Some(&ok), "via {path}"); + } + } + let json = r#"{"340282366920938463463374607431768211456":1}"#; + for (path, r) in map_key_results::>(json) { + assert_eq!(r, None, "u128 overflow via {path}"); + } +} From 2560d0bf0da70192905b2bc26b6503a10916ddd6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 03:13:35 +0000 Subject: [PATCH 3/3] fix: integer map keys when deserializing from a Value Deserializing from an owned or borrowed Value by value (`from_owned_value`, `from_borrowed_value`, `Value::deserialize_*`) passed each object key as `Value::String`, so integer keys never parsed: let v = to_owned_value(br#"{"1":2}"#)?; from_owned_value::>(v) Err(invalid type: string "1", expected i64) The by-reference path (`from_refowned_value`) and the text path accept it, and so do serde_json and sonic-rs. Keys now go through `MapKeyDeserializer` in all four Value paths; its constructor takes `impl Into>`, so one `new` serves borrowed and owned keys. Test: `value_map_integer_keys`; `integer_keys_follow_json_number_syntax` and `i128_map_keys` now also check the by-value paths (all through the same `map_key_results` helper: `from_slice` and the four Value functions). Fails before this change. Found by a differential fuzzer (serde_json, simd-json, sonic-rs, jiter). --- src/serde/value/borrowed/de.rs | 5 +++-- src/serde/value/owned/de.rs | 6 ++++-- src/serde/value/shared.rs | 6 ++---- src/tests/serde.rs | 35 ++++++++++++++++++++-------------- 4 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/serde/value/borrowed/de.rs b/src/serde/value/borrowed/de.rs index 7ff50749..dabcdd5e 100644 --- a/src/serde/value/borrowed/de.rs +++ b/src/serde/value/borrowed/de.rs @@ -230,7 +230,8 @@ impl<'de> MapAccess<'de> for ObjectAccess<'de> { match self.i.next() { Some((k, v)) => { self.v = Some(v); - seed.deserialize(Value::String(k)).map(Some) + // through the key deserializer, as for `&Value`: integer keys parse + seed.deserialize(MapKeyDeserializer::new(k)).map(Some) } _ => Ok(None), } @@ -269,7 +270,7 @@ impl<'de> MapAccess<'de> for ObjectRefAccess<'de> { if let Some((k, v)) = self.i.next() { self.v = Some(v); let s: &str = k; - seed.deserialize(MapKeyDeserializer::borrowed(s)).map(Some) + seed.deserialize(MapKeyDeserializer::new(s)).map(Some) } else { Ok(None) } diff --git a/src/serde/value/owned/de.rs b/src/serde/value/owned/de.rs index 10e907e7..5ae755c6 100644 --- a/src/serde/value/owned/de.rs +++ b/src/serde/value/owned/de.rs @@ -217,7 +217,8 @@ impl<'de> MapAccess<'de> for ObjectAccess { match self.i.next() { Some((k, v)) => { self.v = Some(v); - seed.deserialize(Value::String(k)).map(Some) + // through the key deserializer, as for `&Value`: integer keys parse + seed.deserialize(MapKeyDeserializer::new(k)).map(Some) } _ => Ok(None), } @@ -256,7 +257,8 @@ impl<'de> MapAccess<'de> for ObjectRefAccess<'de> { { if let Some((k, v)) = self.i.next() { self.v = Some(v); - seed.deserialize(MapKeyDeserializer::borrowed(k)).map(Some) + seed.deserialize(MapKeyDeserializer::new(k.as_str())) + .map(Some) } else { Ok(None) } diff --git a/src/serde/value/shared.rs b/src/serde/value/shared.rs index 83d17aa6..edf21c8d 100644 --- a/src/serde/value/shared.rs +++ b/src/serde/value/shared.rs @@ -35,10 +35,8 @@ pub(crate) struct MapKeyDeserializer<'de> { } impl<'de> MapKeyDeserializer<'de> { - pub(crate) fn borrowed(key: &'de str) -> Self { - Self { - key: Cow::from(key), - } + pub(crate) fn new(key: impl Into>) -> Self { + Self { key: key.into() } } } diff --git a/src/tests/serde.rs b/src/tests/serde.rs index 85da4b97..3d01eb42 100644 --- a/src/tests/serde.rs +++ b/src/tests/serde.rs @@ -1450,9 +1450,7 @@ fn integer_keys_follow_json_number_syntax() { } let ok = BTreeMap::from([(0_i64, 1_u8), (-7, 2), (120, 3)]); for (path, r) in map_key_results::>(r#"{"0":1,"-7":2,"120":3}"#) { - if path == "from_slice" || path.starts_with("from_ref") { - assert_eq!(r.as_ref(), Some(&ok), "via {path}"); - } + assert_eq!(r.as_ref(), Some(&ok), "via {path}"); } // string keys are unchanged let ok = BTreeMap::from([("01".to_string(), 2_u8), ("-0".to_string(), 3)]); @@ -1464,24 +1462,33 @@ fn integer_keys_follow_json_number_syntax() { #[test] fn i128_map_keys() { // i128/u128 values deserialize without the `128bit` feature; keys used to fail with - // "i128 is not supported" unless it was enabled. + // "i128 is not supported" unless it was enabled, from text and from every Value. use std::collections::BTreeMap; let ok = BTreeMap::from([(i128::MIN, 1_u8), (7, 2)]); let json = r#"{"-170141183460469231731687303715884105728":1,"7":2}"#; for (path, r) in map_key_results::>(json) { - if path == "from_slice" || path.starts_with("from_ref") { - assert_eq!(r.as_ref(), Some(&ok), "via {path}"); - } + assert_eq!(r.as_ref(), Some(&ok), "via {path}"); } let ok = BTreeMap::from([(u128::MAX, 1_u8)]); - let json = r#"{"340282366920938463463374607431768211455":1}"#; - for (path, r) in map_key_results::>(json) { - if path == "from_slice" || path.starts_with("from_ref") { - assert_eq!(r.as_ref(), Some(&ok), "via {path}"); - } + for (path, r) in + map_key_results::>(r#"{"340282366920938463463374607431768211455":1}"#) + { + assert_eq!(r.as_ref(), Some(&ok), "via {path}"); } - let json = r#"{"340282366920938463463374607431768211456":1}"#; - for (path, r) in map_key_results::>(json) { + for (path, r) in + map_key_results::>(r#"{"340282366920938463463374607431768211456":1}"#) + { assert_eq!(r, None, "u128 overflow via {path}"); } } + +#[test] +fn value_map_integer_keys() { + // From an owned/borrowed Value (not a reference), map keys were deserialized as plain + // strings, so integer keys failed; all four Value functions must parse them. + use std::collections::BTreeMap; + let ok = BTreeMap::from([(1_i64, 2_u8), (-7, 3)]); + for (path, r) in map_key_results::>(r#"{"1":2,"-7":3}"#) { + assert_eq!(r.as_ref(), Some(&ok), "via {path}"); + } +}