Skip to content

Commit

Permalink
🩹 🚸 zv: Signature equality regardless of outer parentheses
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
luukvanderduim committed Jun 21, 2023
1 parent ee7e2b2 commit c1456d4
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 6 deletions.
42 changes: 36 additions & 6 deletions zvariant/src/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -354,24 +354,39 @@ impl<'a> std::ops::Deref for Signature<'a> {
}
}

impl<'a, 'b> PartialEq<Signature<'a>> 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<Signature<'b>> 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<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)
}
}

// 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)
Expand Down Expand Up @@ -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);
}
}
44 changes: 44 additions & 0 deletions zvariant/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

0 comments on commit c1456d4

Please sign in to comment.