From c1456d48a17fa2f4c326a21c52a776243a76bb24 Mon Sep 17 00:00:00 2001 From: Luuk van der Duim Date: Wed, 21 Jun 2023 02:11:59 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=A9=B9=20=F0=9F=9A=B8=20zv:=20`Signature`?= =?UTF-8?q?=20equality=20regardless=20of=20outer=20parentheses?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adjusts the `PartialEq` impls to evaluate equality irrespective of of marshalling of the type. Two signatures representing the same type can differ as a result of D-Bus marshalling. D-Bus marshalling will omit outer parentheses from body singatures. Consider a `MyType { a: i32, b: i32}`: `MyType::signature()` returns a "(ii)" `Signature` whereas `Message::body_signature()` with `MyType` as body, would return a "ii" `Signature`, which may lead to unexpected inequality of `Signature`s. Note: `PartialEq` on `Signature` accounts for marshalling only. One can construct- or deserialize into deeper nested structs or tuples than than the signature describes but at that point the divergence is expected. --- zvariant/src/signature.rs | 42 +++++++++++++++++++++++++++++++------ zvariant/src/utils.rs | 44 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/zvariant/src/signature.rs b/zvariant/src/signature.rs index f1e3b2942..216f22cbd 100644 --- a/zvariant/src/signature.rs +++ b/zvariant/src/signature.rs @@ -83,7 +83,7 @@ impl<'b> std::ops::Deref for Bytes<'b> { /// /// [identifies]: https://dbus.freedesktop.org/doc/dbus-specification.html#type-system /// [`slice`]: #method.slice -#[derive(Eq, Hash, Clone)] +#[derive(Hash, Clone)] pub struct Signature<'a> { bytes: Bytes<'a>, pos: usize, @@ -354,24 +354,39 @@ impl<'a> std::ops::Deref for Signature<'a> { } } -impl<'a, 'b> PartialEq> for Signature<'b> { - fn eq(&self, other: &Signature<'_>) -> bool { - self.as_bytes() == other.as_bytes() +/// Evaluate equality of two signatures, ignoring outer parentheses. +impl<'a, 'b> PartialEq> for Signature<'a> { + fn eq(&self, other: &Signature<'b>) -> bool { + match ( + self.without_outer_parentheses(), + other.without_outer_parentheses(), + ) { + (Some(lhs_without), None) => lhs_without == other.as_bytes(), + (None, Some(rhs_without)) => self.as_bytes() == rhs_without, + _ => self.as_bytes() == other.as_bytes(), + } } } +// Assumes that `str` is a valid signature string. impl<'a> PartialEq for Signature<'a> { fn eq(&self, other: &str) -> bool { - self.as_bytes() == other.as_bytes() + let other = Signature::from_str_unchecked(other); + self.eq(&other) } } +// Assumes that `&str` is a valid signature string. impl<'a> PartialEq<&str> for Signature<'a> { fn eq(&self, other: &&str) -> bool { - self.as_bytes() == other.as_bytes() + let other = Signature::from_str_unchecked(other); + self.eq(&other) } } +// `Eq` docs: "the derive strategy requires all fields are Eq, which isn’t always desired.". +impl Eq for Signature<'_> {} + impl<'a> Display for Signature<'a> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { std::fmt::Display::fmt(&self.as_str(), f) @@ -515,4 +530,19 @@ mod tests { assert_eq!(slice, "t"); assert_eq!(slice.slice(1..), ""); } + + #[test] + fn signature_equality() { + let sig_a = Signature::from_str_unchecked("(asta{sv})"); + let sig_b = Signature::from_str_unchecked("asta{sv}"); + assert_eq!(sig_a, sig_b); + + let sig_a = Signature::from_str_unchecked("((so)ii(uu))"); + let sig_b = Signature::from_str_unchecked("(so)ii(uu)"); + assert_eq!(sig_a, sig_b); + + let sig_a = Signature::from_str_unchecked("(so)i"); + let sig_b = Signature::from_str_unchecked("(so)u"); + assert_ne!(sig_a, sig_b); + } } diff --git a/zvariant/src/utils.rs b/zvariant/src/utils.rs index 359b54849..706c307f6 100644 --- a/zvariant/src/utils.rs +++ b/zvariant/src/utils.rs @@ -316,3 +316,47 @@ where { input.get(index).ok_or(Error::OutOfBounds) } + +impl Signature<'_> { + /// Determines whether the signature has outer parentheses and returns the + /// signature without them if it does. + /// + /// # Safety + /// + /// The byte wise comparison in this method requires the signature to be + /// of an encoding that is equal to utf8 with respect to the encoding of + /// ASCII characters. + /// D-Bus signature type codes each are a single ASCII character. + /// + pub(crate) fn without_outer_parentheses(&self) -> Option<&[u8]> { + let bytes = self.as_bytes(); + + if let [b'(', subslice @ .., b')'] = bytes { + if subslice.iter().fold(0, |count, byte| match byte { + b'(' => count + 1, + b')' if count != 0 => count - 1, + _ => count, + }) == 0 + { + return Some(subslice); + } + }; + + None + } +} + +#[test] +fn test_without_outer_parentheses() { + let sig = Signature::from_str_unchecked("(asta{sv})"); + assert_eq!(sig.without_outer_parentheses(), Some("asta{sv}".as_bytes())); + + let sig = Signature::from_str_unchecked("asta{sv}"); + assert_eq!(sig.without_outer_parentheses(), None); + + let sig = Signature::from_str_unchecked("((so)(uu))"); + assert_eq!(sig.without_outer_parentheses(), Some("(so)(uu)".as_bytes())); + + let sig = Signature::from_str_unchecked("(so)ii(uu)"); + assert_eq!(sig.without_outer_parentheses(), None); +}