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

NamedValues::get footgun. #101

Open
jswrenn opened this issue Aug 24, 2022 · 0 comments
Open

NamedValues::get footgun. #101

jswrenn opened this issue Aug 24, 2022 · 0 comments

Comments

@jswrenn
Copy link

jswrenn commented Aug 24, 2022

NamedValues::get has a subtle footgun: The given reference to a NamedField must be pointer-equal to a NamedField in NamedValues. But, because NamedField is Copy/Clone, it's trivial to construct programs that subtly violate this requirement; e.g., this program panics:

fn main() {
    use valuable::{NamedField, NamedValues, Value};

    let fields = [
        NamedField::new("foo"),
        NamedField::new("bar")
    ];
    let values = [
        Value::U32(123),
        Value::U32(456),
    ];

    let named_values = NamedValues::new(&fields, &values);

    let field = fields[0];

    assert_eq!(
        named_values.get(&field).and_then(Value::as_u32),
        Some(123)
    );
}

...but this program is fine:

fn main() {
    use valuable::{NamedField, NamedValues, Value};

    let fields = [
        NamedField::new("foo"),
        NamedField::new("bar")
    ];
    let values = [
        Value::U32(123),
        Value::U32(456),
    ];

    let named_values = NamedValues::new(&fields, &values);

    let field = &fields[0];

    assert_eq!(
        named_values.get(field).and_then(Value::as_u32),
        Some(123)
    );
}
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

1 participant