lucab / libsystemd-rs

A pure-Rust client library to work with systemd
https://crates.io/crates/libsystemd
Other
106 stars 19 forks source link

Id128 has broken compact serde implementation #78

Open vilgotf opened 3 years ago

vilgotf commented 3 years ago

Adding the following test shows it clearly:

    #[test]
    fn test_ser_de() {
        let id = Id128::parse_str("1071334a-9324-4511-adcc-b8d8b70eb1c2").unwrap();

        assert_tokens(
            &id.readable(),
            &[Token::Str("1071334a93244511adccb8d8b70eb1c2")],
        );

        // expected bytes but serialized as Str
        /*assert_tokens(
            &id.compact(),
            &[Token::Bytes(&[
                16, 113, 51, 74, 147, 36, 69, 17, 173, 204, 184, 216, 183, 14, 177, 194,
            ])],
        );/*

        // invalid type string, expected bytes
        /*assert_tokens(
            &id.compact(),
            &[Token::Str("1071334a93244511adccb8d8b70eb1c2")],
        );*/

I'm working on a fix which will serialize compact to Token::Tuple which should be fine since the code is already broken

lucab commented 3 years ago

Thanks for the report. I think I may have initially overlooked the ser/de codepath, which resulted in this mixup.

Taking a step back here and thinking aloud, to what format should this type encode/decode to? The original C type is a union of byte-slices, but going in that direction doesn't sound very ergonomic. Trying to retro-guess myself, I think that here I originally wanted to go in the direction of https://en.wikipedia.org/wiki/Universally_unique_identifier#Format. If so, it needs to be stated explicitly and enforced by tests, as you highlighted.