diff --git a/src/serde.rs b/src/serde.rs index a08000d9..de653d3a 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. /// @@ -1002,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 ffdc24d2..9ca0b9a1 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)), })) } @@ -555,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/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 690a47f9..edf21c8d 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), } } }; @@ -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() } } } @@ -56,13 +54,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 dacd3973..3d01eb42 100644 --- a/src/tests/serde.rs +++ b/src/tests/serde.rs @@ -1404,3 +1404,91 @@ 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}"#) { + 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}"); + } +} + +#[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, 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) { + assert_eq!(r.as_ref(), Some(&ok), "via {path}"); + } + let ok = BTreeMap::from([(u128::MAX, 1_u8)]); + for (path, r) in + map_key_results::>(r#"{"340282366920938463463374607431768211455":1}"#) + { + assert_eq!(r.as_ref(), Some(&ok), "via {path}"); + } + 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}"); + } +}