Changed strategy to hide wrapped to not use the secret anymore, but look

at the frames. This fixes a bunch of issues when actually doing binary
ops (for which I added a test now).

__len__ must return a non-proxied object, so I added a new parameter to
the name check wrapper to handle that case.

Fixed all but 12 test failures on PyPy.

src/zope/security/proxy.py

@@ -20,24 +20,24 @@
 from zope.security._compat import PYPY
 from zope.security.interfaces import ForbiddenAttribute
 
-# Never expose this attribute in an utrusted environment.
-_secret = str(hash(object()))
-
-def _check_name(meth):
+def _check_name(meth, wrap_result=True):
     name = meth.__name__
     def _wrapper(self, *args, **kw):
         wrapped = super(PyProxyBase, self).__getattribute__('_wrapped')
         checker = super(PyProxyBase, self).__getattribute__('_checker')
-        checker.check_getattr(wrapped, name)
-        return checker.proxy(getattr(wrapped, name)(*args, **kw))
+        checker.check(wrapped, name)
+        res = meth(self, *args, **kw)
+        if not wrap_result:
+            return res
+        return checker.proxy(res)
     return functools.update_wrapper(_wrapper, meth)
 
 def _check_name_inplace(meth):
     name = meth.__name__
     def _wrapper(self, *args, **kw):
         wrapped = super(PyProxyBase, self).__getattribute__('_wrapped')
         checker = super(PyProxyBase, self).__getattribute__('_checker')
-        checker.check_getattr(wrapped, name)
+        checker.check(wrapped, name)
         w_meth = getattr(wrapped, name, None)
         if w_meth is not None:
             # The proxy object cannot change; we are modifying in place.
@@ -74,14 +74,15 @@ def __init__(self, value, checker):
 
     # Attribute protocol
     def __getattribute__(self, name):
-        # Explicitely disallow _wrapped and _checker to be accessed.
         if name in ('_wrapped', '_checker'):
-            raise AttributeError(name)
+            # Only allow _wrapped and _checker to be accessed from inside.
+            if sys._getframe(1).f_locals.get('self') is not self:
+                raise AttributeError(name)
         wrapped = super(PyProxyBase, self).__getattribute__('_wrapped')
-        if name == '_wrapped'+_secret:
+        if name == '_wrapped':
             return wrapped
         checker = super(PyProxyBase, self).__getattribute__('_checker')
