diff --git a/.idea/dictionaries/usernames.xml b/.idea/dictionaries/usernames.xml index 55dafdb672bf..24e5841bf6f9 100644 --- a/.idea/dictionaries/usernames.xml +++ b/.idea/dictionaries/usernames.xml @@ -83,6 +83,7 @@ quiro91 tarekkma vianey + voczi zanki diff --git a/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ACRATest.kt b/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ACRATest.kt index ccfe5039430d..1719e7e7839f 100644 --- a/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ACRATest.kt +++ b/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ACRATest.kt @@ -27,8 +27,10 @@ import com.ichi2.anki.CrashReportService import com.ichi2.anki.CrashReportService.FEEDBACK_REPORT_ALWAYS import com.ichi2.anki.CrashReportService.FEEDBACK_REPORT_ASK import com.ichi2.anki.R +import com.ichi2.anki.analytics.UsageAnalytics import com.ichi2.anki.logging.ProductionCrashReportingTree import com.ichi2.anki.preferences.sharedPrefs +import com.ichi2.anki.servicelayer.ThrowableFilterService import com.ichi2.anki.testutil.GrantStoragePermission import org.acra.ACRA import org.acra.builder.ReportBuilder @@ -230,6 +232,37 @@ class ACRATest : InstrumentedTest() { assertToastMessage(R.string.feedback_auto_toast_text) } + @Test + fun verifyExceptionHandlerChain() { + // contains assumptions about ordering in ACRA, ThrowableFilter and UsageAnalytics + // making sure they are correct is vital though, so we will accept the need to change + // this test if you re-order them + var firstExceptionHandler = Thread.getDefaultUncaughtExceptionHandler() + assertThat("First handler is ThrowableFilterService", firstExceptionHandler is ThrowableFilterService.FilteringExceptionHandler) + ThrowableFilterService.unInstallDefaultExceptionHandler() + var secondExceptionHandler = Thread.getDefaultUncaughtExceptionHandler() + assertThat("Second handler is AnalyticsLoggingExceptionHandler", secondExceptionHandler is UsageAnalytics.AnalyticsLoggingExceptionHandler) + UsageAnalytics.unInstallDefaultExceptionHandler() + var thirdExceptionHandler = Thread.getDefaultUncaughtExceptionHandler() + assertThat("Third handler is neither Analytics nor ThrowableFilter", thirdExceptionHandler !is UsageAnalytics.AnalyticsLoggingExceptionHandler && thirdExceptionHandler !is ThrowableFilterService.FilteringExceptionHandler) + + // chain them again + UsageAnalytics.installDefaultExceptionHandler() + ThrowableFilterService.installDefaultExceptionHandler() + + // reinitialize things and make sure they came through correctly again + CrashReportService.onPreferenceChanged(app!!.applicationContext, FEEDBACK_REPORT_ASK) + firstExceptionHandler = Thread.getDefaultUncaughtExceptionHandler() + assertThat("First handler is ThrowableFilterService", firstExceptionHandler is ThrowableFilterService.FilteringExceptionHandler) + ThrowableFilterService.unInstallDefaultExceptionHandler() + secondExceptionHandler = Thread.getDefaultUncaughtExceptionHandler() + Timber.i("Second handler is a %s", secondExceptionHandler) + assertThat("Second handler is AnalyticsLoggingExceptionHandler", secondExceptionHandler is UsageAnalytics.AnalyticsLoggingExceptionHandler) + UsageAnalytics.unInstallDefaultExceptionHandler() + thirdExceptionHandler = Thread.getDefaultUncaughtExceptionHandler() + assertThat("Third handler is neither Analytics nor ThrowableFilter", thirdExceptionHandler !is UsageAnalytics.AnalyticsLoggingExceptionHandler && thirdExceptionHandler !is ThrowableFilterService.FilteringExceptionHandler) + } + private fun setAcraReportingMode(feedbackReportAlways: String) { CrashReportService.setAcraReportingMode(feedbackReportAlways) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt index bf91fa6f3ac3..f1a250f8a87b 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt @@ -49,6 +49,7 @@ import com.ichi2.anki.logging.RobolectricDebugTree import com.ichi2.anki.preferences.SharedPreferencesProvider import com.ichi2.anki.preferences.sharedPrefs import com.ichi2.anki.servicelayer.DebugInfoService +import com.ichi2.anki.servicelayer.ThrowableFilterService import com.ichi2.anki.services.BootService import com.ichi2.anki.services.NotificationService import com.ichi2.anki.ui.dialogs.ActivityAgnosticDialogs @@ -146,6 +147,9 @@ open class AnkiDroidApp : Application(), Configuration.Provider, ChangeManager.S UsageAnalytics.setDryRun(true) } + // Last in the UncaughtExceptionHandlers chain is our filter service + ThrowableFilterService.initialize() + applicationScope.launch { Timber.i(DebugInfoService.getDebugInfo(this@AnkiDroidApp)) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/CrashReportService.kt b/AnkiDroid/src/main/java/com/ichi2/anki/CrashReportService.kt index 0cc97f963b53..22f807dbfeea 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/CrashReportService.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/CrashReportService.kt @@ -29,6 +29,7 @@ import com.ichi2.anki.analytics.UsageAnalytics.sendAnalyticsException import com.ichi2.anki.exception.ManuallyReportedException import com.ichi2.anki.exception.UserSubmittedException import com.ichi2.anki.preferences.sharedPrefs +import com.ichi2.anki.servicelayer.ThrowableFilterService import com.ichi2.libanki.utils.TimeManager import com.ichi2.utils.WebViewDebugging.setDataDirectorySuffix import org.acra.ACRA @@ -281,6 +282,8 @@ object CrashReportService { } } if (FEEDBACK_REPORT_NEVER != reportMode) { + if (ThrowableFilterService.shouldDiscardThrowable(e)) return + ACRA.errorReporter.putCustomData("origin", origin ?: "") ACRA.errorReporter.putCustomData("additionalInfo", additionalInfo ?: "") ACRA.errorReporter.handleException(e) @@ -319,6 +322,7 @@ object CrashReportService { deleteACRALimiterData(ctx) // We also need to re-chain our UncaughtExceptionHandlers UsageAnalytics.reInitialize() + ThrowableFilterService.reInitialize() } /** diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/analytics/UsageAnalytics.kt b/AnkiDroid/src/main/java/com/ichi2/anki/analytics/UsageAnalytics.kt index ccacce170a00..91c9f5581634 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/analytics/UsageAnalytics.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/analytics/UsageAnalytics.kt @@ -48,6 +48,19 @@ object UsageAnalytics { private var sAnalyticsTrackingId: String? = null private var sAnalyticsSamplePercentage = -1 + @FunctionalInterface + fun interface AnalyticsLoggingExceptionHandler : Thread.UncaughtExceptionHandler + + var uncaughtExceptionHandler = AnalyticsLoggingExceptionHandler { + thread: Thread?, throwable: Throwable -> + sendAnalyticsException(throwable, true) + if (thread == null) { + Timber.w("unexpected: thread was null") + return@AnalyticsLoggingExceptionHandler + } + sOriginalUncaughtExceptionHandler!!.uncaughtException(thread, throwable) + } + /** * Initialize the analytics provider - must be called prior to sending anything. * Usage after that is static @@ -118,23 +131,19 @@ object UsageAnalytics { * We want to send an analytics hit on any exception, then chain to other handlers (e.g., ACRA) */ @Synchronized - private fun installDefaultExceptionHandler() { + @VisibleForTesting + fun installDefaultExceptionHandler() { sOriginalUncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler() - Thread.setDefaultUncaughtExceptionHandler { thread: Thread?, throwable: Throwable -> - sendAnalyticsException(throwable, true) - if (thread == null) { - Timber.w("unexpected: thread was null") - return@setDefaultUncaughtExceptionHandler - } - sOriginalUncaughtExceptionHandler!!.uncaughtException(thread, throwable) - } + Timber.d("Chaining to uncaughtExceptionHandler (%s)", sOriginalUncaughtExceptionHandler) + Thread.setDefaultUncaughtExceptionHandler(uncaughtExceptionHandler) } /** * Reset the default exception handler */ @Synchronized - private fun unInstallDefaultExceptionHandler() { + @VisibleForTesting + fun unInstallDefaultExceptionHandler() { Thread.setDefaultUncaughtExceptionHandler(sOriginalUncaughtExceptionHandler) sOriginalUncaughtExceptionHandler = null } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/servicelayer/ThrowableFilterService.kt b/AnkiDroid/src/main/java/com/ichi2/anki/servicelayer/ThrowableFilterService.kt new file mode 100644 index 000000000000..f37b3bd994a3 --- /dev/null +++ b/AnkiDroid/src/main/java/com/ichi2/anki/servicelayer/ThrowableFilterService.kt @@ -0,0 +1,118 @@ +/**************************************************************************************** + * Copyright (c) 2024 voczi + * Copyright (c) 2024 Mike Hardy * + * * + * This program is free software; you can redistribute it and/or modify it under * + * the terms of the GNU General Public License as published by the Free Software * + * Foundation; either version 3 of the License, or (at your option) any later * + * version. * + * * + * This program is distributed in the hope that it will be useful, but WITHOUT ANY * + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A * + * PARTICULAR PURPOSE. See the GNU General Public License for more details. * + * * + * You should have received a copy of the GNU General Public License along with * + * this program. If not, see . * + ****************************************************************************************/ +package com.ichi2.anki.servicelayer + +import androidx.annotation.VisibleForTesting +import com.ichi2.anki.exception.StorageAccessException +import net.ankiweb.rsdroid.exceptions.BackendNetworkException +import net.ankiweb.rsdroid.exceptions.BackendSyncException +import net.ankiweb.rsdroid.exceptions.BackendSyncException.BackendSyncServerMessageException +import timber.log.Timber + +object ThrowableFilterService { + + @FunctionalInterface + fun interface FilteringExceptionHandler : Thread.UncaughtExceptionHandler + + @VisibleForTesting + var originalUncaughtExceptionHandler: Thread.UncaughtExceptionHandler? = null + + var uncaughtExceptionHandler = FilteringExceptionHandler { + thread: Thread?, throwable: Throwable -> + if (thread == null) { + Timber.w("unexpected: thread was null") + return@FilteringExceptionHandler + } + if (shouldDiscardThrowable(throwable)) { + Timber.i("discarding throwable") + return@FilteringExceptionHandler + } + originalUncaughtExceptionHandler?.uncaughtException(thread, throwable) + } + + fun initialize() { + Timber.i("initialize()") + installDefaultExceptionHandler() + } + + /** + * We want to filter any exceptions with PII, then chain to other handlers + */ + @Synchronized + @VisibleForTesting + fun installDefaultExceptionHandler() { + originalUncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler() + Timber.d("Chaining to uncaughtExceptionHandler (%s)", originalUncaughtExceptionHandler) + Thread.setDefaultUncaughtExceptionHandler(uncaughtExceptionHandler) + } + + /** + * Reset the default exception handler + */ + @Synchronized + @VisibleForTesting + fun unInstallDefaultExceptionHandler() { + Thread.setDefaultUncaughtExceptionHandler(originalUncaughtExceptionHandler) + originalUncaughtExceptionHandler = null + } + + /** + * Re-Initialize the throwable filter + */ + @Synchronized + fun reInitialize() { + // send any pending async hits, re-chain default exception handlers and re-init + Timber.i("reInitialize()") + unInstallDefaultExceptionHandler() + initialize() + } + + fun shouldDiscardThrowable(t: Throwable): Boolean { + // Note that an exception may have a nested BackendSyncException, + // so we check if it is safe from PII despite also filtering by type + return exceptionIsUnwanted(t) || !t.safeFromPII() + } + + // There are few exception types that are common, but are unwanted in + // our analytics or crash report service because they are not actionable + fun exceptionIsUnwanted(t: Throwable): Boolean { + Timber.v("exceptionIsUnwanted - examining %s", t.javaClass.simpleName) + when (t) { + is BackendNetworkException -> return true + is BackendSyncException -> return true + is StorageAccessException -> return true + } + Timber.v("exceptionIsUnwanted - exception was wanted") + return false + } + + /** + * Checks if the [Throwable] is safe from Personally Identifiable Information (PII) + * @return `false` if the [Throwable] contains PII, otherwise `true` + */ + fun Throwable.safeFromPII(): Boolean { + if (this.containsPIINonRecursive()) return false + return this.cause?.safeFromPII() != false + } + + private fun Throwable.containsPIINonRecursive(): Boolean { + // BackendSyncServerMessage may contain PII and we do not want this leaked to ACRA. + // Related: https://github.com/ankidroid/Anki-Android/issues/17392 + // and also https://github.com/ankitects/anki/commit/ba1f5f4 + return this is BackendSyncServerMessageException + } +} diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/ThrowableFilterServiceTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/ThrowableFilterServiceTest.kt new file mode 100644 index 000000000000..2c02c52d73ad --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/anki/ThrowableFilterServiceTest.kt @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2024 voczi + * + * This program is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free Software + * Foundation; either version 3 of the License, or (at your option) any later + * version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT ANY + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A + * PARTICULAR PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + */ + +package com.ichi2.anki + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import anki.backend.BackendError +import com.ichi2.anki.exception.StorageAccessException +import com.ichi2.anki.servicelayer.ThrowableFilterService +import com.ichi2.anki.servicelayer.ThrowableFilterService.safeFromPII +import com.ichi2.testutils.JvmTest +import net.ankiweb.rsdroid.exceptions.BackendDeckIsFilteredException +import net.ankiweb.rsdroid.exceptions.BackendNetworkException +import net.ankiweb.rsdroid.exceptions.BackendSyncException +import net.ankiweb.rsdroid.exceptions.BackendSyncException.BackendSyncServerMessageException +import org.junit.Test +import org.junit.runner.RunWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +class ThrowableFilterServiceTest : JvmTest() { + + @Test + fun `Normal exceptions are flagged as PII-safe`() { + val exception = BackendDeckIsFilteredException(BackendError.newBuilder().build()) + assertTrue(exception.safeFromPII(), "Exception reported as safe from PII") + } + + @Test + fun `BackendSyncServerMessage exceptions are flagged as PII-unsafe`() { + val exception1 = BackendSyncServerMessageException(BackendError.newBuilder().build()) + assertFalse(exception1.safeFromPII(), "Exception reported as not safe from PII") + + val exception2 = Exception("", Exception("", exception1)) + assertFalse(exception2.safeFromPII(), "Nested exception reported as not safe from PII") + } + + @Test + fun `exceptions are discarded correctly by type`() { + // regular exceptions should go through + assertFalse(ThrowableFilterService.shouldDiscardThrowable(Exception("wanted"))) + + // exceptions of known unwanted types should not go through + val exception1 = BackendNetworkException(BackendError.newBuilder().build()) + assertTrue(ThrowableFilterService.shouldDiscardThrowable(exception1)) + val exception2 = BackendSyncException(BackendError.newBuilder().build()) + assertTrue(ThrowableFilterService.shouldDiscardThrowable(exception2)) + val exception3 = StorageAccessException("test exception") + assertTrue(ThrowableFilterService.shouldDiscardThrowable(exception3)) + } +} diff --git a/build.gradle.kts b/build.gradle.kts index 0e4d98a52292..c7f834651da7 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -41,6 +41,9 @@ subprojects { // tell backend to avoid rollover time, and disable interval fuzzing it.environment("ANKI_TEST_MODE", "1") + it.maxHeapSize = "2g" + it.minHeapSize = "1g" + it.useJUnitPlatform() it.testLogging { events("failed", "skipped")