diff options
6 files changed, 238 insertions, 14 deletions
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ServiceEndpoint.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ServiceEndpoint.kt index 4868f80fd2..4230aa14f1 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ServiceEndpoint.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ServiceEndpoint.kt @@ -1,7 +1,6 @@ package net.mullvad.mullvadvpn.service.endpoint import android.content.Context -import android.os.DeadObjectException import android.os.Looper import android.os.Messenger import kotlinx.coroutines.Dispatchers @@ -17,8 +16,11 @@ import net.mullvad.mullvadvpn.ipc.Request import net.mullvad.mullvadvpn.service.MullvadDaemon import net.mullvad.mullvadvpn.service.persistence.SplitTunnelingPersistence import net.mullvad.mullvadvpn.util.Intermittent +import net.mullvad.mullvadvpn.util.trySendEvent import net.mullvad.talpid.ConnectivityListener +const val SHOULD_LOG_DEAD_OBJECT_EXCEPTION = true + class ServiceEndpoint( looper: Looper, internal val intermittentDaemon: Intermittent<MullvadDaemon>, @@ -93,13 +95,14 @@ class ServiceEndpoint( val deadListeners = mutableSetOf<Int>() for ((id, listener) in listeners) { - try { - listener.send(event.message) - } catch (_: DeadObjectException) { + if (!listener.trySendEvent( + event, + SHOULD_LOG_DEAD_OBJECT_EXCEPTION + ) + ) { deadListeners.add(id) } } - deadListeners.forEach { listeners.remove(it) } } } @@ -147,8 +150,14 @@ class ServiceEndpoint( initialEvents.add(Event.VpnPermissionRequest) } - initialEvents.forEach { event -> - listener.send(event.message) + val didSuccessfullySendAllMessages = initialEvents.all { event -> + listener.trySendEvent( + event, + SHOULD_LOG_DEAD_OBJECT_EXCEPTION + ) + } + if (didSuccessfullySendAllMessages.not()) { + listeners.remove(listenerId) } } } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ConnectionProxy.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ConnectionProxy.kt index 5b4b88ad94..fcd8565550 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ConnectionProxy.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ConnectionProxy.kt @@ -10,6 +10,7 @@ import net.mullvad.mullvadvpn.ipc.Event import net.mullvad.mullvadvpn.ipc.EventDispatcher import net.mullvad.mullvadvpn.ipc.Request import net.mullvad.mullvadvpn.model.TunnelState +import net.mullvad.mullvadvpn.util.trySendRequest import net.mullvad.talpid.tunnel.ActionAfterDisconnect import net.mullvad.talpid.util.EventNotifier @@ -34,19 +35,19 @@ class ConnectionProxy(private val connection: Messenger, eventDispatcher: EventD fun connect() { if (anticipateConnectingState()) { - connection.send(Request.Connect.message) + connection.trySendRequest(Request.Connect, true) } } fun disconnect() { if (anticipateReconnectingState()) { - connection.send(Request.Disconnect.message) + connection.trySendRequest(Request.Disconnect, true) } } fun reconnect() { if (anticipateDisconnectingState()) { - connection.send(Request.Reconnect.message) + connection.trySendRequest(Request.Reconnect, true) } } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnectionDeviceDataSource.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnectionDeviceDataSource.kt index 6e445a6d34..56e7ee4d28 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnectionDeviceDataSource.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnectionDeviceDataSource.kt @@ -6,6 +6,7 @@ import kotlinx.coroutines.flow.callbackFlow import net.mullvad.mullvadvpn.ipc.Event import net.mullvad.mullvadvpn.ipc.EventDispatcher import net.mullvad.mullvadvpn.ipc.Request +import net.mullvad.mullvadvpn.util.trySendRequest class ServiceConnectionDeviceDataSource( private val connection: Messenger, @@ -43,18 +44,18 @@ class ServiceConnectionDeviceDataSource( // Async result: Event.DeviceChanged fun refreshDevice() { - connection.send(Request.RefreshDeviceState.message) + connection.trySendRequest(Request.RefreshDeviceState, true) } fun getDevice() { - connection.send(Request.GetDevice.message) + connection.trySendRequest(Request.GetDevice, true) } fun removeDevice(accountToken: String, deviceId: String) { - connection.send(Request.RemoveDevice(accountToken, deviceId).message) + connection.trySendRequest(Request.RemoveDevice(accountToken, deviceId), true) } fun refreshDeviceList(accountToken: String) { - connection.send(Request.GetDeviceList(accountToken).message) + connection.trySendRequest(Request.GetDeviceList(accountToken), true) } } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/MessengerExtensions.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/MessengerExtensions.kt new file mode 100644 index 0000000000..71ce51ad79 --- /dev/null +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/MessengerExtensions.kt @@ -0,0 +1,40 @@ +package net.mullvad.mullvadvpn.util + +import android.os.DeadObjectException +import android.os.Message +import android.os.Messenger +import android.os.RemoteException +import android.util.Log +import net.mullvad.mullvadvpn.ipc.Event +import net.mullvad.mullvadvpn.ipc.Request + +fun Messenger.trySendEvent(event: Event, logErrors: Boolean): Boolean { + return trySend(event.message, logErrors, event::class.qualifiedName) +} + +fun Messenger.trySendRequest(request: Request, logErrors: Boolean): Boolean { + return trySend(request.message, logErrors, request::class.qualifiedName) +} + +private fun Messenger.trySend(message: Message, logErrors: Boolean, messageName: String?): Boolean { + return try { + this.send(message) + true + } catch (deadObjectException: DeadObjectException) { + if (logErrors) { + Log.e( + "mullvad", + "Failed to send message ${messageName ?: "<missing>"} due to DeadObjectException" + ) + } + false + } catch (remoteException: RemoteException) { + if (logErrors) { + Log.e( + "mullvad", + "Failed to send message ${messageName ?: "<missing>"} due to RemoteException" + ) + } + false + } +} diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/service/ConnectionProxyTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/service/ConnectionProxyTest.kt new file mode 100644 index 0000000000..44ccdf013b --- /dev/null +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/service/ConnectionProxyTest.kt @@ -0,0 +1,92 @@ +package net.mullvad.mullvadvpn.service + +import android.os.DeadObjectException +import android.os.Looper +import android.os.Messenger +import android.util.Log +import io.mockk.MockKAnnotations +import io.mockk.Runs +import io.mockk.every +import io.mockk.impl.annotations.MockK +import io.mockk.just +import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.mockkStatic +import io.mockk.slot +import io.mockk.unmockkAll +import kotlin.reflect.KClass +import kotlin.test.assertEquals +import net.mullvad.mullvadvpn.ipc.Event +import net.mullvad.mullvadvpn.ipc.EventDispatcher +import net.mullvad.mullvadvpn.ipc.Request +import net.mullvad.mullvadvpn.ui.serviceconnection.ConnectionProxy +import org.junit.After +import org.junit.Before +import org.junit.Test + +class ConnectionProxyTest { + + @MockK + private lateinit var mockedMainLooper: Looper + + @MockK + private lateinit var connection: Messenger + + @MockK + private lateinit var mockedDispatchingHandler: EventDispatcher + lateinit var connectionProxy: ConnectionProxy + + @Before + fun setup() { + mockkStatic(Looper::class) + mockkStatic(Log::class) + MockKAnnotations.init(this) + mockkObject(Request.Connect, Request.Disconnect) + every { Request.Connect.message } returns mockk() + every { Request.Disconnect.message } returns mockk() + every { Looper.getMainLooper() } returns mockedMainLooper + every { Log.e(any(), any()) } returns mockk(relaxed = true) + } + + @After + fun tearDown() { + unmockkAll() + } + + @Test + fun test_initialize_connection_proxy() { + // Arrange + val eventType = slot<KClass<Event.TunnelStateChange>>() + every { + mockedDispatchingHandler.registerHandler(capture(eventType), any()) + } just Runs + // Create ConnectionProxy instance and assert initial Event type + connectionProxy = ConnectionProxy(connection, mockedDispatchingHandler) + assertEquals(Event.TunnelStateChange::class, eventType.captured.java.kotlin) + } + + @Test + fun test_tunnel_normal_connect_disconnect() { + // Arrange + every { connection.send(any()) } just Runs + every { + mockedDispatchingHandler.registerHandler(any<KClass<Event>>(), any()) + } just Runs + // Act and Assert no crashes + connectionProxy = ConnectionProxy(connection, mockedDispatchingHandler) + connectionProxy.connect() + connectionProxy.disconnect() + } + + @Test + fun test_tunnel_handle_crash_on_connect() { + // Arrange + every { connection.send(any()) } throws DeadObjectException() + every { + mockedDispatchingHandler.registerHandler(any<KClass<Event>>(), any()) + } just Runs + // Act and Assert no crashes + connectionProxy = ConnectionProxy(connection, mockedDispatchingHandler) + connectionProxy.connect() + } +} diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/service/ServiceConnectionDeviceDataSourceTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/service/ServiceConnectionDeviceDataSourceTest.kt new file mode 100644 index 0000000000..8b737cba69 --- /dev/null +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/service/ServiceConnectionDeviceDataSourceTest.kt @@ -0,0 +1,81 @@ +package net.mullvad.mullvadvpn.service + +import android.os.DeadObjectException +import android.os.Looper +import android.os.Messenger +import io.mockk.MockKAnnotations +import io.mockk.Runs +import io.mockk.every +import io.mockk.impl.annotations.MockK +import io.mockk.just +import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import kotlin.reflect.KClass +import net.mullvad.mullvadvpn.ipc.Event +import net.mullvad.mullvadvpn.ipc.EventDispatcher +import net.mullvad.mullvadvpn.ipc.Request +import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionDeviceDataSource +import net.mullvad.mullvadvpn.util.JobTracker +import org.junit.After +import org.junit.Before +import org.junit.Test + +class ServiceConnectionDeviceDataSourceTest { + private val tracker = JobTracker() + + @MockK + private lateinit var mockedMainLooper: Looper + + @MockK + private lateinit var mockedDispatchingHandler: EventDispatcher + + @MockK + private lateinit var connection: Messenger + + lateinit var serviceConnectionDeviceDataSource: ServiceConnectionDeviceDataSource + + @Before + fun setup() { + mockkStatic(Looper::class) + mockkStatic(android.util.Log::class) + MockKAnnotations.init(this) + mockkObject(Request.GetDevice, Request.RefreshDeviceState) + every { Request.GetDevice.message } returns mockk() + every { Request.RefreshDeviceState.message } returns mockk() + every { Looper.getMainLooper() } returns mockedMainLooper + every { android.util.Log.e(any(), any()) } returns mockk(relaxed = true) + } + + @After + fun tearDown() { + unmockkAll() + } + + @Test + fun test_get_devices_list() { + // Arrange + every { connection.send(any()) } just Runs + every { + mockedDispatchingHandler.registerHandler(any<KClass<Event>>(), any()) + } just Runs + // Act and Assert no crashes + serviceConnectionDeviceDataSource = + ServiceConnectionDeviceDataSource(connection, mockedDispatchingHandler) + serviceConnectionDeviceDataSource.getDevice() + } + + @Test + fun test_catch_exception_on_devices_list() { + // Arrange + every { connection.send(any()) } throws DeadObjectException() + every { + mockedDispatchingHandler.registerHandler(any<KClass<Event>>(), any()) + } just Runs + // Act and Assert no crashes + serviceConnectionDeviceDataSource = + ServiceConnectionDeviceDataSource(connection, mockedDispatchingHandler) + serviceConnectionDeviceDataSource.getDevice() + } +} |