self-protect against broken Mapping implementations (getsentry#306)

crepererum · untitaker · commit 812200532f36 · 2019-03-27T20:27:50.000+01:00
diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py
@@ -349,39 +349,43 @@ def safe_repr(value):
 
 
 def object_to_json(obj, remaining_depth=4, memo=None):
-    if memo is None:
-        memo = Memo()
-    if memo.memoize(obj):
-        return CYCLE_MARKER
+    with capture_internal_exceptions():
+        if memo is None:
+            memo = Memo()
+        if memo.memoize(obj):
+            return CYCLE_MARKER
 
-    try:
-        if remaining_depth > 0:
-            hints = {"memo": memo, "remaining_depth": remaining_depth}
-            for processor in global_repr_processors:
-                with capture_internal_exceptions():
-                    result = processor(obj, hints)
-                    if result is not NotImplemented:
-                        return result
-
-            if isinstance(obj, (list, tuple)):
-                # It is not safe to iterate over another sequence types as this may raise errors or
-                # bring undesired side-effects (e.g. Django querysets are executed during iteration)
-                return [
-                    object_to_json(x, remaining_depth=remaining_depth - 1, memo=memo)
-                    for x in obj
-                ]
-
-            if isinstance(obj, Mapping):
-                return {
-                    safe_str(k): object_to_json(
-                        v, remaining_depth=remaining_depth - 1, memo=memo
-                    )
-                    for k, v in list(obj.items())
-                }
-
-        return safe_repr(obj)
-    finally:
-        memo.unmemoize(obj)
+        try:
+            if remaining_depth > 0:
+                hints = {"memo": memo, "remaining_depth": remaining_depth}
+                for processor in global_repr_processors:
+                    with capture_internal_exceptions():
+                        result = processor(obj, hints)
+                        if result is not NotImplemented:
+                            return result
+
+                if isinstance(obj, (list, tuple)):
+                    # It is not safe to iterate over another sequence types as this may raise errors or
+                    # bring undesired side-effects (e.g. Django querysets are executed during iteration)
+                    return [
+                        object_to_json(
+                            x, remaining_depth=remaining_depth - 1, memo=memo
+                        )
+                        for x in obj
+                    ]
+
+                if isinstance(obj, Mapping):
+                    return {
+                        safe_str(k): object_to_json(
+                            v, remaining_depth=remaining_depth - 1, memo=memo
+                        )
+                        for k, v in list(obj.items())
+                    }
+
+            return safe_repr(obj)
+        finally:
+            memo.unmemoize(obj)
+    return u"<broken repr>"
 
 
 def extract_locals(frame):
diff --git a/tests/test_client.py b/tests/test_client.py
@@ -18,9 +18,18 @@
 )
 from sentry_sdk.hub import HubMeta
 from sentry_sdk.transport import Transport
-from sentry_sdk._compat import reraise, text_type
+from sentry_sdk._compat import reraise, text_type, PY2
 from sentry_sdk.utils import HAS_CHAINED_EXCEPTIONS
 
+if PY2:
+    # Importing ABCs from collections is deprecated, and will stop working in 3.8
+    # https://github.com/python/cpython/blob/master/Lib/collections/__init__.py#L49
+    from collections import Mapping
+else:
+    # New in 3.3
+    # https://docs.python.org/3/library/collections.abc.html
+    from collections.abc import Mapping
+
 
 class EventCaptured(Exception):
     pass
@@ -404,3 +413,34 @@ def test_chained_exceptions(sentry_init, capture_events):
     event, = events
 
     assert len(event["exception"]["values"]) == 2
+
+
+@pytest.mark.tests_internal_exceptions
+def test_broken_mapping(sentry_init, capture_events):
+    sentry_init()
+    events = capture_events()
+
+    class C(Mapping):
+        def broken(self, *args, **kwargs):
+            raise Exception("broken")
+
+        __getitem__ = broken
+        __setitem__ = broken
+        __delitem__ = broken
+        __iter__ = broken
+        __len__ = broken
+
+        def __repr__(self):
+            return "broken"
+
+    try:
+        a = C()  # noqa
+        1 / 0
+    except Exception:
+        capture_exception()
+
+    event, = events
+    assert (
+        event["exception"]["values"][0]["stacktrace"]["frames"][0]["vars"]["a"]
+        == "<broken repr>"
+    )
