Skip to content

Commit

Permalink
Take order into account for certain cases
Browse files Browse the repository at this point in the history
After we have dropped OrderedDict in e3b267e
we are relying on python3.7+ default behavior to preserve the insertion
order, but there is one caveat.
When comparing dictionaries the order is still irrelevant compared to
OrderedDict. For example:
>>> OrderedDict([(1,1), (2,2)]) == OrderedDict([(2,2), (1,1)])
False
>>> dict([(1,1), (2,2)]) == dict([(2,2), (1,1)])
True

There are two special attributes, defined in the specification, where
the order makes a difference when comparing two objects:
- Metadata.signatures
- Targets.delegations.roles.
We want to make sure that the order in those two cases makes a
difference when comparing two objects and that's why those changes
are required inside two __eq__ implementations.

Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
  • Loading branch information
MVrachev committed Feb 28, 2022
1 parent a17ceda commit 6ea5372
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 0 deletions.
61 changes: 61 additions & 0 deletions tests/test_metadata_eq_.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import unittest
from typing import Any, ClassVar, Dict

from securesystemslib.signer import Signature

from tests import utils
from tuf.api.metadata import (
TOP_LEVEL_ROLE_NAMES,
Expand Down Expand Up @@ -60,6 +62,23 @@ def test_metadata_eq_(self) -> None:
setattr(md_2, attr, value)
self.assertNotEqual(md, md_2, f"Failed case: {attr}")

def test_md_eq_signatures_reversed_order(self) -> None:
# Test comparing objects with same signatures but different order.

# Remove all signatures and create new ones.
md = Metadata.from_bytes(self.metadata["snapshot"])
md.signatures = {"a": Signature("a", "a"), "b": Signature("b", "b")}
md_2 = copy.deepcopy(md)
# Reverse signatures order in md_2.
# In python3.7 we need to cast to a list and then reverse.
md_2.signatures = dict(reversed(list(md_2.signatures.items())))
# Assert that both objects are not the same because of signatures order.
self.assertNotEqual(md, md_2)

# but if we fix the signatures order they will be equal
md_2.signatures = {"a": Signature("a", "a"), "b": Signature("b", "b")}
self.assertEqual(md, md_2)

def test_md_eq_special_signatures_tests(self) -> None:
# Test that metadata objects with different signatures are not equal.
md = Metadata.from_bytes(self.metadata["snapshot"])
Expand Down Expand Up @@ -226,6 +245,48 @@ def test_targetfile_eq_(self) -> None:
setattr(targetfile_2, "path", "")
self.assertNotEqual(targetfile, targetfile_2)

def test_delegations_eq_roles_reversed_order(self) -> None:
# Test comparing objects with same delegated roles but different order.
role_one_dict = {
"keyids": ["keyid1"],
"name": "a",
"terminating": False,
"paths": ["fn1"],
"threshold": 1,
}
role_two_dict = {
"keyids": ["keyid2"],
"name": "b",
"terminating": True,
"paths": ["fn2"],
"threshold": 4,
}

delegations_dict = {
"keys": {
"keyid2": {
"keytype": "ed25519",
"scheme": "ed25519",
"keyval": {"public": "bar"},
}
},
"roles": [role_one_dict, role_two_dict],
}
delegations = Delegations.from_dict(copy.deepcopy(delegations_dict))

# Create a second delegations obj with reversed roles order
delegations_2 = copy.deepcopy(delegations)
# In python3.7 we need to cast to a list and then reverse.
delegations_2.roles = dict(reversed(list(delegations.roles.items())))

# Both objects are not the equal because of delegated roles order.
self.assertNotEqual(delegations, delegations_2)

# but if we fix the delegated roles order they will be equal
delegations_2.roles = delegations.roles

self.assertEqual(delegations, delegations_2)

def test_targets_eq_(self) -> None:
md = Metadata.from_bytes(self.metadata["targets"])
signed_copy: Targets = self.copy_and_simple_assert(md.signed)
Expand Down
4 changes: 4 additions & 0 deletions tuf/api/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ def __eq__(self, other: Any) -> bool:

return (
self.signatures == other.signatures
# Order of the signatures matters (see issue #1788).
and list(self.signatures.items()) == list(other.signatures.items())
and self.signed == other.signed
and self.unrecognized_fields == other.unrecognized_fields
)
Expand Down Expand Up @@ -1429,6 +1431,8 @@ def __eq__(self, other: Any) -> bool:

return (
self.keys == other.keys
# Order of the delegated roles matters (see issue #1788).
and list(self.roles.items()) == list(other.roles.items())
and self.roles == other.roles
and self.unrecognized_fields == other.unrecognized_fields
)
Expand Down

0 comments on commit 6ea5372

Please sign in to comment.