Merge pull request #11 from NextThought/pypy-support

Compatibility with zope.proxy 4.1.5 under PyPy.

3 files changed, 60 insertions, 15 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 76a6154..71bfffc 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -4,7 +4,10 @@ Changes
 4.0.2 (unreleased)
 ------------------
 
-- TBD
+- Fixed compatibility with ``zope.proxy`` 4.1.5 under PyPy.
+
+- Fixed the very first call to ``removeSecurityProxy`` returning
+  incorrect results if given a proxy under PyPy.
 
 4.0.1 (2014-03-19)
 ------------------
diff --git a/src/zope/security/proxy.py b/src/zope/security/proxy.py
index 16bb95f..f023ca3 100644
--- a/src/zope/security/proxy.py
+++ b/src/zope/security/proxy.py
@@ -84,11 +84,34 @@ class ProxyPy(PyProxyBase):
         checker = super(PyProxyBase, self).__getattribute__('_checker')
         if name == '_checker':
             return checker
-        if name not in ['__cmp__', '__hash__', '__bool__', '__nonzero__',
+        if name not in ('__cmp__', '__hash__', '__bool__', '__nonzero__',
                         '__lt__', '__le__', '__eq__', '__ne__', '__ge__',
-                        '__gt__']:
+                        '__gt__'):
             checker.check_getattr(wrapped, name)
-        return checker.proxy(super(ProxyPy, self).__getattribute__(name))
+        if name in ('__reduce__', '__reduce_ex__'):
+            # The superclass specifically denies access to __reduce__
+            # and __reduce__ex__, not letting proxies be pickled. But
+            # for backwards compatibility, we need to be able to
+            # pickle proxies. See checker:Global for an example.
+            val = getattr(wrapped, name)
+        elif name == '__module__':
+            # The superclass deals with descriptors found in the type
+            # of this object just like the Python language spec states, letting
+            # them have precedence over things found in the instance. This
+            # normally makes us a better proxy implementation. However, the
+            # C version of this code in _proxy doesn't take that same care and instead
+            # uses the generic object attribute access methods directly on
+            # the wrapped object. This is a behaviour difference; so far, it's
+            # only been noticed for the __module__ attribute, which checker:Global
+            # wants to override but couldn't because this object's type's __module__ would
+            # get in the way. That broke pickling, and checker:Global can't return
+            # anything more sophisticated than a str (a tuple) because it gets proxied
+            # and breaks pickling again. Our solution is to match the C version for this
+            # one attribute.
+            val = getattr(wrapped, name)
+        else:
+            val = super(ProxyPy, self).__getattribute__(name)
+        return checker.proxy(val)
 
     def __getattr__(self, name):
         wrapped = super(PyProxyBase, self).__getattribute__('_wrapped')
@@ -292,9 +315,13 @@ for name in ['__iadd__',
 def getCheckerPy(proxy):
     return super(PyProxyBase, proxy).__getattribute__('_checker')
 
+if PYPY:
+    _builtin_isinstance = __builtins__.isinstance
+else:
+    _builtin_isinstance = __builtins__['isinstance']
+
 def getObjectPy(proxy):
-    isinstance_func = builtin_isinstance or isinstance
-    if not isinstance_func(proxy, ProxyPy):
+    if not _builtin_isinstance(proxy, ProxyPy):
         return proxy
     return super(PyProxyBase, proxy).__getattribute__('_wrapped')
 
@@ -318,18 +345,11 @@ def getTestProxyItems(proxy):
     return sorted(checker.get_permissions.items())
 
 
-builtin_isinstance = None
 def isinstance(object, cls):
     """Test whether an object is an instance of a type.
 
-    This works even if the object is security proxied:
+    This works even if the object is security proxied.
     """
-    global builtin_isinstance
-    if builtin_isinstance is None:
-        if PYPY:
-            builtin_isinstance = getattr(__builtins__, 'isinstance')
-        else:
-            builtin_isinstance = __builtins__['isinstance']
     # The removeSecurityProxy call is OK here because it is *only*
     # being used for isinstance
-    return builtin_isinstance(removeSecurityProxy(object), cls)
+    return _builtin_isinstance(removeSecurityProxy(object), cls)
diff --git a/src/zope/security/tests/test_proxy.py b/src/zope/security/tests/test_proxy.py
index fbe4964..aaed8e6 100644
--- a/src/zope/security/tests/test_proxy.py
+++ b/src/zope/security/tests/test_proxy.py
@@ -1389,6 +1389,28 @@ class ProxyPyTests(unittest.TestCase, ProxyTestBase):
         # pow(i, j, proxy(k)) will fail with a TypeError
         self.assertRaises(TypeError, pow, (x, y, proxy))
 
+    def test_getObjectPy_initial_conditions(self):
+        # Once upon a time, we dynamically set _builtin_isinstance
+        # in z.s.proxy.isinstance itself. And at that time getObjectPy
+        # (aka removeSecurityProxy) called z.s.proxy.isinstance if
+        # _builtin_isinstance was not set...which recursively calls
+        # getObjectPy. The net result was that the very first call
+        # to getObjectPy would falsely return the proxy object if passed
+        # a proxy, not the wrapped object!
+        # This test makes sure we're not dynamically setting that attribute
+        # any more.
+        import zope.security.proxy
+
+        target = object()
+        checker = object()
+        proxy = self._makeOne(target, checker)
+
+        orig_builtin_isinstance = zope.security.proxy._builtin_isinstance
+        zope.security.proxy._builtin_isinstance = None
+        try:
+            self.assertRaises(TypeError, zope.security.proxy.getObjectPy, proxy)
+        finally:
+            zope.security.proxy._builtin_isinstance = orig_builtin_isinstance
 
 class DummyChecker(object):
     _proxied = _checked = None