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

Handle IllegalStateException on VpnService.prepare #7227

Closed
Closed
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
1 change: 1 addition & 0 deletions android/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Line wrap the file at 100 chars. Th
- Fix a bug where the Android account expiry notifications would not be updated if the app was
running in the background for a long time.
- Fix ANR due to the tokio runtime being blocked by `getaddrinfo` when dropped.
- Fix crash when having a legacy VPN profile as always-on.


## [android/2024.8] - 2024-11-01
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ class ConnectScreenTest {
}

// Assert
onNodeWithText("FAILED TO SECURE CONNECTION").assertExists()
onNodeWithText("FAILED TO CONNECT").assertExists()
onNodeWithText(mockLocationName).assertExists()
onNodeWithText("Dismiss").assertExists()
onNodeWithText(text = "Critical error (your attention is required)", ignoreCase = true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.vector.ImageVector
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.text.toUpperCase
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import androidx.constraintlayout.compose.ConstraintLayout
Expand Down Expand Up @@ -129,7 +130,7 @@ private fun Notification(notificationBannerData: NotificationData) {
},
)
Text(
text = title.uppercase(),
text = title.toUpperCase(),
modifier =
Modifier.constrainAs(textTitle) {
top.linkTo(parent.top)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package net.mullvad.mullvadvpn.compose.component.notificationbanner
import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.automirrored.filled.OpenInNew
import androidx.compose.material.icons.filled.Clear
import androidx.compose.material.icons.filled.OpenInNew
import androidx.compose.material3.MaterialTheme
import androidx.compose.runtime.Composable
import androidx.compose.ui.graphics.vector.ImageVector
Expand All @@ -13,26 +12,29 @@ import androidx.compose.ui.text.AnnotatedString
import androidx.compose.ui.text.SpanStyle
import androidx.compose.ui.text.font.FontWeight
import androidx.core.text.HtmlCompat
import java.net.InetAddress
import net.mullvad.mullvadvpn.R
import net.mullvad.mullvadvpn.compose.extensions.getExpiryQuantityString
import net.mullvad.mullvadvpn.compose.extensions.toAnnotatedString
import net.mullvad.mullvadvpn.lib.common.util.getErrorNotificationResources
import net.mullvad.mullvadvpn.lib.model.AuthFailedError
import net.mullvad.mullvadvpn.lib.model.ErrorState
import net.mullvad.mullvadvpn.lib.model.ErrorStateCause
import net.mullvad.mullvadvpn.lib.model.ParameterGenerationError
import net.mullvad.mullvadvpn.repository.InAppNotification
import net.mullvad.mullvadvpn.ui.notification.StatusLevel

data class NotificationData(
val title: String,
val title: AnnotatedString,
val message: AnnotatedString? = null,
val statusLevel: StatusLevel,
val action: NotificationAction? = null,
) {
constructor(
title: String,
message: String?,
message: String? = null,
statusLevel: StatusLevel,
action: NotificationAction?,
) : this(title, message?.let { AnnotatedString(it) }, statusLevel, action)
action: NotificationAction? = null,
) : this(AnnotatedString(title), message?.let { AnnotatedString(it) }, statusLevel, action)
}

data class NotificationAction(
Expand All @@ -51,22 +53,11 @@ fun InAppNotification.toNotificationData(
when (this) {
is InAppNotification.NewDevice ->
NotificationData(
title = stringResource(id = R.string.new_device_notification_title),
title =
AnnotatedString(stringResource(id = R.string.new_device_notification_title)),
message =
HtmlCompat.fromHtml(
stringResource(
id = R.string.new_device_notification_message,
deviceName,
),
HtmlCompat.FROM_HTML_MODE_COMPACT,
)
.toAnnotatedString(
boldSpanStyle =
SpanStyle(
color = MaterialTheme.colorScheme.onSurface,
fontWeight = FontWeight.ExtraBold,
)
),
stringResource(id = R.string.new_device_notification_message, deviceName)
.formatWithHtml(),
statusLevel = StatusLevel.Info,
action =
NotificationAction(
Expand Down Expand Up @@ -111,23 +102,94 @@ fun InAppNotification.toNotificationData(

@Composable
private fun errorMessageBannerData(error: ErrorState) =
error.getErrorNotificationResources(LocalContext.current).run {
NotificationData(
title = stringResource(id = titleResourceId),
message =
HtmlCompat.fromHtml(
optionalMessageArgument?.let { stringResource(id = messageResourceId, it) }
?: stringResource(id = messageResourceId),
HtmlCompat.FROM_HTML_MODE_COMPACT,
)
.toAnnotatedString(
boldSpanStyle =
SpanStyle(
color = MaterialTheme.colorScheme.onSurface,
fontWeight = FontWeight.ExtraBold,
)
),
statusLevel = StatusLevel.Error,
action = null,
NotificationData(
title = error.title().formatWithHtml(),
message = error.message().formatWithHtml(),
statusLevel = StatusLevel.Error,
action = null,
)

@Composable
private fun String.formatWithHtml(): AnnotatedString =
HtmlCompat.fromHtml(this, HtmlCompat.FROM_HTML_MODE_COMPACT)
.toAnnotatedString(
boldSpanStyle =
SpanStyle(
color = MaterialTheme.colorScheme.onSurface,
fontWeight = FontWeight.ExtraBold,
)
)

@Composable
private fun ErrorState.title(): String {
val cause = this.cause
return when {
cause is ErrorStateCause.InvalidDnsServers -> stringResource(R.string.blocking_internet)
cause is ErrorStateCause.NotPrepared ->
stringResource(R.string.vpn_permission_error_notification_title)
cause is ErrorStateCause.OtherAlwaysOnApp ->
stringResource(R.string.always_on_vpn_error_notification_title, cause.appName)
cause is ErrorStateCause.OtherLegacyAlwaysOnApp ->
stringResource(R.string.legacy_always_on_vpn_error_notification_title)
isBlocking -> stringResource(R.string.blocking_internet)
else -> stringResource(R.string.critical_error)
}
}

@Composable
private fun ErrorState.message(): String {
val cause = this.cause
return when {
isBlocking -> cause.errorMessageId()
else -> stringResource(R.string.failed_to_block_internet)
}
}

@Composable
private fun ErrorStateCause.errorMessageId(): String =
when (this) {
is ErrorStateCause.AuthFailed -> stringResource(error.errorMessageId())
is ErrorStateCause.Ipv6Unavailable -> stringResource(R.string.ipv6_unavailable)
is ErrorStateCause.FirewallPolicyError -> stringResource(R.string.set_firewall_policy_error)
is ErrorStateCause.DnsError -> stringResource(R.string.set_dns_error)
is ErrorStateCause.StartTunnelError -> stringResource(R.string.start_tunnel_error)
is ErrorStateCause.IsOffline -> stringResource(R.string.is_offline)
is ErrorStateCause.TunnelParameterError -> stringResource(error.errorMessageId())
is ErrorStateCause.NotPrepared ->
stringResource(R.string.vpn_permission_error_notification_message)
is ErrorStateCause.OtherAlwaysOnApp ->
stringResource(R.string.always_on_vpn_error_notification_content, appName)
is ErrorStateCause.OtherLegacyAlwaysOnApp ->
stringResource(R.string.legacy_always_on_vpn_error_notification_content)
is ErrorStateCause.InvalidDnsServers ->
stringResource(
R.string.invalid_dns_servers,
addresses.joinToString { address -> address.addressString() },
)
}

private fun AuthFailedError.errorMessageId(): Int =
when (this) {
AuthFailedError.ExpiredAccount -> R.string.account_credit_has_expired
AuthFailedError.InvalidAccount,
AuthFailedError.TooManyConnections,
AuthFailedError.Unknown -> R.string.auth_failed
}

private fun ParameterGenerationError.errorMessageId(): Int =
when (this) {
ParameterGenerationError.NoMatchingRelay,
ParameterGenerationError.NoMatchingBridgeRelay -> {
R.string.no_matching_relay
}
ParameterGenerationError.NoWireguardKey -> R.string.no_wireguard_key
ParameterGenerationError.CustomTunnelHostResultionError ->
R.string.custom_tunnel_host_resolution_error
}

private fun InetAddress.addressString(): String {
val hostNameAndAddress = this.toString().split('/', limit = 2)
val address = hostNameAndAddress[1]

return address
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ import com.ramcosta.composedestinations.generated.destinations.SettingsDestinati
import com.ramcosta.composedestinations.navigation.DestinationsNavigator
import com.ramcosta.composedestinations.result.ResultRecipient
import kotlinx.coroutines.launch
import mullvad_daemon.management_interface.tunnelState
import net.mullvad.mullvadvpn.R
import net.mullvad.mullvadvpn.compose.button.ConnectionButton
import net.mullvad.mullvadvpn.compose.button.SwitchLocationButton
Expand All @@ -84,8 +83,8 @@ import net.mullvad.mullvadvpn.compose.test.RECONNECT_BUTTON_TEST_TAG
import net.mullvad.mullvadvpn.compose.test.SELECT_LOCATION_BUTTON_TEST_TAG
import net.mullvad.mullvadvpn.compose.transitions.HomeTransition
import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle
import net.mullvad.mullvadvpn.compose.util.CreateVpnProfile
import net.mullvad.mullvadvpn.compose.util.OnNavResultValue
import net.mullvad.mullvadvpn.compose.util.RequestVpnPermission
import net.mullvad.mullvadvpn.compose.util.showSnackbarImmediately
import net.mullvad.mullvadvpn.constant.SECURE_ZOOM
import net.mullvad.mullvadvpn.constant.SECURE_ZOOM_ANIMATION_MILLIS
Expand All @@ -100,6 +99,7 @@ import net.mullvad.mullvadvpn.lib.model.GeoIpLocation
import net.mullvad.mullvadvpn.lib.model.LatLong
import net.mullvad.mullvadvpn.lib.model.Latitude
import net.mullvad.mullvadvpn.lib.model.Longitude
import net.mullvad.mullvadvpn.lib.model.PrepareError
import net.mullvad.mullvadvpn.lib.model.TunnelState
import net.mullvad.mullvadvpn.lib.theme.AppTheme
import net.mullvad.mullvadvpn.lib.theme.Dimens
Expand Down Expand Up @@ -142,9 +142,9 @@ fun Connect(

val snackbarHostState = remember { SnackbarHostState() }

val launchVpnPermission =
rememberLauncherForActivityResult(RequestVpnPermission()) {
connectViewModel.requestVpnPermissionResult(it)
val createVpnProfile =
rememberLauncherForActivityResult(CreateVpnProfile()) {
connectViewModel.createVpnProfileResult(it)
}

val openAccountPage = LocalUriHandler.current.createOpenAccountPageHook()
Expand All @@ -154,9 +154,8 @@ fun Connect(
minActiveState = Lifecycle.State.RESUMED,
) { sideEffect ->
when (sideEffect) {
is ConnectViewModel.UiSideEffect.OpenAccountManagementPageInBrowser -> {
is ConnectViewModel.UiSideEffect.OpenAccountManagementPageInBrowser ->
openAccountPage(sideEffect.token)
}

is ConnectViewModel.UiSideEffect.OutOfTime ->
navigator.navigate(OutOfTimeDestination) {
Expand All @@ -170,21 +169,37 @@ fun Connect(
popUpTo(NavGraphs.root) { inclusive = true }
}

is ConnectViewModel.UiSideEffect.NoVpnPermission -> launchVpnPermission.launch(Unit)
is ConnectViewModel.UiSideEffect.NotPrepared ->
when (sideEffect.prepareError) {
is PrepareError.OtherLegacyAlwaysOnVpn ->
launch {
snackbarHostState.showSnackbarImmediately(
message = sideEffect.prepareError.toMessage(context)
)
}

is PrepareError.OtherAlwaysOnApp ->
launch {
snackbarHostState.showSnackbarImmediately(
message = sideEffect.prepareError.toMessage(context)
)
}
is PrepareError.NotPrepared ->
createVpnProfile.launch(sideEffect.prepareError.prepareIntent)
}
is ConnectViewModel.UiSideEffect.ConnectError ->
launch {
snackbarHostState.showSnackbarImmediately(
message = sideEffect.toMessage(context)
)
}

is ConnectViewModel.UiSideEffect.OpenUri -> {
is ConnectViewModel.UiSideEffect.OpenUri ->
try {
uriHandler.openUri(sideEffect.uri.toString())
} catch (e: IllegalArgumentException) {
Logger.w("Failed to open uri", e)
}
}
}
}

Expand Down Expand Up @@ -571,15 +586,17 @@ fun GeoIpLocation.toLatLong() =

private fun ConnectViewModel.UiSideEffect.ConnectError.toMessage(context: Context): String =
when (this) {
ConnectViewModel.UiSideEffect.ConnectError.NoVpnPermission ->
context.getString(R.string.vpn_permission_denied_error)

is ConnectViewModel.UiSideEffect.ConnectError.AlwaysOnVpn ->
// Snackbar currently do not support annotated string
context
.getString(R.string.always_on_vpn_error_notification_content, appName)
.removeHtmlTags()

ConnectViewModel.UiSideEffect.ConnectError.Generic ->
context.getString(R.string.error_occurred)

ConnectViewModel.UiSideEffect.ConnectError.PermissionDenied ->
context.getString(R.string.vpn_permission_denied_error)
}

private fun PrepareError.OtherLegacyAlwaysOnVpn.toMessage(context: Context) =
context
.getString(R.string.always_on_vpn_error_notification_content, "Legacy app")
.removeHtmlTags()

private fun PrepareError.OtherAlwaysOnApp.toMessage(context: Context) =
context.getString(R.string.always_on_vpn_error_notification_content, appName).removeHtmlTags()
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,26 @@ import androidx.compose.runtime.DisposableEffect
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.ui.ExperimentalComposeUiApi
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.semantics.testTagsAsResourceId
import androidx.navigation.NavHostController
import arrow.core.merge
import co.touchlab.kermit.Logger
import com.ramcosta.composedestinations.DestinationsNavHost
import com.ramcosta.composedestinations.generated.NavGraphs
import com.ramcosta.composedestinations.generated.destinations.NoDaemonDestination
import com.ramcosta.composedestinations.navigation.DestinationsNavigator
import com.ramcosta.composedestinations.rememberNavHostEngine
import com.ramcosta.composedestinations.utils.rememberDestinationsNavigator
import net.mullvad.mullvadvpn.compose.util.RequestVpnPermission
import net.mullvad.mullvadvpn.compose.util.CreateVpnProfile
import net.mullvad.mullvadvpn.lib.common.util.prepareVpnSafe
import net.mullvad.mullvadvpn.lib.model.PrepareError
import net.mullvad.mullvadvpn.lib.model.Prepared
import net.mullvad.mullvadvpn.viewmodel.DaemonScreenEvent
import net.mullvad.mullvadvpn.viewmodel.NoDaemonViewModel
import net.mullvad.mullvadvpn.viewmodel.VpnPermissionSideEffect
import net.mullvad.mullvadvpn.viewmodel.VpnPermissionViewModel
import net.mullvad.mullvadvpn.viewmodel.VpnProfileSideEffect
import net.mullvad.mullvadvpn.viewmodel.VpnProfileViewModel
import org.koin.androidx.compose.koinViewModel

@OptIn(ExperimentalComposeUiApi::class)
Expand All @@ -32,7 +37,7 @@ fun MullvadApp() {
val navigator: DestinationsNavigator = navHostController.rememberDestinationsNavigator()

val serviceVm = koinViewModel<NoDaemonViewModel>()
val permissionVm = koinViewModel<VpnPermissionViewModel>()
val permissionVm = koinViewModel<VpnProfileViewModel>()

DisposableEffect(Unit) {
navHostController.addOnDestinationChangedListener(serviceVm)
Expand Down Expand Up @@ -64,11 +69,20 @@ fun MullvadApp() {

// Ask for VPN Permission
val launchVpnPermission =
rememberLauncherForActivityResult(RequestVpnPermission()) { _ -> permissionVm.connect() }
rememberLauncherForActivityResult(CreateVpnProfile()) { _ -> permissionVm.connect() }
val context = LocalContext.current
LaunchedEffect(Unit) {
permissionVm.uiSideEffect.collect {
if (it is VpnPermissionSideEffect.ShowDialog) {
launchVpnPermission.launch(Unit)
if (it is VpnProfileSideEffect.RequestVpnProfile) {
val prepareResult = context.prepareVpnSafe().merge()
when (prepareResult) {
is PrepareError.NotPrepared ->
launchVpnPermission.launch(prepareResult.prepareIntent)
// If legacy or other always on connect at let daemon generate a error state
is PrepareError.OtherLegacyAlwaysOnVpn,
is PrepareError.OtherAlwaysOnApp,
Prepared -> permissionVm.connect()
}
}
}
}
Expand Down
Loading
Loading