diff --git a/CHANGES.rst b/CHANGES.rst index f35a8d2..073b791 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -8,6 +8,13 @@ For changes before version 3.0, see ``HISTORY.rst``. - Nothing changed yet. +- Make dict views (`.keys()`, `.items()` and `.values()`) behave like their + unrestricted versions. + (`#147 `_) + +- Make `.items()` validate each keys and values, like `.keys()` and + `.values()` do. + 6.3 (2023-11-20) ---------------- diff --git a/src/AccessControl/ZopeGuards.py b/src/AccessControl/ZopeGuards.py index 84c2e9e..73515f6 100644 --- a/src/AccessControl/ZopeGuards.py +++ b/src/AccessControl/ZopeGuards.py @@ -12,6 +12,7 @@ ############################################################################## +import collections.abc import math import random import string @@ -127,13 +128,18 @@ def guarded_pop(key, default=_marker): return guarded_pop -def get_iter(c, name): - iter = getattr(c, name) +def get_mapping_view(c, name): - def guarded_iter(): - return SafeIter(iter(), c) + view_class = { + 'keys': SafeKeysView, + 'items': SafeItemsView, + 'values': SafeValuesView, + } - return guarded_iter + def guarded_mapping_view(): + return view_class[name](c) + + return guarded_mapping_view def get_list_pop(lst, name): @@ -153,18 +159,15 @@ def guarded_pop(index=-1): 'copy': 1, 'fromkeys': 1, 'get': get_dict_get, - 'items': 1, + 'items': get_mapping_view, + 'keys': get_mapping_view, 'pop': get_dict_pop, 'popitem': 1, 'setdefault': 1, 'update': 1, + 'values': get_mapping_view, } -_dict_white_list.update({ - 'keys': get_iter, - 'values': get_iter, -}) - def _check_dict_access(name, value): # Check whether value is a dict method @@ -272,6 +275,40 @@ def __next__(self): next = __next__ +# The following three view classes are used only for mappings of type `dict` +# (not subclasses). Therefore, the mapping does not have security assertions +# and cannot acquire ones. As a consequence, the `guard` calls used in their +# methods verify that the checked key or value is accessible based solely on +# its own virtues, i.e. either because it is public or has its own security +# assertions allowing access. +class _SafeMappingView: + __allow_access_to_unprotected_subobjects__ = 1 + + def __iter__(self): + for e in super().__iter__(): + guard(self._mapping, e) + yield e + + +class SafeKeysView(_SafeMappingView, collections.abc.KeysView): + pass + + +class SafeValuesView(_SafeMappingView, collections.abc.ValuesView): + pass + + +class SafeItemsView(_SafeMappingView, collections.abc.ItemsView): + def __iter__(self): + for k, v in super().__iter__(): + guard(self._mapping, k) + # When checking the value, we check the guard with index=None, + # not with index=k, the key name does not matter. guard just + # needs to verify that the value itself can be accessed. + guard(self._mapping, v) + yield k, v + + def _error(index): raise Unauthorized('unauthorized access to element') diff --git a/src/AccessControl/tests/actual_python.py b/src/AccessControl/tests/actual_python.py index 3405b8e..866a480 100644 --- a/src/AccessControl/tests/actual_python.py +++ b/src/AccessControl/tests/actual_python.py @@ -123,6 +123,39 @@ def f7(): access = getattr(d, meth) result = sorted(access()) assert result == expected[kind], (meth, kind, result, expected[kind]) + assert len(access()) == len(expected[kind]), (meth, kind, "len") + iter_ = access() # iterate twice on the same view + assert list(iter_) == list(iter_) + + assert sorted([k for k in getattr(d, meth)()]) == expected[kind] + assert sorted(k for k in getattr(d, meth)()) == expected[kind] + assert {k: v for k, v in d.items()} == d + + assert 1 in d + assert 1 in d.keys() + assert 2 in d.values() + assert (1, 2) in d.items() + + assert d + assert d.keys() + assert d.values() + assert d.items() + + empty_d = {} + assert not empty_d + assert not empty_d.keys() + assert not empty_d.values() + assert not empty_d.items() + + smaller_d = {1: 2} + for m, _ in methods: + assert getattr(d, m)() != getattr(smaller_d, m)() + assert not getattr(d, m)() == getattr(smaller_d, m)() + if m != 'values': + assert getattr(d, m)() > getattr(smaller_d, m)() + assert getattr(d, m)() >= getattr(smaller_d, m)() + assert getattr(smaller_d, m)() < getattr(d, m)() + assert getattr(smaller_d, m)() <= getattr(d, m)() f7() diff --git a/src/AccessControl/tests/testZopeGuards.py b/src/AccessControl/tests/testZopeGuards.py index 533bfa2..50eeca9 100644 --- a/src/AccessControl/tests/testZopeGuards.py +++ b/src/AccessControl/tests/testZopeGuards.py @@ -258,23 +258,40 @@ def test_pop_validates(self): self.assertTrue(sm.calls) def test_keys_empty(self): - from AccessControl.ZopeGuards import get_iter - keys = get_iter({}, 'keys') + from AccessControl.ZopeGuards import get_mapping_view + keys = get_mapping_view({}, 'keys') self.assertEqual(list(keys()), []) + def test_kvi_len(self): + from AccessControl.ZopeGuards import get_mapping_view + for attr in ("keys", "values", "items"): + with self.subTest(attr): + view = get_mapping_view({'a': 1}, attr) + self.assertEqual(len(view()), 1) + def test_keys_validates(self): sm = SecurityManager() old = self.setSecurityManager(sm) keys = guarded_getattr({GuardTestCase: 1}, 'keys') try: - next(keys()) + next(iter(keys())) finally: self.setSecurityManager(old) self.assertTrue(sm.calls) + def test_items_validates(self): + sm = SecurityManager() + old = self.setSecurityManager(sm) + items = guarded_getattr({GuardTestCase: GuardTestCase}, 'items') + try: + next(iter(items())) + finally: + self.setSecurityManager(old) + self.assertEqual(len(sm.calls), 2) + def test_values_empty(self): - from AccessControl.ZopeGuards import get_iter - values = get_iter({}, 'values') + from AccessControl.ZopeGuards import get_mapping_view + values = get_mapping_view({}, 'values') self.assertEqual(list(values()), []) def test_values_validates(self): @@ -282,18 +299,17 @@ def test_values_validates(self): old = self.setSecurityManager(sm) values = guarded_getattr({GuardTestCase: 1}, 'values') try: - next(values()) + next(iter(values())) finally: self.setSecurityManager(old) self.assertTrue(sm.calls) def test_kvi_iteration(self): - from AccessControl.ZopeGuards import SafeIter d = dict(a=1, b=2) for attr in ("keys", "values", "items"): v = getattr(d, attr)() - si = SafeIter(v) - self.assertEqual(next(si), next(iter(v))) + si = guarded_getattr(d, attr)() + self.assertEqual(next(iter(si)), next(iter(v))) class TestListGuards(GuardTestCase):