Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deserializing into a type with an unsigned integer doesn't give the expected result #93

Closed
repnop opened this issue Jan 27, 2019 · 3 comments

Comments

@repnop
Copy link

repnop commented Jan 27, 2019

Minimal example:

main.rs

use config::Config;
use serde::Deserialize;

#[derive(Deserialize)]
struct Foo {
    bar: u64,
}

fn main() {
    let mut settings = Config::new();
    settings.merge(config::File::with_name("test")).unwrap();
    
    let foo: Foo = settings.try_into().unwrap();
    println!("{}", foo.bar);
}

test.toml

bar = -1

Running this code produces 18446744073709551615 instead of erroring that the value was outside of the range of u64

@jcrevier
Copy link

Additionally, related to this is that values greater than i64::max_value() will fail to de-serialize into a u64. The problem seems to be the Deserialize implementation uses the into_int() method that converts everything into i64 then casts back to the needed type.

kesyog added a commit to kesyog/config-rs that referenced this issue Jun 28, 2022
* Attempt to convert between integer types using `TryInto`-based
conversions rather than blanket failing for some source and
destination types.
* Use `into_uint` instead of `into_int` in `Value` Deserialize
implementations for unsigned integer types. Previously, we were
converting from signed types to unsigned types using `as`, which can
lead to surprise integer values conversions (mehcode#93).

Fixes mehcode#352 and mehcode#93
@kesyog
Copy link
Contributor

kesyog commented Jun 28, 2022

Updated example:

use config::Config;
use serde::Deserialize;

#[derive(Deserialize)]
struct Foo {
    bar: u64,
}

fn main() {
    let settings = Config::builder()
        .add_source(config::File::with_name("test"))
        .build()
        .unwrap();

    let foo: Foo = settings.try_deserialize().unwrap();
    println!("{}", foo.bar);
}

kesyog added a commit to kesyog/config-rs that referenced this issue Jun 29, 2022
* Attempt to convert between integer types using `TryInto`-based
conversions rather than blanket failing for some source and
destination types.
* Use `into_uint` instead of `into_int` in `Value` Deserialize
implementations for unsigned integer types. Previously, we were
converting from signed types to unsigned types using `as`, which can
lead to surprise integer values conversions (mehcode#93).

Fixes mehcode#352 and mehcode#93
@kesyog
Copy link
Contributor

kesyog commented Jun 29, 2022

Fixed on master by #353

matthiasbeyer pushed a commit to matthiasbeyer/config-rs that referenced this issue Aug 2, 2022
* Attempt to convert between integer types using `TryInto`-based
conversions rather than blanket failing for some source and
destination types.
* Use `into_uint` instead of `into_int` in `Value` Deserialize
implementations for unsigned integer types. Previously, we were
converting from signed types to unsigned types using `as`, which can
lead to surprise integer values conversions (mehcode#93).

Fixes mehcode#352 and mehcode#93

(cherry picked from commit 7db2e8b)
matthiasbeyer pushed a commit that referenced this issue Aug 2, 2022
* Attempt to convert between integer types using `TryInto`-based
conversions rather than blanket failing for some source and
destination types.
* Use `into_uint` instead of `into_int` in `Value` Deserialize
implementations for unsigned integer types. Previously, we were
converting from signed types to unsigned types using `as`, which can
lead to surprise integer values conversions (#93).

Fixes #352 and #93

(cherry picked from commit 7db2e8b)
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants