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

zvariant: Inconsistent serialization of varible key length dictionary contained within zvariant::Value #868

Closed
felinira opened this issue Jun 26, 2024 · 1 comment · Fixed by #870
Labels
gvariant zvariant Issues/PRs related to zvariant crate

Comments

@felinira
Copy link
Contributor

The following test code uses glib-rs to construct a VARIANT type gvariant with glib::Variant::from_variant and the same variant with zvariant::Value in gvariant mode.

    #[test]
    fn gvariant_vs_zvariant() {
        let mut map_glib = std::collections::HashMap::<&str, &str>::new();
        map_glib.insert("k", "v");
        let variant_glib = glib::Variant::from_variant(&map_glib.to_variant()).normal_form();
        let data_glib = variant_glib.data();

        let mut map_zvariant = std::collections::HashMap::<&str, &str>::new();
        map_zvariant.insert("k", "v".into());
        let ctxt = zvariant::serialized::Context::new_gvariant(zvariant::LE, 0);

        let data_zvariant = zvariant::to_bytes(ctxt, &zvariant::Value::new(map_zvariant)).unwrap();
        //let data_zvariant = zvariant::to_bytes(ctxt, &zvariant::SerializeValue(&map_zvariant)).unwrap();

        assert_eq!(data_glib, &*data_zvariant, "gvariant vs zvariant");
    }

This is a byte comparison of the serialisation. Left is glib, right is zvariant:

Diff < left / right > :
< 00000000  6B 00 76 00  02 05 00 61  7B 73 73 7D               k.v....a{ss}
> 00000000  6B 00 76 00  04 00 61 7B  73 73 7D                  k.v...a{ss}

It looks like zvariant does not encode the key framing offset, while glib does. https://developer.gnome.org/documentation/specifications/gvariant-specification-1.0.html#dictionary-entries.

Dictionary entries are treated as structures with exactly two items — first the key, then the value. In the case that the key is fixed-sized, there will be no framing offsets, and in the case the key is non-fixed-size there will be exactly one. As the value is treated as the last item in the structure, it will never have a framing offset.

glib will look for the framing offset, read the dict entry offset instead, and then fail to parse the entry correctly.

This is a comparison of glib::Variant::print on these variants, for completion sake:

Diff < left / right > :
<<{'k': 'v'}>
><{'': ''}>

Replacing zvariant::Value::new with zvariant::SerializeValue fixes the serialisation issue. However this is not always possible, especially when having to store variants in unserialized form, or when nesting variants.

@zeenix
Copy link
Contributor

zeenix commented Jun 26, 2024

Replacing zvariant::Value::new with zvariant::SerializeValue fixes the serialisation issue.

That's very strange. 🤔 The encoding of both should be exactly the same.

I'm afraid I won't have the time to investigate and fix this anytime soon. PRs welcome of course.

felinira added a commit to felinira/zbus that referenced this issue Jun 27, 2024
Adds a test for serializing dictionaries from zvariant::Value in contrast
to using zvariant::SerializeValue directly. Reproduces issue dbus2#868.
felinira added a commit to felinira/zbus that referenced this issue Jun 27, 2024
Dictionaries with variable length keys require a framing offset. The
SerializeMap implementation already handles this, the manual serialization
of dict entries used by Value does not. We now call directly into
SerializeMap when serializing Value to align these serializations.

Closes dbus2#868
felinira added a commit to felinira/zbus that referenced this issue Jun 27, 2024
Adds a test for serializing dictionaries from zvariant::Value in contrast
to using zvariant::SerializeValue directly. Reproduces issue dbus2#868.
felinira added a commit to felinira/zbus that referenced this issue Jun 27, 2024
Dictionaries with variable length keys require a framing offset. The
SerializeMap implementation already handles this, the manual serialization
of dict entries used by Value does not. We now call directly into
SerializeMap when serializing Value to align these serializations.

Closes dbus2#868
felinira added a commit to felinira/zbus that referenced this issue Jun 27, 2024
Adds a test for serializing dictionaries from zvariant::Value in contrast
to using zvariant::SerializeValue directly. Reproduces issue dbus2#868.
@zeenix zeenix closed this as completed in 0706b04 Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gvariant zvariant Issues/PRs related to zvariant crate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants