mirror of
https://salsa.debian.org/freeipa-team/freeipa.git
synced 2025-02-25 18:55:28 -06:00
Raise an error when updating CIDict with duplicate keys
Updating a CIDict with data like {'A': 1, 'a': 2} would lead to data loss since only one of the items would get to the CIDict. This can result in non-obvious bugs similar to this one in python-ldap: https://bugzilla.redhat.com/show_bug.cgi?id=1007820 Raise an error in this case; any resolution must be done by the caller.
This commit is contained in:
parent
0226064bac
commit
a93fc02af6
@ -455,8 +455,18 @@ class CIDict(dict):
|
||||
def __getitem__(self, key):
|
||||
return super(CIDict, self).__getitem__(key.lower())
|
||||
|
||||
def __setitem__(self, key, value):
|
||||
def __setitem__(self, key, value, seen_keys=None):
|
||||
"""cidict[key] = value
|
||||
|
||||
The ``seen_keys`` argument is used by ``update()`` to keep track of
|
||||
duplicate keys. It should be an initially empty set that is
|
||||
passed to all calls to __setitem__ that should not set duplicate keys.
|
||||
"""
|
||||
lower_key = key.lower()
|
||||
if seen_keys is not None:
|
||||
if lower_key in seen_keys:
|
||||
raise ValueError('Duplicate key in update: %s' % key)
|
||||
seen_keys.add(lower_key)
|
||||
self._keys[lower_key] = key
|
||||
return super(CIDict, self).__setitem__(lower_key, value)
|
||||
|
||||
@ -466,6 +476,14 @@ class CIDict(dict):
|
||||
return super(CIDict, self).__delitem__(lower_key)
|
||||
|
||||
def update(self, new=None, **kwargs):
|
||||
"""Update self from dict/iterable new and kwargs
|
||||
|
||||
Functions like ``dict.update()``.
|
||||
|
||||
Neither ``new`` nor ``kwargs`` may contain two keys that only differ in
|
||||
case, as this situation would result in loss of data.
|
||||
"""
|
||||
seen = set()
|
||||
if new:
|
||||
try:
|
||||
keys = new.keys
|
||||
@ -473,9 +491,10 @@ class CIDict(dict):
|
||||
self.update(dict(new))
|
||||
else:
|
||||
for key in keys():
|
||||
self[key] = new[key]
|
||||
self.__setitem__(key, new[key], seen)
|
||||
seen = set()
|
||||
for key, value in kwargs.iteritems():
|
||||
self[key] = value
|
||||
self.__setitem__(key, value, seen)
|
||||
|
||||
def __contains__(self, key):
|
||||
return super(CIDict, self).__contains__(key.lower())
|
||||
|
@ -243,6 +243,18 @@ class TestCIDict(object):
|
||||
'a': 'va', 'b': 'vb',
|
||||
'Key1': 'val1', 'key2': 'val2', 'KEY3': 'VAL3'}
|
||||
|
||||
def test_update_duplicate_values_dict(self):
|
||||
with nose.tools.assert_raises(ValueError):
|
||||
self.cidict.update({'a': 'va', 'A': None, 'b': 3})
|
||||
|
||||
def test_update_duplicate_values_list(self):
|
||||
with nose.tools.assert_raises(ValueError):
|
||||
self.cidict.update([('a', 'va'), ('A', None), ('b', 3)])
|
||||
|
||||
def test_update_duplicate_values_kwargs(self):
|
||||
with nose.tools.assert_raises(ValueError):
|
||||
self.cidict.update(a='va', A=None, b=3)
|
||||
|
||||
def test_update_kwargs(self):
|
||||
self.cidict.update(b='vb', key2='val2')
|
||||
assert dict(self.cidict.items()) == {
|
||||
|
Loading…
Reference in New Issue
Block a user