-        if name == '_checker'+_secret:
+        if name == '_checker':
             return checker
         if name not in ['__cmp__', '__hash__', '__bool__', '__nonzero__',
                         '__lt__', '__le__', '__eq__', '__ne__', '__ge__',
@@ -113,13 +114,13 @@ def __delattr__(self, name):
 
     @_check_name
     def __getslice__(self, start, end):
-        getitem = super(PyProxyBase, self).__getattribute__('__getitem__')
-        return getitem(start, end)
+        getitem = PyProxyBase.__getattribute__(self, '__getitem__')
+        return getitem(slice(start, end))
 
     @_check_name
     def __setslice__(self, i, j, value):
-        setitem = super(PyProxyBase, self).__getattribute__('__setitem__')
-        return setitem(i, j, value)
+        setitem = PyProxyBase.__getattribute__(self, '__setitem__')
+        return setitem(slice(i, j), value)
 
     def __cmp__(self, other):
         # no check
@@ -212,7 +213,6 @@ def __repr__(self):
              #'__bool__',    # Unchecked in C proxy (rich coparison)
              #'__hash__',    # Unchecked in C proxy (rich coparison)
              #'__cmp__',     # Unchecked in C proxy
-             '__len__',
              '__getitem__',
              '__setitem__',
              '__delitem__',
@@ -263,6 +263,11 @@ def __repr__(self):
     meth = getattr(PyProxyBase, name)
     setattr(ProxyPy, name, _check_name(meth))
 
+for name in ['__len__',
+             ]:
+    meth = getattr(PyProxyBase, name)
+    setattr(ProxyPy, name, _check_name(meth, False))
+
 for name in ['__iadd__',
              '__isub__',
              '__imul__',
@@ -281,10 +286,13 @@ def __repr__(self):
     setattr(ProxyPy, name, _check_name_inplace(meth))
 
 def getCheckerPy(proxy):
-    return getattr(proxy, '_checker'+_secret)
+    return super(PyProxyBase, proxy).__getattribute__('_checker')
 
 def getObjectPy(proxy):
-    return getattr(proxy, '_wrapped'+_secret)
+    isinstance_func = builtin_isinstance or isinstance
+    if not isinstance_func(proxy, ProxyPy):
+        return proxy
+    return super(PyProxyBase, proxy).__getattribute__('_wrapped')
 
 try:
     from zope.security._proxy import _Proxy

src/zope/security/tests/test_adapter.py

@@ -165,6 +165,7 @@ class _NotAdapter(object):
     @_skip_wo_zope_location
     def test__call__w_non_ILocation_w_spacesuit(self):
         from zope.proxy import getProxiedObject
+        from zope.security.proxy import getObject
         from zope.security.proxy import ProxyFactory
         from zope.security.proxy import removeSecurityProxy
         factory = self._makeFactory()
@@ -239,7 +240,7 @@ class _Extra(object):
     def test__call__w_ILocation_w_spacesuit(self):
         from zope.interface import directlyProvides
         from zope.location import ILocation
-        from zope.proxy import getProxiedObject
+        from zope.security.proxy import getObject
         from zope.security.proxy import ProxyFactory
         from zope.security.proxy import removeSecurityProxy
         factory = self._makeFactory()
@@ -256,7 +257,7 @@ class _Adapter(object):
         self.assertFalse(returned is factory)
         ploc = removeSecurityProxy(returned)
         self.assertTrue(ploc.__parent__ is adapter)
-        unwrapped = getProxiedObject(ploc)
+        unwrapped = getObject(ploc)
         self.assertTrue(unwrapped is factory)
         after = dict([(k, v) for k, v in unwrapped.__dict__.items()
                          if k not in ('_called_with', '__parent__')])
@@ -268,7 +269,7 @@ class _Adapter(object):
     def test__call__w_ILocation_w_spacesuit_w_existing_parent(self):
         from zope.interface import directlyProvides
         from zope.location import ILocation
-        from zope.proxy import getProxiedObject
+        from zope.security.proxy import getObject
         from zope.security.proxy import ProxyFactory
         from zope.security.proxy import removeSecurityProxy
         factory = self._makeFactory()
@@ -286,7 +287,7 @@ class _Adapter(object):
         self.assertFalse(returned is factory)
         ploc = removeSecurityProxy(returned)
         self.assertTrue(ploc.__parent__ is parent)
-        unwrapped = getProxiedObject(ploc)
+        unwrapped = getObject(ploc)
         self.assertTrue(unwrapped is factory)
         after = dict([(k, v) for k, v in unwrapped.__dict__.items()
                          if k not in ('_called_with', '__parent__')])

src/zope/security/tests/test_checker.py

@@ -32,8 +32,7 @@ def _callFUT(self, object, checker=None):
         return ProxyFactory(object, checker)
 
     def test_w_already_proxied_no_checker(self):
-        from zope.security._proxy import _Proxy as Proxy
-        from zope.security._proxy import getChecker
+        from zope.security.proxy import Proxy, getChecker
         obj = object()
         def _check(*x):
             pass
@@ -43,8 +42,7 @@ def _check(*x):
         self.assertTrue(getChecker(returned) is _check)
 
     def test_w_already_proxied_same_checker(self):
-        from zope.security._proxy import _Proxy as Proxy
-        from zope.security._proxy import getChecker
+        from zope.security.proxy import Proxy, getChecker
         obj = object()
         def _check(*x):
             pass
@@ -54,7 +52,7 @@ def _check(*x):
         self.assertTrue(getChecker(returned) is _check)
 
     def test_w_already_proxied_different_checker(self):
-        from zope.security._proxy import _Proxy as Proxy
+        from zope.security.proxy import Proxy
         obj = object()
         def _check(*x):
             pass
@@ -64,7 +62,7 @@ def _sneaky(*x):
         self.assertRaises(TypeError, self._callFUT, proxy, _sneaky)
 
     def test_w_explicit_checker(self):
-        from zope.security._proxy import getChecker
+        from zope.security.proxy import getChecker
         obj = object()
         def _check(*x):
             pass
@@ -78,8 +76,7 @@ def test_no_checker_no_dunder_no_select(self):
         self.assertTrue(returned is obj)
 
     def test_no_checker_w_dunder(self):
-        from zope.security._proxy import getChecker
-        from zope.security._proxy import getObject
+        from zope.security.proxy import getChecker, getObject
         _check = object() # don't use a func, due to bound method
         class _WithChecker(object):
             __Security_checker__ = _check
@@ -93,8 +90,7 @@ def test_no_checker_no_dunder_w_select(self):
         from zope.security.checker import Checker
         from zope.security.checker import _checkers
         from zope.security.checker import _clear
-        from zope.security._proxy import getChecker
-        from zope.security._proxy import getObject
+        from zope.security.proxy import getChecker, getObject
         class _Obj(object):
             pass
         obj = _Obj()
@@ -128,37 +124,37 @@ def check_setattr(self, obj, name):
         return _Checker()
 
     def test_ok(self):
-        from zope.security._proxy import _Proxy as Proxy
+        from zope.security.proxy import Proxy
         obj = object()
         proxy = Proxy(obj, self._makeChecker())
         self.assertTrue(self._callFUT(proxy, 'whatever'))
 
     def test_w_setattr_unauth(self):
         from zope.security.interfaces import Unauthorized
-        from zope.security._proxy import _Proxy as Proxy
+        from zope.security.proxy import Proxy
         obj = object()
         proxy = Proxy(obj, self._makeChecker(ch_set=Unauthorized))
         self.assertFalse(self._callFUT(proxy, 'whatever'))
 
     def test_w_setattr_forbidden_getattr_allowed(self):
         from zope.security.interfaces import ForbiddenAttribute
-        from zope.security._proxy import _Proxy as Proxy
+        from zope.security.proxy import Proxy
         obj = object()
         proxy = Proxy(obj, self._makeChecker(ch_set=ForbiddenAttribute))
         self.assertFalse(self._callFUT(proxy, 'whatever'))
 
     def test_w_setattr_forbidden_getattr_unauth(self):
         from zope.security.interfaces import ForbiddenAttribute
         from zope.security.interfaces import Unauthorized
-        from zope.security._proxy import _Proxy as Proxy
+        from zope.security.proxy import Proxy
         obj = object()
         proxy = Proxy(obj, self._makeChecker(ch_get=Unauthorized,
                                              ch_set=ForbiddenAttribute))
         self.assertFalse(self._callFUT(proxy, 'whatever'))
 
     def test_w_setattr_forbidden_getattr_forbidden(self):
         from zope.security.interfaces import ForbiddenAttribute
-        from zope.security._proxy import _Proxy as Proxy
+        from zope.security.proxy import Proxy
         obj = object()
         proxy = Proxy(obj, self._makeChecker(ch_get=ForbiddenAttribute,
                                              ch_set=ForbiddenAttribute))
@@ -179,21 +175,21 @@ def check_getattr(self, obj, name):
         return _Checker()
 
     def test_ok(self):
-        from zope.security._proxy import _Proxy as Proxy
+        from zope.security.proxy import Proxy
         obj = object()
         proxy = Proxy(obj, self._makeChecker())
         self.assertTrue(self._callFUT(proxy, 'whatever'))
 
     def test_w_getattr_unauth(self):
         from zope.security.interfaces import Unauthorized
-        from zope.security._proxy import _Proxy as Proxy
+        from zope.security.proxy import Proxy
         obj = object()
         proxy = Proxy(obj, self._makeChecker(ch_get=Unauthorized))
         self.assertFalse(self._callFUT(proxy, 'whatever'))
 
     def test_w_setattr_forbidden_getattr_allowed(self):
         from zope.security.interfaces import ForbiddenAttribute
-        from zope.security._proxy import _Proxy as Proxy
+        from zope.security.proxy import Proxy
         obj = object()
         proxy = Proxy(obj, self._makeChecker(ch_get=ForbiddenAttribute))
         self.assertRaises(ForbiddenAttribute, self._callFUT, proxy, 'whatever')
@@ -344,8 +340,7 @@ def checkPermission(self, obj, perm):
             del thread_local.interaction
 
     def test_proxy_already_proxied(self):
-        from zope.security._proxy import _Proxy as Proxy
-        from zope.security._proxy import getChecker
+        from zope.security.proxy import Proxy, getChecker
         obj = object()
         def _check(*x):
             pass
@@ -362,8 +357,7 @@ def test_proxy_no_dunder_no_select(self):
         self.assertTrue(returned is obj)
 
     def test_proxy_no_checker_w_dunder(self):
-        from zope.security._proxy import getChecker
-        from zope.security._proxy import getObject
+        from zope.security.proxy import getChecker, getObject
         _check = object() # don't use a func, due to bound method
         class _WithChecker(object):
             __Security_checker__ = _check
@@ -378,8 +372,7 @@ def test_proxy_no_checker_no_dunder_w_select(self):
         from zope.security.checker import Checker
         from zope.security.checker import _checkers
         from zope.security.checker import _clear
-        from zope.security._proxy import getChecker
-        from zope.security._proxy import getObject
+        from zope.security.proxy import getChecker, getObject
         class _Obj(object):
             pass
         obj = _Obj()
@@ -696,7 +689,7 @@ def test_w_basic_types_NoProxy(self):
                     datetime.datetime.now(),
                     datetime.date.today(),
                     datetime.datetime.now().time(),
-                    datetime.tzinfo('UTC'),
+                    datetime.tzinfo(),
                    ]:
             self.assertTrue(self._callFUT(obj) is None)
 
@@ -1588,8 +1581,7 @@ def test_proxy(self):
 
     def testLayeredProxies(self):
         #Test that a Proxy will not be re-proxied.
-        from zope.proxy import getProxiedObject
-        from zope.security.proxy import Proxy
+        from zope.security.proxy import Proxy, getObject
         from zope.security.checker import Checker
         from zope.security.checker import NamesChecker
         class Base:
@@ -1600,12 +1592,12 @@ class Base:
         # base is not proxied, so we expect a proxy
         proxy1 = checker.proxy(base)
         self.assertTrue(type(proxy1) is Proxy)
-        self.assertTrue(getProxiedObject(proxy1) is base)
+        self.assertTrue(getObject(proxy1) is base)
 
         # proxy is a proxy, so we don't expect to get another
         proxy2 = checker.proxy(proxy1)
         self.assertTrue(proxy2 is proxy1)
-        self.assertTrue(getProxiedObject(proxy2) is base)
+        self.assertTrue(getObject(proxy2) is base)
 
 
     def testMultiChecker(self):