Ticket #3008: DN objects hash differently depending on case

Because the attrs & values in DN's, RDN's and AVA's are comparison case-
insensitive the hash value between two objects which compare as equal but
differ in case must also yield the same hash value. This is critical when
these objects are used as a dict key or in a set because dicts and sets
use the object's __hash__ value in conjunction with the objects __eq__
method to lookup the object.

The defect is the DN, RDN & AVA objects computed their hash from the case-
preserving string representation thus two otherwise equal objects
incorrectly yielded different hash values.

The problem manifests itself when one of these objects is used as a key in
a dict, for example a dn.

dn1 = DN(('cn', 'Bob'))
dn2 = DN(('cn', 'bob'))

dn1 == dn2 --> True

hash(dn1) == hash(dn2) --> False

d = {}

d[dn1] = x
d[dn2] = y

len(d) --> 2

The patch fixes the above by lower casing the string representation of
the object prior to computing it's hash.

The patch also corrects a spelling mistake and a bogus return value in
ldapupdate.py which happened to be discovered while researching this
bug.

3 files changed, 45 insertions, 30 deletions

diff --git a/ipapython/dn.py b/ipapython/dn.py
index 6cce40e29..834291fbe 100644
--- a/ipapython/dn.py
+++ b/ipapython/dn.py
@@ -385,7 +385,7 @@ if container_dn in dn:
 # (e.g. cmp()). The general rule is for objects supporting multiple
 # values first their lengths are compared, then if the lengths match
 # the respective components of each are pair-wise compared until one
-# is discovered to be  non-equal. The comparision is case insensitive.
+# is discovered to be  non-equal. The comparison is case insensitive.
 
 Cloning (Object Copy):
 
@@ -544,7 +544,7 @@ class AVA(object):
 
     AVA objects support equality testing and comparsion (e.g. cmp()). When they
     are compared the attr is compared first, if the 2 attr's are equal then the
