Skip to content

Commit

Permalink
Make dict views behave like their unrestricted versions
Browse files Browse the repository at this point in the history
unlike the restricted versions, the unrestricted versions:
 - are not iterators, they are views
 - have a len
 - are false when the mapping is empty, true otherwise
 - are instances of collections.abc.MappingView

During this refactoring, also change `.items()` to validate
ach keys and values, like `.keys()` and `.values()` do.
  • Loading branch information
perrinjerome committed Mar 8, 2024
1 parent 738db73 commit c5d4a05
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 20 deletions.
7 changes: 7 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ For changes before version 3.0, see ``HISTORY.rst``.

- Add preliminary support for Python 3.13 as of 3.13a3.

- Make dict views (`.keys()`, `.items()` and `.values()`) behave like their
unrestricted versions.
(`#99999999999999 <https://github.com/zopefoundation/AccessControl/issues/99999999999999>`_)

- Make `.items()` validate each keys and values, like `.keys()` and
`.values()` do.


6.3 (2023-11-20)
----------------
Expand Down
48 changes: 37 additions & 11 deletions src/AccessControl/ZopeGuards.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
##############################################################################


import collections.abc
import math
import random
import string
Expand Down Expand Up @@ -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):
Expand All @@ -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
Expand Down Expand Up @@ -262,6 +265,29 @@ def __next__(self):
next = __next__


class _SafeMappingView:
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)
guard(self._mapping, v)
yield k, v


class NullIter(SafeIter):
def __init__(self, ob):
self._iter = ob
Expand Down
29 changes: 29 additions & 0 deletions src/AccessControl/tests/actual_python.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,35 @@ 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 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()
Expand Down
34 changes: 25 additions & 9 deletions src/AccessControl/tests/testZopeGuards.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,42 +258,58 @@ 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):
sm = SecurityManager()
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):
Expand Down

0 comments on commit c5d4a05

Please sign in to comment.