Skip to content

Commit cb44bda

Browse files
committed
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::<BTreeMap<i64, u8>>(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 integer parsing also used `str::parse`, which accepts `+1` and `01`; it now requires a JSON integer, as the text deserializer does (`is_json_integer`). Test: `value_map_integer_keys` (fails before this change). Found by a differential fuzzer (serde_json, simd-json, sonic-rs, jiter).
1 parent f25762b commit cb44bda

4 files changed

Lines changed: 80 additions & 7 deletions

File tree

‎src/serde/value/borrowed/de.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,8 @@ impl<'de> MapAccess<'de> for ObjectAccess<'de> {
230230
match self.i.next() {
231231
Some((k, v)) => {
232232
self.v = Some(v);
233-
seed.deserialize(Value::String(k)).map(Some)
233+
// through the key deserializer, as for `&Value`: integer keys parse
234+
seed.deserialize(MapKeyDeserializer::from_cow(k)).map(Some)
234235
}
235236
_ => Ok(None),
236237
}

‎src/serde/value/owned/de.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// A lot of this logic is a re-implementation or copy of serde_json::Value
22
use crate::ErrorType;
3+
use crate::cow::Cow;
34
use crate::{Error, ObjectHasher};
45
use crate::{
56
prelude::*,
@@ -217,7 +218,9 @@ impl<'de> MapAccess<'de> for ObjectAccess {
217218
match self.i.next() {
218219
Some((k, v)) => {
219220
self.v = Some(v);
220-
seed.deserialize(Value::String(k)).map(Some)
221+
// through the key deserializer, as for `&Value`: integer keys parse
222+
seed.deserialize(MapKeyDeserializer::from_cow(Cow::from(k)))
223+
.map(Some)
221224
}
222225
_ => Ok(None),
223226
}

‎src/serde/value/shared.rs‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,42 @@ use serde::de::{Unexpected, Visitor};
44
use serde::forward_to_deserialize_any;
55
use serde_ext::de::{self, IntoDeserializer};
66

7+
fn is_json_integer(s: &str) -> bool {
8+
let digits = s.strip_prefix('-').unwrap_or(s).as_bytes();
9+
match digits {
10+
[b'0'] => true,
11+
[b'1'..=b'9', rest @ ..] => rest.iter().all(u8::is_ascii_digit),
12+
_ => false,
13+
}
14+
}
15+
716
macro_rules! deserialize_integer_key {
817
($method:ident => $visit:ident) => {
918
fn $method<V>(self, visitor: V) -> Result<V::Value, Self::Error>
1019
where
1120
V: Visitor<'de>,
1221
{
13-
match (self.key.parse(), self.key) {
14-
(Ok(integer), _) => visitor.$visit(integer),
22+
// only a JSON integer (`-?(0|[1-9][0-9]*)`) is an integer key; `str::parse` alone also
23+
// takes `+1` and `01`
24+
let parsed = if is_json_integer(&self.key) {
25+
self.key.parse().ok()
26+
} else {
27+
None
28+
};
29+
match (parsed, self.key) {
30+
(Some(integer), _) => visitor.$visit(integer),
1531
#[cfg(feature = "beef")]
16-
(Err(_), s) => {
32+
(None, s) => {
1733
if s.is_borrowed() {
1834
visitor.visit_borrowed_str(s.unwrap_borrowed())
1935
} else {
2036
visitor.visit_string(s.into_owned())
2137
}
2238
}
2339
#[cfg(not(feature = "beef"))]
24-
(Err(_), Cow::Borrowed(s)) => visitor.visit_borrowed_str(s),
40+
(None, Cow::Borrowed(s)) => visitor.visit_borrowed_str(s),
2541
#[cfg(not(feature = "beef"))]
26-
(Err(_), Cow::Owned(s)) => visitor.visit_string(s),
42+
(None, Cow::Owned(s)) => visitor.visit_string(s),
2743
}
2844
}
2945
};
@@ -40,6 +56,9 @@ impl<'de> MapKeyDeserializer<'de> {
4056
key: Cow::from(key),
4157
}
4258
}
59+
pub(crate) fn from_cow(key: Cow<'de, str>) -> Self {
60+
Self { key }
61+
}
4362
}
4463

4564
impl<'de> serde::Deserializer<'de> for MapKeyDeserializer<'de> {

‎src/tests/serde.rs‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1374,3 +1374,53 @@ fn i128_map_keys() {
13741374
let mut d = br#"{"340282366920938463463374607431768211456":1}"#.to_vec();
13751375
assert!(from_slice::<BTreeMap<u128, u8>>(&mut d).is_err());
13761376
}
1377+
1378+
#[test]
1379+
fn value_map_integer_keys() {
1380+
// From an owned/borrowed Value (not a reference), map keys were deserialized as plain
1381+
// strings, so integer keys failed; through `&Value` they parsed, but also accepted `+1`, `01`.
1382+
use crate::serde::{
1383+
from_borrowed_value, from_owned_value, from_refborrowed_value, from_refowned_value,
1384+
};
1385+
use std::collections::BTreeMap;
1386+
let ok = BTreeMap::from([(1_i64, 2_u8), (-7, 3)]);
1387+
let mut d = br#"{"1":2,"-7":3}"#.to_vec();
1388+
let v = to_owned_value(&mut d).expect("valid");
1389+
assert_eq!(
1390+
from_owned_value::<BTreeMap<i64, u8>>(v.clone()).ok(),
1391+
Some(ok.clone())
1392+
);
1393+
assert_eq!(
1394+
from_refowned_value::<BTreeMap<i64, u8>>(&v).ok(),
1395+
Some(ok.clone())
1396+
);
1397+
let mut d = br#"{"1":2,"-7":3}"#.to_vec();
1398+
let v = to_borrowed_value(&mut d).expect("valid");
1399+
assert_eq!(
1400+
from_borrowed_value::<BTreeMap<i64, u8>>(v.clone()).ok(),
1401+
Some(ok.clone())
1402+
);
1403+
assert_eq!(
1404+
from_refborrowed_value::<BTreeMap<i64, u8>>(&v).ok(),
1405+
Some(ok)
1406+
);
1407+
for key in ["+1", "01", "1x", " 1"] {
1408+
let mut d = format!(r#"{{"{key}":2}}"#).into_bytes();
1409+
let v = to_owned_value(&mut d).expect("valid");
1410+
assert!(
1411+
from_owned_value::<BTreeMap<i64, u8>>(v.clone()).is_err(),
1412+
"{key:?}"
1413+
);
1414+
assert!(
1415+
from_refowned_value::<BTreeMap<i64, u8>>(&v).is_err(),
1416+
"{key:?}"
1417+
);
1418+
}
1419+
// string keys are unchanged
1420+
let mut d = br#"{"01":2}"#.to_vec();
1421+
let v = to_owned_value(&mut d).expect("valid");
1422+
assert_eq!(
1423+
from_owned_value::<BTreeMap<String, u8>>(v).ok(),
1424+
Some(BTreeMap::from([("01".to_string(), 2)]))
1425+
);
1426+
}

0 commit comments

Comments
 (0)