-    values are compared. The comparision is case insensitive (because attr's map
+    values are compared. The comparison is case insensitive (because attr's map
     to numeric OID's and their values derive from from the 'name' atribute type
     (OID 2.5.4.41) whose EQUALITY MATCH RULE is caseIgnoreMatch.
 
@@ -648,8 +648,13 @@ class AVA(object):
                                 (key.__class__.__name__))
 
     def __hash__(self):
-        # Hash is computed from AVA's string representation because it's immutable
-        return hash(str(self))
+        # Hash is computed from AVA's string representation because it's immutable.
+        #
+        # Because attrs & values are comparison case-insensitive the
+        # hash value between two objects which compare as equal but
+        # differ in case must yield the same hash value.
+
+        return hash(str(self).lower())
 
     def __eq__(self, other):
         '''
@@ -676,7 +681,7 @@ class AVA(object):
         if not isinstance(other, AVA):
             return False
 
-        # Perform comparision between objects of same type
+        # Perform comparison between objects of same type
         return self._attr_unicode.lower() == other.attr.lower() and \
             self._value_unicode.lower() == other.value.lower()
 
@@ -684,7 +689,7 @@ class AVA(object):
         return not self.__eq__(other)
 
     def __cmp__(self, other):
-        'comparision is case insensitive, see __eq__ doc for explanation'
+        'comparison is case insensitive, see __eq__ doc for explanation'
 
         if not isinstance(other, AVA):
             raise TypeError("expected AVA but got %s" % (other.__class__.__name__))
@@ -803,8 +808,8 @@ class RDN(object):
     the first AVA's properties, programmer beware! Recall the AVA's in the RDN
     are sorted according the to AVA collating semantics.
 
-    RDN objects support equality testing and comparision. See AVA for the
-    definition of the comparision method.
+    RDN objects support equality testing and comparison. See AVA for the
+    definition of the comparison method.
 
     RDN objects support concatenation and addition with other RDN's or AVA's
 
@@ -933,7 +938,12 @@ class RDN(object):
 
     def __hash__(self):
         # Hash is computed from RDN's string representation because it's immutable
-        return hash(str(self))
+        #
+        # Because attrs & values are comparison case-insensitive the
+        # hash value between two objects which compare as equal but
+        # differ in case must yield the same hash value.
+
+        return hash(str(self).lower())
 
     def __eq__(self, other):
         # Try coercing string to RDN, if successful compare to coerced object
@@ -948,7 +958,7 @@ class RDN(object):
         if not isinstance(other, RDN):
             return False
 
-        # Perform comparision between objects of same type
+        # Perform comparison between objects of same type
         return self.avas == other.avas
 
     def __ne__(self, other):
@@ -1170,8 +1180,8 @@ class DN(object):
     dn.insert(i, RDN(('cn','Bob')))
     dn[i:i] = [('cn','Bob')]
 
-    DN objects support equality testing and comparision. See RDN for the
-    definition of the comparision method.
+    DN objects support equality testing and comparison. See RDN for the
+    definition of the comparison method.
 
     DN objects implement startswith(), endswith() and the "in" membership
     operator. You may pass a DN or RDN object to these. Examples:
@@ -1290,7 +1300,12 @@ class DN(object):
 
     def __hash__(self):
         # Hash is computed from DN's string representation because it's immutable
-        return hash(str(self))
+        #
+        # Because attrs & values are comparison case-insensitive the
+        # hash value between two objects which compare as equal but
+        # differ in case must yield the same hash value.
+
+        return hash(str(self).lower())
 
     def __eq__(self, other):
         # Try coercing string to DN, if successful compare to coerced object
@@ -1305,7 +1320,7 @@ class DN(object):
         if not isinstance(other, DN):
             return False
 
-        # Perform comparision between objects of same type
+        # Perform comparison between objects of same type
         return self.rdns == other.rdns
 
     def __ne__(self, other):
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index a51edff99..d673ad57d 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -265,7 +265,7 @@ class LDAPUpdate:
 
         if not all_updates.get(dn):
             all_updates[dn] = update
-            return all_updates
+            return
 
         existing_update = all_updates[dn]
         if 'default' in update:
diff --git a/tests/test_ipapython/test_dn.py b/tests/test_ipapython/test_dn.py
index 441affa4f..cdeab9374 100644
--- a/tests/test_ipapython/test_dn.py
+++ b/tests/test_ipapython/test_dn.py
@@ -267,12 +267,12 @@ class TestAVA(unittest.TestCase):
             self.assertEqual(result, -1)
 
     def test_hashing(self):
-        # create AVA's that have the same value
-        immutable_ava1 = AVA((self.attr1, self.value1))
-        immutable_ava2 = AVA((self.attr1, self.value1))
+        # create AVA's that are equal but differ in case
+        immutable_ava1 = AVA((self.attr1.lower(), self.value1.upper()))
+        immutable_ava2 = AVA((self.attr1.upper(), self.value1.lower()))
 
-        mutable_ava1   = EditableAVA((self.attr1, self.value1))
-        mutable_ava2   = EditableAVA((self.attr1, self.value1))
+        mutable_ava1   = EditableAVA((self.attr1.lower(), self.value1.upper()))
+        mutable_ava2   = EditableAVA((self.attr1.upper(), self.value1.lower()))
 
         # Immutable AVA's that are equal should hash to the same value.
         # Mutable AVA's should not be hashable.
@@ -828,12 +828,12 @@ class TestRDN(unittest.TestCase):
 
 
     def test_hashing(self):
-        # create RDN's that have the same value
-        immutable_rdn1 = RDN((self.attr1, self.value1))
-        immutable_rdn2 = RDN((self.attr1, self.value1))
+        # create RDN's that are equal but differ in case
+        immutable_rdn1 = RDN((self.attr1.lower(), self.value1.upper()))
+        immutable_rdn2 = RDN((self.attr1.upper(), self.value1.lower()))
 
-        mutable_rdn1   = EditableRDN((self.attr1, self.value1))
-        mutable_rdn2   = EditableRDN((self.attr1, self.value1))
+        mutable_rdn1   = EditableRDN((self.attr1.lower(), self.value1.upper()))
+        mutable_rdn2   = EditableRDN((self.attr1.upper(), self.value1.lower()))
 
         # Immutable RDN's that are equal should hash to the same value.
         # Mutable RDN's should not be hashable.
@@ -1706,12 +1706,12 @@ class TestDN(unittest.TestCase):
                     self.assertExpectedClass(DN_class, dn[i][j], 'AVA')
 
     def test_hashing(self):
-        # create DN's that have the same value
-        immutable_dn1 = DN((self.attr1, self.value1))
-        immutable_dn2 = DN((self.attr1, self.value1))
+        # create DN's that are equal but differ in case
+        immutable_dn1 = DN((self.attr1.lower(), self.value1.upper()))
+        immutable_dn2 = DN((self.attr1.upper(), self.value1.lower()))
 
-        mutable_dn1   = EditableDN((self.attr1, self.value1))
-        mutable_dn2   = EditableDN((self.attr1, self.value1))
+        mutable_dn1   = EditableDN((self.attr1.lower(), self.value1.upper()))
+        mutable_dn2   = EditableDN((self.attr1.upper(), self.value1.lower()))
 
         # Immutable DN's that are equal should hash to the same value.
         # Mutable DN's should not be hashable.