diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 4586ac5d5e..0d8c209239 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -553,11 +553,47 @@ def safe_str(value: "Any") -> str: return safe_repr(value) -def safe_repr(value: "Any") -> str: +#: Hard upper bound on the number of characters we will materialize from a +#: single object's repr before truncating. This exists so that an object whose +#: ``__repr__`` happens to walk a large graph (eg. a FastAPI ``_IncludedRouter`` +#: traversing nested routers, or any domain object with an expensive repr) +#: cannot pin an event loop for seconds while we build a string we will +#: immediately truncate anyway (issue #6649). +#: +#: The downstream serializer still truncates to ``max_value_length`` (default +#: 1024 chars), so values larger than that bound are already discarded; the +#: issue is just the CPU + memory cost of building them. Keeping this cap +#: comfortably above the default ``max_value_length`` means normal values are +#: unaffected, and only the pathological "repr materialises hundreds of MiB" +#: case is cut short. +SAFE_REPR_MAX_CHARS = 100_000 + + +def safe_repr(value: "Any", max_chars: "Optional[int]" = SAFE_REPR_MAX_CHARS) -> str: + """Return ``repr(value)``, with belt-and-braces guards so a hostile + ``__repr__`` cannot: + + 1. Raise through us (returns ``""`` instead). + 2. Burn seconds / gigabytes materialising a huge string that the serializer + is about to truncate anyway. If ``max_chars`` is not ``None`` and the + repr grows past it, we stop pulling characters as soon as the cap is + exceeded and append a truncation marker. Passing ``max_chars=None`` + preserves the original unbounded behaviour. + + See :data:`SAFE_REPR_MAX_CHARS` for the rationale. + """ + if max_chars is None: + try: + return repr(value) + except Exception: + return "" try: - return repr(value) + full = repr(value) except Exception: return "" + if len(full) <= max_chars: + return full + return full[:max_chars] + "..." def filename_for_module( diff --git a/tests/utils/test_general.py b/tests/utils/test_general.py index ed0df8b3b5..8bcf2a1c2f 100644 --- a/tests/utils/test_general.py +++ b/tests/utils/test_general.py @@ -610,3 +610,56 @@ def model_dump(self): # Test with dict containing class result = _normalize_data({"schema": TestClass, "count": 5}) assert result == {"schema": "", "count": 5} + + +def test_safe_repr_truncates_pathological_repr(): + """Regression for #6649 — safe_repr must not spend seconds / megabytes + materialising a huge repr. A hostile __repr__ returning 10 MiB of + characters must be bounded by SAFE_REPR_MAX_CHARS. + """ + from sentry_sdk.utils import SAFE_REPR_MAX_CHARS + + class _Pathological: + """Mimics FastAPI _IncludedRouter: a repr that materialises a huge + string (~10 MiB). The real-world value was ~117 MiB per router per + frame, with 8 nested frames per request — ~15-20s event-loop stall. + """ + + def __repr__(self): + return "x" * (10 * 1024 * 1024) # 10 MiB + + result = safe_repr(_Pathological()) + assert len(result) <= SAFE_REPR_MAX_CHARS + len("...") + assert result.endswith("...") + + +def test_safe_repr_small_repr_unchanged(): + """Values whose repr fits comfortably within the cap must be returned + exactly as before (no truncation marker, no behaviour change).""" + assert safe_repr(123) == "123" + assert safe_repr("hello") == "'hello'" + assert safe_repr([1, 2, 3]) == "[1, 2, 3]" + + +def test_safe_repr_opt_in_unbounded(): + """Callers that explicitly want the historical unbounded behaviour + (eg. tests, or callers that supply their own downstream cap) can pass + max_chars=None to disable the guard.""" + from sentry_sdk.utils import SAFE_REPR_MAX_CHARS + + big = "y" * (SAFE_REPR_MAX_CHARS + 1000) + result = safe_repr(big, max_chars=None) + # repr adds surrounding quotes, so len is >= original length + assert len(result) >= len(big) + assert "" not in result + + +def test_safe_repr_broken_still_graceful(): + """A __repr__ that raises must still return the historical sentinel + rather than propagating.""" + + class _Broken: + def __repr__(self): + raise RuntimeError("boom") + + assert safe_repr(_Broken()) == ""