Zvariant cannot deserialize a byte array and another sequence

[zeenix]

In GitLab by @rosslannen on Aug 31, 2021, 02:08

The following code

let ctxt = Context::<LE>::new_dbus(0);
let value: (Vec<u8>, HashMap<String, Value<'_>>) = (Vec::new(), HashMap::new());
let value = zvariant::to_bytes(ctxt, &value).unwrap();
let (ba, hm): (&[u8], HashMap<&str, Value<'_>>) = zvariant::from_slice(&value, ctxt).unwrap();

panics with:

panicked at 'called `Result::unwrap()` on an `Err` value: Message("invalid type: character `y`, expected `v`, `a` or `(`")'

when the value is attempted to be de-serialized. The overall signature of the value is (aya{sv}). I was also able to recreate this with 2 byte vectors ((ayay)).

I don't understand de-serialization or parsing stuff much, so some help on this one would be greatly appreciated. From an initial look parsing the ay byte array doesn't appear to advance the signature parser enough and the hash map parser is encountering the y, but making the deserialize_ay() method advance the signature parser twice didn't seem to help.

[zeenix]

@rosslannen Ooh that's pretty bad! W/o looking deeply, I think this is very likely a breakage from !267 that greatly optimized byte deserialization. Could you please try reverting those changes and see if that helps?

BTW. It's good that you didn't (and hence found this bug) but for future ref, you might want to make use of serde_bytes when it comes to bytes. That's with serde in general, not only for zvariant.

[zeenix]

In GitLab by @rosslannen on Aug 31, 2021, 18:34

@zeenix I tried reverting all of the commits in that MR. There is still a failure, but with a different error message: invalid type: sequence, expected a borrowed byte array.

I also tried using the serde_bytes support on main. My test code now looks like:

let ctxt = Context::<LE>::new_dbus(0);
let value: (Vec<u8>, HashMap<String, Value<'_>>) = (Vec::new(), HashMap::new());
let value = zvariant::to_bytes(ctxt, &value).unwrap();
#[cfg(feature = "serde_bytes")]
let (ba, hm): (&serde_bytes::Bytes, HashMap<&str, Value<'_>>) =
    zvariant::from_slice(&value, ctxt).expect("Could not deserialize serde_bytes::Bytes in struct.");
#[cfg(not(feature = "serde_bytes"))]
let (ba, hm): (&[u8], HashMap<&str, Value<'_>>) =
    zvariant::from_slice(&value, ctxt).expect("Could not deserialize u8 slice in struct");

This code fails both with and without the serde_bytes feature enabled.

It appears that in the merge request you linked, some tests were added in https://gitlab.freedesktop.org/dbus/zbus/-/merge_requests/267/diffs?commit_id=29488f7481e3e6eaec569f244686eb51c9a610d3. I was also able to get those tests to break by adding a second &'a [u8] field into the struct after the first - and commenting out the length check:

#[derive(Deserialize, Serialize, Type, PartialEq, Debug)]      
struct AStruct<'s> {                                           
    field1: u16,                                               
    field2: &'s [u8],                                          
    field3: &'s [u8],                                          
    field4: i64,                                               
}                                                              
assert_eq!(AStruct::signature(), "(qayayx)");                  
let s = AStruct {                                              
    field1: 0xFF_FF,                                           
    field2: &[77u8; 8],                                        
    field3: &[77u8; 8],                                        
    field4: 0xFF_FF_FF_FF_FF_FF,                               
};                                                             
let encoded = to_bytes(ctxt, &s).unwrap();                     
// assert_eq!(encoded.len(), 24);                              
let decoded: AStruct<'_> = from_slice(&encoded, ctxt).unwrap();
assert_eq!(decoded, s);

This caused the test to fail during de-serialization with invalid type: sequence, expected ay.

Any thoughts on what else to try?

[zeenix]

In GitLab by @rosslannen on Aug 31, 2021, 19:54

@zeenix I found the issue. I was on the right track with the signature parser advancement, I just had it in the wrong location so it was breaking calls to find the signature of the array values. MR is up: https://gitlab.freedesktop.org/dbus/zbus/-/merge_requests/376.

[zeenix]

Great work! Many thanks for this.

[zeenix]

mentioned in commit 4c9f451c1c7dfb6de657416980ef8977c65a490e