Pure-Python BTree.items() can't be iterated, C implementation allows it

[jamadden]

(As reported in zopefoundation/zope.proxy#16.)

Given this code:

import BTrees.OOBTree
from zope.security.proxy import Proxy
from zope.security.checker import Checker

bt = BTrees.OOBTree.OOBTree()
bt[''] = 42
items = bt.items()

checker = Checker({})
proxy = Proxy(items, checker)

iter(proxy)

It passes using the C implementation of everything (specifically, the C implementation of BTrees), but it fails using pure-python (Python implementation of BTrees):

Traceback (most recent call last):
  File "test.py", line 15, in <module>
    iter(proxy)
  File "//zope.filepypy/site-packages/zope/security/proxy.py", line 28, in _wrapper
    checker.check(wrapped, name)
  File "//zope.filepypy/site-packages/zope/security/checker.py", line 229, in check
    raise ForbiddenAttribute(name, object)
ForbiddenAttribute: ('__iter__', <BTrees._base._TreeItems object at 0x000000010da483a0>)

(Note that using CheckerPy also works with the C implementation of BTrees and fails with the Python version. Also note this happens for both empty trees and for trees that have objects in them. In the empty case, items() returns a tuple and the exception is ForbiddenAttribute: ('__iter__', ()))

The checker logic doesn't raise an exception if the object doesn't have the __iter__ attribute:

/*         if name != '__iter__' or hasattr(object, name): */
/*             __traceback_supplement__ = (TracebackSupplement, object) */
/*             raise ForbiddenAttribute, (name, object) */

      if (strcmp("__iter__", MAKE_STRING(name)) == 0
          && ! PyObject_HasAttr(object, name))
        /* We want an attr error if we're asked for __iter__ and we don't
           have it. We'll get one by allowing the access. */
        return 0;
    }

The C implementation of TreeItems does not have an __iter__ method:

Traceback (most recent call last):
  File "test.py", line 9, in <module>
    print(items.__iter__)
AttributeError: 'OOBTreeItems' object has no attribute '__iter__'

Instead, the TreeItems implements both tp_iter and tp_iternext and is its own iter (tp_iter just returns self). But the objects returned by the Python implementation all (necessarily) have an __iter__.

Thoughts on how to resolve this? Maybe this isn't a zope.security bug, per-se, but whatever sets up the permissions for zope.app should specifically allow iter on BTrees._base._TreeItems? (That smells bad to me, not least because it fails for the empty/tuple case.)

[jamadden]

Some observations:

1. I don't think the empty BTree case actually matters. Tuples are wrapped in a checker that permits iteration. The example above is contrived; the real tests that are failing have a BTree or Folder or something that's wrapped in a proxy to start with, not the items itself.
2. The default list checker also allows iteration.
3. Under Python 3, the views of dict.keys, dict.values and dict.items get no checkers (because they're immutable).
4. There are special cases for list iterator, tuple iterator, etc, to allow iteration to actually work.
5. Similarly, zope.app.security provides access to XXTreeIterator classes to allow iteration to work (though those are considered implementation details) as well as __getitem__, __len, similarly to the special cases for list, etc, iterator.
6. However, none of the XXTreeItems classes have any special checkers assigned to them. Thus attempting to, for instance, index them with __getitem__ fails. iteration only works as a consequence of the implementation in C.
7. XXTreeItems are immutable.

So, all of that leads me to this conclusion:

• It would be safe to allow access to BTrees._base._TreeItems.__iter__ (it currently returns a generator, which is also special cased as an iterator); indeed it would be safe and possibly useful to allow access to __getitem__ and __len__ too.

This could be done in a few different ways:

1. In zope.security.checker._default_checkers via a conditional import (easy and doesn't need to know implementation details, though it's somewhat tedious due to all the families)
2. In zope.app.security._protectionts.zcml. That might be tricky because the C implementation doesn't expose XXTreeItems as an accessible/importable name, and it also doesn't have __iter__. I think that can be avoided if you're willing to rely directly on BTrees._base._TreeItems. Update: No, that fails. You do need to handle _TreeItems for the case that both Python and C implementations are in use, but you still also need to handle XXTreeItems from the C implementation, because there are cases that Python wants to access the __len__ of them.
3. The BTrees Python implementation of _TreeItems can conditionally (on the importability of zope.security) define a __Security_checker__ attribute to give the desired access. This is the easiest of all since it only has to be done once in one class. Update: No, that fails for the C items for the reason given above.
4. Alternately, that __Security_checker__ could be added in zope.security. This has the advantage of keeping zope.security out of BTrees, but the disadvantage of having BTree implementation knowledge embedded in zope.security. Update: No, that fails for the C items for the reason given above.

I've tested that the __Security_checker__ method works.

Thoughts?

[jamadden]

Another wrinkle is that Python 3.x will attempt to call __len__ to get the length hint in some cases. That fails with both the C and Python implementations. That makes several of the possible options I thought of incorrect.

I'm working on a PR. This package seems like the most logical place to address the issue.