Skip to content

Commit 2560d0b

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 constructor takes `impl Into<Cow<str>>`, 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).
1 parent d7c46bf commit 2560d0b

4 files changed

Lines changed: 30 additions & 22 deletions

File tree

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

Lines changed: 3 additions & 2 deletions
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::new(k)).map(Some)
234235
}
235236
_ => Ok(None),
236237
}
@@ -269,7 +270,7 @@ impl<'de> MapAccess<'de> for ObjectRefAccess<'de> {
269270
if let Some((k, v)) = self.i.next() {
270271
self.v = Some(v);
271272
let s: &str = k;
272-
seed.deserialize(MapKeyDeserializer::borrowed(s)).map(Some)
273+
seed.deserialize(MapKeyDeserializer::new(s)).map(Some)
273274
} else {
274275
Ok(None)
275276
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,8 @@ impl<'de> MapAccess<'de> for ObjectAccess {
217217
match self.i.next() {
218218
Some((k, v)) => {
219219
self.v = Some(v);
220-
seed.deserialize(Value::String(k)).map(Some)
220+
// through the key deserializer, as for `&Value`: integer keys parse
221+
seed.deserialize(MapKeyDeserializer::new(k)).map(Some)
221222
}
222223
_ => Ok(None),
223224
}
@@ -256,7 +257,8 @@ impl<'de> MapAccess<'de> for ObjectRefAccess<'de> {
256257
{
257258
if let Some((k, v)) = self.i.next() {
258259
self.v = Some(v);
259-
seed.deserialize(MapKeyDeserializer::borrowed(k)).map(Some)
260+
seed.deserialize(MapKeyDeserializer::new(k.as_str()))
261+
.map(Some)
260262
} else {
261263
Ok(None)
262264
}

‎src/serde/value/shared.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,8 @@ pub(crate) struct MapKeyDeserializer<'de> {
3535
}
3636

3737
impl<'de> MapKeyDeserializer<'de> {
38-
pub(crate) fn borrowed(key: &'de str) -> Self {
39-
Self {
40-
key: Cow::from(key),
41-
}
38+
pub(crate) fn new(key: impl Into<Cow<'de, str>>) -> Self {
39+
Self { key: key.into() }
4240
}
4341
}
4442

‎src/tests/serde.rs‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1450,9 +1450,7 @@ fn integer_keys_follow_json_number_syntax() {
14501450
}
14511451
let ok = BTreeMap::from([(0_i64, 1_u8), (-7, 2), (120, 3)]);
14521452
for (path, r) in map_key_results::<BTreeMap<i64, u8>>(r#"{"0":1,"-7":2,"120":3}"#) {
1453-
if path == "from_slice" || path.starts_with("from_ref") {
1454-
assert_eq!(r.as_ref(), Some(&ok), "via {path}");
1455-
}
1453+
assert_eq!(r.as_ref(), Some(&ok), "via {path}");
14561454
}
14571455
// string keys are unchanged
14581456
let ok = BTreeMap::from([("01".to_string(), 2_u8), ("-0".to_string(), 3)]);
@@ -1464,24 +1462,33 @@ fn integer_keys_follow_json_number_syntax() {
14641462
#[test]
14651463
fn i128_map_keys() {
14661464
// i128/u128 values deserialize without the `128bit` feature; keys used to fail with
1467-
// "i128 is not supported" unless it was enabled.
1465+
// "i128 is not supported" unless it was enabled, from text and from every Value.
14681466
use std::collections::BTreeMap;
14691467
let ok = BTreeMap::from([(i128::MIN, 1_u8), (7, 2)]);
14701468
let json = r#"{"-170141183460469231731687303715884105728":1,"7":2}"#;
14711469
for (path, r) in map_key_results::<BTreeMap<i128, u8>>(json) {
1472-
if path == "from_slice" || path.starts_with("from_ref") {
1473-
assert_eq!(r.as_ref(), Some(&ok), "via {path}");
1474-
}
1470+
assert_eq!(r.as_ref(), Some(&ok), "via {path}");
14751471
}
14761472
let ok = BTreeMap::from([(u128::MAX, 1_u8)]);
1477-
let json = r#"{"340282366920938463463374607431768211455":1}"#;
1478-
for (path, r) in map_key_results::<BTreeMap<u128, u8>>(json) {
1479-
if path == "from_slice" || path.starts_with("from_ref") {
1480-
assert_eq!(r.as_ref(), Some(&ok), "via {path}");
1481-
}
1473+
for (path, r) in
1474+
map_key_results::<BTreeMap<u128, u8>>(r#"{"340282366920938463463374607431768211455":1}"#)
1475+
{
1476+
assert_eq!(r.as_ref(), Some(&ok), "via {path}");
14821477
}
1483-
let json = r#"{"340282366920938463463374607431768211456":1}"#;
1484-
for (path, r) in map_key_results::<BTreeMap<u128, u8>>(json) {
1478+
for (path, r) in
1479+
map_key_results::<BTreeMap<u128, u8>>(r#"{"340282366920938463463374607431768211456":1}"#)
1480+
{
14851481
assert_eq!(r, None, "u128 overflow via {path}");
14861482
}
14871483
}
1484+
1485+
#[test]
1486+
fn value_map_integer_keys() {
1487+
// From an owned/borrowed Value (not a reference), map keys were deserialized as plain
1488+
// strings, so integer keys failed; all four Value functions must parse them.
1489+
use std::collections::BTreeMap;
1490+
let ok = BTreeMap::from([(1_i64, 2_u8), (-7, 3)]);
1491+
for (path, r) in map_key_results::<BTreeMap<i64, u8>>(r#"{"1":2,"-7":3}"#) {
1492+
assert_eq!(r.as_ref(), Some(&ok), "via {path}");
1493+
}
1494+
}

0 commit comments

Comments
 (0)