diff --git a/CHANGELOG.md b/CHANGELOG.md index b7a2b54f8f7..a23a3453901 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ - Make the tombstone merge time threshold configurable via `SentryAndroidOptions.setTombstoneMergeTimeThresholdMillis` and the `io.sentry.tombstone.merge-time-threshold-millis` manifest option ([#6154](https://github.com/getsentry/sentry-java/pull/6154)) - Add `sentry-apollo-5` integration for Apollo Kotlin 5, providing HTTP tracing and failed GraphQL request reporting ([#6074](https://github.com/getsentry/sentry-java/pull/6074)) +### Fixes + +- Prevent infinite loops when capturing exceptions with cyclic cause chains ([#6073](https://github.com/getsentry/sentry-java/pull/6073)) + ### Improvements - Recover Android 17 `MemoryLimiter` app exits recorded as `ApplicationExitInfo.REASON_MEMORY_LIMITER` ([#6174](https://github.com/getsentry/sentry-java/pull/6174)) diff --git a/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java b/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java index fc178d789f3..b46eb91746b 100644 --- a/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java +++ b/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java @@ -2,8 +2,10 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.IdentityHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.WeakHashMap; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -54,10 +56,12 @@ private static boolean containsAnyKey( private static @NotNull List allCauses(final @NotNull Throwable throwable) { final List causes = new ArrayList<>(); - Throwable ex = throwable; - while (ex.getCause() != null) { - causes.add(ex.getCause()); - ex = ex.getCause(); + final Set visited = Collections.newSetFromMap(new IdentityHashMap<>()); + visited.add(throwable); + Throwable cause = throwable.getCause(); + while (cause != null && visited.add(cause)) { + causes.add(cause); + cause = cause.getCause(); } return causes; } diff --git a/sentry/src/main/java/io/sentry/util/ExceptionUtils.java b/sentry/src/main/java/io/sentry/util/ExceptionUtils.java index 329bc84ba81..c17470bfac3 100644 --- a/sentry/src/main/java/io/sentry/util/ExceptionUtils.java +++ b/sentry/src/main/java/io/sentry/util/ExceptionUtils.java @@ -1,5 +1,7 @@ package io.sentry.util; +import java.util.Collections; +import java.util.IdentityHashMap; import java.util.Set; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -16,8 +18,12 @@ public final class ExceptionUtils { public static @NotNull Throwable findRootCause(final @NotNull Throwable throwable) { Objects.requireNonNull(throwable, "throwable cannot be null"); Throwable rootCause = throwable; - while (rootCause.getCause() != null && rootCause.getCause() != rootCause) { - rootCause = rootCause.getCause(); + final Set visited = Collections.newSetFromMap(new IdentityHashMap<>()); + visited.add(rootCause); + Throwable cause = rootCause.getCause(); + while (cause != null && visited.add(cause)) { + rootCause = cause; + cause = rootCause.getCause(); } return rootCause; } diff --git a/sentry/src/test/java/io/sentry/DuplicateEventDetectionEventProcessorTest.kt b/sentry/src/test/java/io/sentry/DuplicateEventDetectionEventProcessorTest.kt index 3e4141dfeee..8fb929224ea 100644 --- a/sentry/src/test/java/io/sentry/DuplicateEventDetectionEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/DuplicateEventDetectionEventProcessorTest.kt @@ -8,6 +8,21 @@ import kotlin.test.assertNotNull import kotlin.test.assertNull class DuplicateEventDetectionEventProcessorTest { + private class CircularCauseThrowable : RuntimeException() { + private var nextCause: Throwable? = null + private var causeReads = 0 + + fun linkTo(cause: Throwable) { + nextCause = cause + } + + override val cause: Throwable? + get() { + check(causeReads++ < 10) { "Throwable cause cycle was not detected" } + return nextCause + } + } + class Fixture { fun getSut(enableDeduplication: Boolean? = null): DuplicateEventDetectionEventProcessor { val options = @@ -99,6 +114,19 @@ class DuplicateEventDetectionEventProcessorTest { assertNull(result) } + @Test + fun `does not loop indefinitely for cyclic cause chain`() { + val processor = fixture.getSut() + val first = CircularCauseThrowable() + val second = CircularCauseThrowable() + first.linkTo(second) + second.linkTo(first) + + val result = processor.process(SentryEvent(first), Hint()) + + assertNotNull(result) + } + @Test fun `does not deduplicate is deduplication is disabled`() { val processor = fixture.getSut(enableDeduplication = false) diff --git a/sentry/src/test/java/io/sentry/util/ExceptionUtilsTest.kt b/sentry/src/test/java/io/sentry/util/ExceptionUtilsTest.kt index 13f2cffc142..385fe92dbae 100644 --- a/sentry/src/test/java/io/sentry/util/ExceptionUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/ExceptionUtilsTest.kt @@ -76,4 +76,29 @@ class ExceptionUtilsTest { ExceptionUtils.rethrowIfFatal(RuntimeException()) assertFalse(Thread.currentThread().isInterrupted) } + + @Test + fun `does not loop indefinitely for cyclic cause chain`() { + val first = CircularCauseThrowable() + val second = CircularCauseThrowable() + first.linkTo(second) + second.linkTo(first) + + assertThat(ExceptionUtils.findRootCause(first)).isSameInstanceAs(second) + } + + private class CircularCauseThrowable : RuntimeException() { + private var nextCause: Throwable? = null + private var causeReads = 0 + + fun linkTo(cause: Throwable) { + nextCause = cause + } + + override val cause: Throwable? + get() { + check(causeReads++ < 10) { "Throwable cause cycle was not detected" } + return nextCause + } + } }