ComparableX509 and ComparableX509Req: __eq__, __ne__, __hash__ data model fixes.

acme/jose/json_util_test.py

@@ -12,12 +12,12 @@ from acme.jose import interfaces
 from acme.jose import util
 
 
-CERT = OpenSSL.crypto.load_certificate(
+CERT = util.ComparableX509(OpenSSL.crypto.load_certificate(
     OpenSSL.crypto.FILETYPE_PEM, pkg_resources.resource_string(
-        'letsencrypt.tests', os.path.join('testdata', 'cert.pem')))
-CSR = OpenSSL.crypto.load_certificate_request(
+        'letsencrypt.tests', os.path.join('testdata', 'cert.pem'))))
+CSR = util.ComparableX509(OpenSSL.crypto.load_certificate_request(
     OpenSSL.crypto.FILETYPE_PEM, pkg_resources.resource_string(
-        'letsencrypt.tests', os.path.join('testdata', 'csr.pem')))
+        'letsencrypt.tests', os.path.join('testdata', 'csr.pem'))))
 
 
 class FieldTest(unittest.TestCase):

acme/jose/util.py

@@ -35,26 +35,31 @@ class ComparableX509(object):  # pylint: disable=too-few-public-methods
 
     """
     def __init__(self, wrapped):
+        assert isinstance(wrapped, OpenSSL.crypto.X509) or isinstance(
+            wrapped, OpenSSL.crypto.X509Req)
         self._wrapped = wrapped
 
     def __getattr__(self, name):
         return getattr(self._wrapped, name)
 
+    def _dump(self, filetype=OpenSSL.crypto.FILETYPE_ASN1):
+        # pylint: disable=missing-docstring,protected-access
+        if isinstance(self._wrapped, OpenSSL.crypto.X509):
+            func = OpenSSL.crypto.dump_certificate
+        else:  # assert in __init__ makes sure this is X509Req
+            func = OpenSSL.crypto.dump_certificate_request
+        return func(filetype, self._wrapped)
+
     def __eq__(self, other):
-        filetype = OpenSSL.crypto.FILETYPE_ASN1
-        def as_der(obj):
-            # pylint: disable=missing-docstring,protected-access
-            if isinstance(obj, type(self)):
-                obj = obj._wrapped
-            if isinstance(obj, OpenSSL.crypto.X509):
-                func = OpenSSL.crypto.dump_certificate
-            elif isinstance(obj, OpenSSL.crypto.X509Req):
-                func = OpenSSL.crypto.dump_certificate_request
-            else:
-                raise TypeError(
-                    "Equality for {0} not provided".format(obj.__class__))
-            return func(filetype, obj)
-        return as_der(self) == as_der(other)
+        if not isinstance(other, self.__class__):
+            return NotImplemented
+        return self._dump() == other._dump()  # pylint: disable=protected-access
+
+    def __hash__(self):
+        return hash((self.__class__, self._dump()))
+
+    def __ne__(self, other):
+        return not self == other
 
     def __repr__(self):
         return '<{0}({1!r})>'.format(self.__class__.__name__, self._wrapped)
@@ -78,7 +83,7 @@ class ComparableRSAKey(object):  # pylint: disable=too-few-public-methods
         # pylint: disable=protected-access
         if (not isinstance(other, self.__class__) or
                 self._wrapped.__class__ is not other._wrapped.__class__):
-            return False
+            return NotImplemented
         # RSA*KeyWithSerialization requires cryptography>=0.8
         if isinstance(self._wrapped, rsa.RSAPrivateKeyWithSerialization):
             return self.private_numbers() == other.private_numbers()
@@ -87,24 +92,26 @@ class ComparableRSAKey(object):  # pylint: disable=too-few-public-methods
         else:
             return False  # we shouldn't reach here...
 
+    def __ne__(self, other):
+        return not self == other
 
     def __hash__(self):
         # public_numbers() hasn't got stable hash!
         if isinstance(self._wrapped, rsa.RSAPrivateKeyWithSerialization):
             priv = self.private_numbers()
             pub = priv.public_numbers
-            return hash((type(self), priv.p, priv.q, priv.dmp1,
+            return hash((self.__class__, priv.p, priv.q, priv.dmp1,
                          priv.dmq1, priv.iqmp, pub.n, pub.e))
         elif isinstance(self._wrapped, rsa.RSAPublicKeyWithSerialization):
             pub = self.public_numbers()
-            return hash((type(self), pub.n, pub.e))
+            return hash((self.__class__, pub.n, pub.e))
 
     def __repr__(self):
         return '<{0}({1!r})>'.format(self.__class__.__name__, self._wrapped)
 
     def public_key(self):
         """Get wrapped public key."""
-        return type(self)(self._wrapped.public_key())
+        return self.__class__(self._wrapped.public_key())
 
 
 class ImmutableMap(collections.Mapping, collections.Hashable):

acme/jose/util_test.py

@@ -27,13 +27,15 @@ class ComparableX509Test(unittest.TestCase):
     def test_eq(self):
         self.assertEqual(self.cert, self.cert_same)
 
-    def test_not_eq(self):
+    def test_eq_wrong_types(self):
+        self.assertNotEqual(self.cert, 5)
+
+    def test_ne(self):
         self.assertNotEqual(self.cert, self.cert2)
 
-    def test_eq_wrong_types(self):
-        from acme.jose.util import ComparableX509
-        self.assertRaises(
-            TypeError, ComparableX509(5).__eq__, ComparableX509(5))
+    def test_hash(self):
+        self.assertEqual(hash(self.cert), hash(self.cert_same))
+        self.assertNotEqual(hash(self.cert), hash(self.cert2))
 
     def test_repr(self):
         self.assertTrue(repr(self.cert).startswith(
@@ -53,6 +55,10 @@ class ComparableRSAKeyTest(unittest.TestCase):
                 password=None, backend=backend))
         self.key = load_key()
         self.key_same = load_key()
+        self.key2 = ComparableRSAKey(serialization.load_pem_private_key(
+            pkg_resources.resource_string(
+                __name__, os.path.join('testdata', 'rsa512_key.pem')),
+            password=None, backend=backend))
 
     def test_getattr_proxy(self):
         self.assertEqual(256, self.key.key_size)
@@ -60,20 +66,24 @@ class ComparableRSAKeyTest(unittest.TestCase):
     def test_eq(self):
         self.assertEqual(self.key, self.key_same)
 
-    def test_not_eq_different_types(self):
-        self.assertFalse(self.key.__eq__(5))
+    def test_ne(self):
+        self.assertNotEqual(self.key, self.key2)
 
-    def test_not_eq_not_wrapped(self):
+    def test_ne_different_types(self):
+        self.assertNotEqual(self.key, 5)
+
+    def test_ne_not_wrapped(self):
         # pylint: disable=protected-access
-        self.assertFalse(self.key.__eq__(self.key_same._wrapped))
+        self.assertNotEqual(self.key, self.key_same._wrapped)
 
-    def test_not_eq_no_serialization(self):
+    def test_ne_no_serialization(self):
         from acme.jose.util import ComparableRSAKey
-        self.assertFalse(ComparableRSAKey(5).__eq__(ComparableRSAKey(5)))
+        self.assertNotEqual(ComparableRSAKey(5), ComparableRSAKey(5))
 
     def test_hash(self):
         self.assertTrue(isinstance(hash(self.key), int))
         self.assertEqual(hash(self.key), hash(self.key_same))
+        self.assertNotEqual(hash(self.key), hash(self.key2))
 
     def test_repr(self):
         self.assertTrue(repr(self.key).startswith(