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

Fix crash in request vpn permission #6547

Conversation

Pururun
Copy link
Contributor

@Pururun Pururun commented Jul 30, 2024

This crash could occur if create intent in request vpn permission was called while vpn permission was already approved

Fixed by returning true immediately in those cases


This change is Reviewable

@Pururun Pururun added the Android Issues related to Android label Jul 30, 2024
@Pururun Pururun requested a review from Rawa July 30, 2024 15:44
Copy link

linear bot commented Jul 30, 2024

Copy link
Contributor

@Rawa Rawa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @Pururun)


android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/RequestVpnPermission.kt line 11 at r1 (raw file):

class RequestVpnPermission : ActivityResultContract<Unit, Boolean>() {
    override fun createIntent(context: Context, input: Unit): Intent {
        return VpnService.prepare(context)

This is a bit unclear, return type is Intent but VpnService.prepare(context) can return null right? We should at least do !! to make it clear that we don't expect a null.

@Pururun Pururun force-pushed the crash-on-first-connection-attempt-after-installing-latest-droid-1214 branch from bbf2c87 to e3de3f4 Compare July 31, 2024 08:27
Copy link
Contributor Author

@Pururun Pururun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @Rawa)


android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/RequestVpnPermission.kt line 11 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

This is a bit unclear, return type is Intent but VpnService.prepare(context) can return null right? We should at least do !! to make it clear that we don't expect a null.

Done.

@Pururun Pururun force-pushed the crash-on-first-connection-attempt-after-installing-latest-droid-1214 branch from e3de3f4 to 595e519 Compare July 31, 2024 11:08
Copy link
Contributor

@Rawa Rawa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

This crash could occur if create intent in request vpn permission
was called while vpn permission was already approved

Fixed by returning true immediately in those cases
@Rawa Rawa force-pushed the crash-on-first-connection-attempt-after-installing-latest-droid-1214 branch from 595e519 to 84fbae1 Compare July 31, 2024 12:21
@Rawa Rawa merged commit 59929c1 into main Jul 31, 2024
24 checks passed
@Rawa Rawa deleted the crash-on-first-connection-attempt-after-installing-latest-droid-1214 branch July 31, 2024 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android Issues related to Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants