Make C's __providedBy__ stop ignoring all errors and catch only AttributeError.issue239

Fixes #239

There was a similar bug in the Python side where it would ignore a __provides__ of None, unlike the C implementation.
I documented this in the code but not the CHANGES.rst because I can't imagine anyone relying on that.

4 files changed, 77 insertions, 10 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 600fbb8..0cfdab7 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -5,6 +5,13 @@
 5.4.0 (unreleased)
 ==================
 
+- Make the C implementation of the ``__providedBy__`` descriptor stop
+  ignoring all errors raised when accessing the instance's
+  ``__provides__``. Now it behaves like the Python version and only
+  catches ``AttributeError``. The previous behaviour could lead to
+  crashing the interpreter in cases of recursion and errors. See
+  `issue 239 <https://github.com/zopefoundation/zope.interface/issues>`_.
+
 - Update the ``repr()`` and ``str()`` of various objects to be shorter
   and more informative. In many cases, the ``repr()`` is now something
   that can be evaluated to produce an equal object. For example, what
diff --git a/src/zope/interface/_zope_interface_coptimizations.c b/src/zope/interface/_zope_interface_coptimizations.c
index 374311e..0b0713e 100644
--- a/src/zope/interface/_zope_interface_coptimizations.c
+++ b/src/zope/interface/_zope_interface_coptimizations.c
@@ -526,8 +526,10 @@ OSD_descr_get(PyObject *self, PyObject *inst, PyObject *cls)
     return getObjectSpecification(NULL, cls);
 
   provides = PyObject_GetAttr(inst, str__provides__);
-  if (provides != NULL)
+  /* Return __provides__ if we got it, or return NULL and propagate non-AttributeError. */
+  if (provides != NULL || !PyErr_ExceptionMatches(PyExc_AttributeError))
     return provides;
+
   PyErr_Clear();
   return implementedBy(NULL, cls);
 }
diff --git a/src/zope/interface/declarations.py b/src/zope/interface/declarations.py
index 935b026..0289ddc 100644
--- a/src/zope/interface/declarations.py
+++ b/src/zope/interface/declarations.py
@@ -1243,10 +1243,19 @@ def providedBy(ob):
 
 @_use_c_impl
 class ObjectSpecificationDescriptor(object):
-    """Implement the `__providedBy__` attribute
-
-    The `__providedBy__` attribute computes the interfaces provided by
-    an object.
+    """Implement the ``__providedBy__`` attribute
+
+    The ``__providedBy__`` attribute computes the interfaces provided by
+    an object. If an object has an ``__provides__`` attribute
+
+    .. versionchanged:: 5.4.0
+       Both the default (C) implementation and the Python implementation
+       now let exceptions raised by accessing ``__provides__`` propagate.
+       Previously, the C version ignored all exceptions.
+    .. versionchanged:: 5.4.0
+       The Python implementation now matches the C implementation and lets
+       a ``__provides__`` of ``None`` override what the class is declared to
+       implement.
     """
 
     def __get__(self, inst, cls):
@@ -1255,11 +1264,10 @@ class ObjectSpecificationDescriptor(object):
         if inst is None:
             return getObjectSpecification(cls)
 
-        provides = getattr(inst, '__provides__', None)
-        if provides is not None:
-            return provides
-
-        return implementedBy(cls)
+        try:
+            return inst.__provides__
+        except AttributeError:
+            return implementedBy(cls)
 
 
 ##############################################################################
diff --git a/src/zope/interface/tests/test_declarations.py b/src/zope/interface/tests/test_declarations.py
index 8efe2d8..eb8b2a7 100644
--- a/src/zope/interface/tests/test_declarations.py
+++ b/src/zope/interface/tests/test_declarations.py
@@ -2531,6 +2531,56 @@ class ObjectSpecificationDescriptorFallbackTests(unittest.TestCase):
         directlyProvides(foo, IBaz)
         self.assertEqual(list(foo.__providedBy__), [IBaz, IFoo])
 
+    def test_arbitrary_exception_accessing_provides_not_caught(self):
+
+        class MyException(Exception):
+            pass
+
+        class Foo(object):
+            __providedBy__ = self._makeOne()
+
+            @property
+            def __provides__(self):
+                raise MyException
+
+        foo = Foo()
+        with self.assertRaises(MyException):
+            getattr(foo, '__providedBy__')
+
+    def test_AttributeError_accessing_provides_caught(self):
+
+        class MyException(Exception):
+            pass
+
+        class Foo(object):
+            __providedBy__ = self._makeOne()
+
+            @property
+            def __provides__(self):
+                raise AttributeError
+
+        foo = Foo()
+        provided = getattr(foo, '__providedBy__')
+        self.assertIsNotNone(provided)
+
+    def test_None_in__provides__overrides(self):
+        from zope.interface import Interface
+        from zope.interface import implementer
+
+        class IFoo(Interface):
+            pass
+
+        @implementer(IFoo)
+        class Foo(object):
+
+            @property
+            def __provides__(self):
+                return None
+
+        Foo.__providedBy__ = self._makeOne()
+
+        provided = getattr(Foo(), '__providedBy__')
+        self.assertIsNone(provided)
 
 class ObjectSpecificationDescriptorTests(
         ObjectSpecificationDescriptorFallbackTests,