google / serde_json5

Apache License 2.0
12 stars 9 forks source link

Specified number deserialization wrong #12

Open Cinea4678 opened 2 months ago

Cinea4678 commented 2 months ago

I came across this by accident:

fn test() {
        let test_str = r#"{"num": 1713749402391424255}"#;

        let parsed_by_serde_json5: HashMap<String, i64> = serde_json5::from_str(test_str).unwrap();
        let parsed_by_serde_json: HashMap<String, i64> = serde_json::from_str(test_str).unwrap();

        let parsed_by_serde_json5 = dbg!(parsed_by_serde_json5.get("num").unwrap());
        let parsed_by_serde_json = dbg!(parsed_by_serde_json.get("num").unwrap());

        assert_eq!(parsed_by_serde_json5, &1713749402391424255);
        assert_eq!(parsed_by_serde_json, &1713749402391424255);
}

Above is a code snippet deserializing 1713749402391424255 into i64, and as it runs, it panicked because of assertion error:

running 1 test
[src/parse.rs:190:37] parsed_by_serde_json5.get("num").unwrap() = 1713749402391424256
[src/parse.rs:191:36] parsed_by_serde_json.get("num").unwrap() = 1713749402391424255
thread 'parse::test::test_very_big_number' panicked at src/parse.rs:193:9:
assertion `left == right` failed
  left: 1713749402391424256
 right: 1713749402391424255
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test parse::test::test_very_big_number ... FAILED

Here is the version and my environment:

$ cargo tree | grep -E "serde_json5*"           
└── serde_json5 v0.2.0 (https://github.com/google/serde_json5.git?branch=master#788a2cf8)
└── serde_json v1.0.116

$ rustup default                     
stable-aarch64-apple-darwin (default)

$ rustc --version       
rustc 1.77.0 (aedd173a2 2024-03-17)
Cinea4678 commented 2 months ago

Add: I just found modifying this function like below can fix my problem while keep tests passing. If you think it's fine, I'd be happy to organize it into a PR, or if there's a better way, we can discuss it together.

third_party/src/de.rs:274

    fn deserialize_i64<V>(self, visitor: V) -> Result<V::Value>
    where
        V: de::Visitor<'de>,
    {
        let pair = self.pair.take().unwrap();
        let span = pair.as_span();

        let mut res = (move || if matches!(pair.as_rule(), Rule::number) && is_int(pair.as_str()) {
            visitor.visit_i64(parse_integer(&pair)? as i64)
        } else {
            visitor.visit_i64(parse_number(&pair)? as i64)
        })();
        error::set_location(&mut res, &span);
        res
    }