fix: catch registerReceiver exceptions to prevent rare crashes

bugsnag · May 10, 2021 · 9f2a0db · 9f2a0db
1 parent 20df8b1
commit 9f2a0db
Show file tree

Hide file tree

Showing 8 changed files with 113 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,9 @@
 * Prevent rare NPE when capturing thread traces
   [#1237](https://github.com/bugsnag/bugsnag-android/pull/1237)
 
+* Catch exceptions thrown by Context.registerReceiver to prevent rare crashes
+  [#1240](https://github.com/bugsnag/bugsnag-android/pull/1240)
+
 ## 5.9.1 (2021-04-22)
 
 ### Bug fixes

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java
@@ -343,7 +343,7 @@ public Unit invoke(String oldOrientation, String newOrientation) {
                     }
                 }
         );
-        appContext.registerReceiver(receiver, configFilter);
+        ContextExtensionsKt.registerReceiverSafe(appContext, receiver, configFilter, logger);
     }
 
     void setupNdkPlugin() {
@@ -923,7 +923,8 @@ EventStore getEventStore() {
     protected void finalize() throws Throwable {
         if (systemBroadcastReceiver != null) {
             try {
-                appContext.unregisterReceiver(systemBroadcastReceiver);
+                ContextExtensionsKt.unregisterReceiverSafe(appContext,
+                        systemBroadcastReceiver, logger);
             } catch (IllegalArgumentException exception) {
                 logger.w("Receiver not registered");
             }

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/ConnectivityCompat.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/ConnectivityCompat.kt
@@ -61,10 +61,10 @@ internal class ConnectivityLegacy(
 
     override fun registerForNetworkChanges() {
         val intentFilter = IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION)
-        context.registerReceiver(changeReceiver, intentFilter)
+        context.registerReceiverSafe(changeReceiver, intentFilter)
     }
 
-    override fun unregisterForNetworkChanges() = context.unregisterReceiver(changeReceiver)
+    override fun unregisterForNetworkChanges() = context.unregisterReceiverSafe(changeReceiver)
 
     override fun hasNetworkConnection(): Boolean {
         return cm.activeNetworkInfo?.isConnectedOrConnecting ?: false

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/ContextExtensions.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/ContextExtensions.kt
@@ -0,0 +1,43 @@
+package com.bugsnag.android
+
+import android.content.BroadcastReceiver
+import android.content.Context
+import android.content.Intent
+import android.content.IntentFilter
+import android.os.RemoteException
+
+/**
+ * Calls [Context.registerReceiver] but swallows [SecurityException] and [RemoteException]
+ * to avoid terminating the process in rare cases where the registration is unsuccessful.
+ */
+internal fun Context.registerReceiverSafe(
+    receiver: BroadcastReceiver?,
+    filter: IntentFilter?,
+    logger: Logger? = null
+): Intent? {
+    try {
+        return registerReceiver(receiver, filter)
+    } catch (exc: SecurityException) {
+        logger?.w("Failed to register receiver", exc)
+    } catch (exc: RemoteException) {
+        logger?.w("Failed to register receiver", exc)
+    }
+    return null
+}
+
+/**
+ * Calls [Context.unregisterReceiver] but swallows [SecurityException] and [RemoteException]
+ * to avoid terminating the process in rare cases where the registration is unsuccessful.
+ */
+internal fun Context.unregisterReceiverSafe(
+    receiver: BroadcastReceiver?,
+    logger: Logger? = null
+) {
+    try {
+        unregisterReceiver(receiver)
+    } catch (exc: SecurityException) {
+        logger?.w("Failed to register receiver", exc)
+    } catch (exc: RemoteException) {
+        logger?.w("Failed to register receiver", exc)
+    }
+}
diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt
@@ -131,7 +131,7 @@ internal class DeviceDataCollector(
     private fun getBatteryLevel(): Float? {
         try {
             val ifilter = IntentFilter(Intent.ACTION_BATTERY_CHANGED)
-            val batteryStatus = appContext.registerReceiver(null, ifilter)
+            val batteryStatus = appContext.registerReceiverSafe(null, ifilter, logger)
 
             if (batteryStatus != null) {
                 return batteryStatus.getIntExtra(
@@ -151,7 +151,7 @@ internal class DeviceDataCollector(
     private fun isCharging(): Boolean? {
         try {
             val ifilter = IntentFilter(Intent.ACTION_BATTERY_CHANGED)
-            val batteryStatus = appContext.registerReceiver(null, ifilter)
+            val batteryStatus = appContext.registerReceiverSafe(null, ifilter, logger)
 
             if (batteryStatus != null) {
                 val status = batteryStatus.getIntExtra("status", -1)

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/SystemBroadcastReceiver.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/SystemBroadcastReceiver.java
@@ -32,7 +32,7 @@ class SystemBroadcastReceiver extends BroadcastReceiver {
     }
 
     static SystemBroadcastReceiver register(final Client client,
-                                            Logger logger,
+                                            final Logger logger,
                                             BackgroundTaskService bgTaskService) {
         final SystemBroadcastReceiver receiver = new SystemBroadcastReceiver(client, logger);
         if (receiver.getActions().size() > 0) {
@@ -41,7 +41,9 @@ static SystemBroadcastReceiver register(final Client client,
                     @Override
                     public void run() {
                         IntentFilter intentFilter = receiver.getIntentFilter();
-                        client.appContext.registerReceiver(receiver, intentFilter);
+                        Context context = client.appContext;
+                        ContextExtensionsKt.registerReceiverSafe(context,
+                                receiver, intentFilter, logger);
                     }
                 });
             } catch (RejectedExecutionException ex) {

diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ConnectivityLegacyTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/ConnectivityLegacyTest.kt
@@ -29,14 +29,14 @@ class ConnectivityLegacyTest {
     fun registerForNetworkChanges() {
         val conn = ConnectivityLegacy(context, cm, null)
         conn.registerForNetworkChanges()
-        Mockito.verify(context, times(1)).registerReceiver(any(), any())
+        Mockito.verify(context, times(1)).registerReceiverSafe(any(), any())
     }
 
     @Test
     fun unregisterForNetworkChanges() {
         val conn = ConnectivityLegacy(context, cm, null)
         conn.unregisterForNetworkChanges()
-        Mockito.verify(context, times(1)).unregisterReceiver(any())
+        Mockito.verify(context, times(1)).unregisterReceiverSafe(any())
     }
 
     @Test

diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ContextExtensionsKtTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/ContextExtensionsKtTest.kt
@@ -0,0 +1,54 @@
+package com.bugsnag.android
+
+import android.content.BroadcastReceiver
+import android.content.Context
+import android.content.IntentFilter
+import org.junit.Assert.assertEquals
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.Mock
+import org.mockito.Mockito.`when`
+import org.mockito.Mockito.times
+import org.mockito.Mockito.verify
+import org.mockito.junit.MockitoJUnitRunner
+
+@RunWith(MockitoJUnitRunner::class)
+class ContextExtensionsKtTest {
+
+    @Mock
+    lateinit var context: Context
+
+    @Mock
+    lateinit var receiver: BroadcastReceiver
+
+    @Mock
+    lateinit var filter: IntentFilter
+
+    @Test
+    fun registerReceiverSafe() {
+        context.registerReceiverSafe(receiver, filter)
+        verify(context, times(1)).registerReceiver(receiver, filter)
+    }
+
+    @Test
+    fun registerReceiverSecurityException() {
+        val logger = InterceptingLogger()
+        `when`(context.registerReceiver(receiver, filter)).thenThrow(SecurityException())
+        context.registerReceiverSafe(receiver, filter, logger)
+        assertEquals("Failed to register receiver", logger.msg)
+    }
+
+    @Test
+    fun unregisterReceiverSafe() {
+        context.unregisterReceiverSafe(receiver)
+        verify(context, times(1)).unregisterReceiver(receiver)
+    }
+
+    @Test
+    fun unregisterReceiverSecurityException() {
+        val logger = InterceptingLogger()
+        `when`(context.unregisterReceiver(receiver)).thenThrow(SecurityException())
+        context.unregisterReceiverSafe(receiver, logger)
+        assertEquals("Failed to register receiver", logger.msg)
+    }
+}