Integer map keys: JSON integer syntax, i128/u128, and from a Value

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<T: std::str::FromStr>(key: &str) -> Option<T> {
+    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<u16, i8>);
         ser_deser_map!(3_u32 => 3_i8, HashMap<u32, i8>);
         ser_deser_map!(3_u64 => 3_i8, HashMap<u64, i8>);
-        #[cfg(feature = "128bit")]
-        {
-            ser_deser_map!(3_i128 => 3_i8, HashMap<i128, i8>);
-            ser_deser_map!(3_u128 => 3_i8, HashMap<u128, i8>);
-        }
+        ser_deser_map!(3_i128 => 3_i8, HashMap<i128, i8>);
+        ser_deser_map!(3_u128 => 3_i8, HashMap<u128, i8>);
         ser_deser_map!(NewTypeStruct(1) => 3_i8, HashMap<NewTypeStruct, i8>);
         ser_deser_map!(E::UnitVariant => 3_i8, HashMap<E, i8>);
     }

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)]

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)
         }

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)
         }

src/serde/value/shared.rs

@@ -10,20 +10,20 @@ 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 {
                         visitor.visit_string(s.into_owned())
                     }
                 }
                 #[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<Cow<'de, str>>) -> 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)]

src/tests/serde.rs

@@ -1404,3 +1404,91 @@ fn tuple_variants_reject_extra_elements() {
     assert_eq!(from_refowned_value::<Enm>(&o).ok(), Some(Enm::Var(1, 2)));
     assert_eq!(from_owned_value::<Enm>(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<T>(json: &str) -> Vec<(&'static str, Option<T>)>
+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::<T>(&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::<T>(&o).ok()),
+        ("from_owned_value", from_owned_value::<T>(o).ok()),
+        (
+            "from_refborrowed_value",
+            from_refborrowed_value::<T>(&b).ok(),
+        ),
+        ("from_borrowed_value", from_borrowed_value::<T>(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::<BTreeMap<i64, u8>>(&json) {
+            assert_eq!(r, None, "i64 key {key:?} via {path}");
+        }
+        for (path, r) in map_key_results::<BTreeMap<u32, u8>>(&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::<BTreeMap<i64, u8>>(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::<BTreeMap<String, u8>>(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::<BTreeMap<i128, u8>>(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::<BTreeMap<u128, u8>>(r#"{"340282366920938463463374607431768211455":1}"#)
+    {
+        assert_eq!(r.as_ref(), Some(&ok), "via {path}");
+    }
+    for (path, r) in
+        map_key_results::<BTreeMap<u128, u8>>(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::<BTreeMap<i64, u8>>(r#"{"1":2,"-7":3}"#) {
+        assert_eq!(r.as_ref(), Some(&ok), "via {path}");
+    }
+}