Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch exceptions thrown by Context.registerReceiver to prevent rare crashes #1240

Merged
merged 1 commit into from
May 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ public Unit invoke(String oldOrientation, String newOrientation) {
}
}
);
appContext.registerReceiver(receiver, configFilter);
ContextExtensionsKt.registerReceiverSafe(appContext, receiver, configFilter, logger);
}

void setupNdkPlugin() {
Expand Down Expand Up @@ -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");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
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)
} catch (exc: IllegalArgumentException) {
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)
} catch (exc: IllegalArgumentException) {
logger?.w("Failed to register receiver", exc)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
}