seed-rs / seed

A Rust framework for creating web apps
MIT License
3.81k stars 155 forks source link

Deserializing large numbers fail #675

Closed kujeger closed 1 year ago

kujeger commented 2 years ago

Hi,

I discovered that when I was deserializing larger numbers in json payloads retrieved using fetch, there were off. If they are too large, deserialization completely fails with something like JsonError(Serde(JsValue("invalid value: integer '9223372036854776000', expected i64 at line 1 column 70"))).

This is new with 0.9; it does not happen in 0.8.

Example based on the simple fetch example:

// data.json
{
    "num1": 1234567890,
    "num2": 12345678901234567,
    "num3": 1234567890123456789
}
pub struct Data {
    num1: i64,
    num2: i64,
    num3: i64,
}
[..]
let response = fetch("data.json").await.expect("HTTP request failed");
let data = response
    .check_status()
    .expect("status check failed")
    .json::<Data>()
    .await
    .expect("deserialization failed");

After deserialization, the data has now become:

{
    "num1": 1234567890,
    "num2": 12345678901234568,
    "num3": 1234567890123456800
}

As you can see, something has gone wrong here.

As a workaround, manually deserializing the raw text using something like

response = fetch().[..].text().unwrap()
serde_json::from_str(&response)

works fine.

I put up a complete example runnable with trunk serve here: https://kujeger.net/dump/seed-json-deser-test.tar.gz

fosskers commented 2 years ago

Which JSON backend are you telling Seed to use - serde_json or serde-wasm-bindgen?

kujeger commented 2 years ago

Whichever is the default; looks like serde-wasm-bindgen

fosskers commented 2 years ago

Try serde_json and see what happens. It'll probably fix it in the meantime, although swb is supposed to have BigInt support :thinking:

kujeger commented 2 years ago

Looks like I was wrong; according to https://docs.rs/crate/seed/latest/features serde-json is the default.

Manually specifying serde-json in features gives the same result. Using serde-wasm-bindgen instead completely fails to deserialize the same numbers with this error:

JsonError(Serde(JsValue(Error: invalid type: floating point `12345678901234568`, expected i64
fosskers commented 2 years ago

From Rust itself:

In: 9223372036854776000_i64
error: literal out of range for `i64`
 --> src/main.rs:4:1
  |
4 | 9223372036854776000_i64
  | ^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[deny(overflowing_literals)]` on by default
  = note: the literal `9223372036854776000_i64` does not fit into the type `i64` whose range is `-9223372036854775808..=9223372036854775807`
  = help: consider using the type `u64` instead

but it does fit into u64. Consider changing the number type that you're using, if you know that value can never be negative.

flosse commented 2 years ago

@kujeger I downloaded your example code and it just works.

kujeger commented 2 years ago

From Rust itself:

In: 9223372036854776000_i64
error: literal out of range for `i64`
 --> src/main.rs:4:1
  |
4 | 9223372036854776000_i64
  | ^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[deny(overflowing_literals)]` on by default
  = note: the literal `9223372036854776000_i64` does not fit into the type `i64` whose range is `-9223372036854775808..=9223372036854775807`
  = help: consider using the type `u64` instead

but it does fit into u64. Consider changing the number type that you're using, if you know that value can never be negative.

To clarify, that was not the actual number I was attempting to deserialize when that error message was generated. The fetched payload was 9223372036854775807, the precise i64 max. It was just an example of "too large" a number resulting a panic, presumably because the buggy behaviour rounded it upwards outside of the i64 limit.

kujeger commented 2 years ago

@kujeger I downloaded your example code and it just works.

Could it be a platform issue? I've reproduced it across multiple machines and browsers (firefox and chromium), but they were all on linux. I don't have any windows or mac machines available for testing.

e: Just in case there's confusion, both buttons "work" in my example, but clicking the different buttons gives a different output below: "Fetch data" -> Data: 1234567890, 12345678901234568, 1234567890123456800 "Fetch data using serde" -> Data: 1234567890, 12345678901234567, 1234567890123456789

The numbers seem to be rounded up due to precision loss in the "Fetch data" case.

kujeger commented 2 years ago

I can reproduce the rounding behaviour with this:

let foo = 1234567890123456789_i64 as f64;
println!("{}", foo);

1234567890123456800

So it looks like fetch[..].json() might be casting the numbers as f64 or something like that.

fosskers commented 2 years ago

JS numbers are 53-bit, by the way.

flosse commented 1 year ago

obsolete since v0.10.0