Skip to content

fix(crash): NPE in PermissionsActivity #2349

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.

Sign up for GitHub

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

Merged
nan-li merged 2 commits into from
Aug 14, 2025
Merged

fix(crash): NPE in PermissionsActivity #2349

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
59fa734
Return early if extras for PermissionsActivity is null
nan-li Aug 13, 2025
6047d99
finish the activity when we fail init or null extras
nan-li Aug 14, 2025
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
18 changes: 14 additions & 4 deletions ...gnalSDK/onesignal/core/src/main/java/com/onesignal/core/activities/PermissionsActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ class PermissionsActivity : Activity() {
super.onCreate(savedInstanceState)

if (!OneSignal.initWithContext(this)) {
finishActivity()
return
}

if (intent.extras == null) {
// This should never happen, but extras is null in rare crash reports
finishActivity()
return
Copy link
Contributor

@jinliu9508 jinliu9508 Aug 13, 2025

Choose a reason for hiding this comment

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

Returning early keeps the Activity alive (it will still hit onStart/onResume), leaves it on the back stack, and can cause a blank/flickery frame. Even though this was most likely triggered by tests passing null extras, we should harden production code.

I would recommend reusing the existing exit path at line 49 so behavior is deterministic.

Copy link
Contributor Author

@nan-li nan-li Aug 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is because we already called super.onCreate() so we need to call finish()?
Should the initWithContext check do that as well? I think it is more common for initWithContext to return false, than for the extras to be null?

Copy link
Contributor

@jinliu9508 jinliu9508 Aug 13, 2025

Choose a reason for hiding this comment

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

In practice, I suspect the initWithContext check never fail: showing the permission prompt requires OneSignal.getNotification() or OneSignal.getLocation(), and if initWithContext wasn’t invoked earlier, the “must call initWithContext before use” exception would already be thrown.

For the case of the automated tests, initializing OneSignal by calling initWithContext in onCreate() here seems to work, so was continued to handleBundleParams().

Copy link
Contributor Author

@nan-li nan-li Aug 14, 2025

Choose a reason for hiding this comment

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

Let's call finish on all early returns, updated the PR.

}

Expand All @@ -37,12 +44,16 @@ class PermissionsActivity : Activity() {
handleBundleParams(intent.extras)
}

private fun finishActivity() {
finish()
overridePendingTransition(R.anim.onesignal_fade_in, R.anim.onesignal_fade_out)
}

private fun handleBundleParams(extras: Bundle?) {
// https://github.com/OneSignal/OneSignal-Android-SDK/issues/30
// Activity maybe invoked directly through automated testing, omit prompting on old Android versions.
if (Build.VERSION.SDK_INT < 23) {
finish()
overridePendingTransition(R.anim.onesignal_fade_in, R.anim.onesignal_fade_out)
finishActivity()
return
}

Expand Down Expand Up @@ -127,8 +138,7 @@ class PermissionsActivity : Activity() {
}, DELAY_TIME_CALLBACK_CALL.toLong())
}

finish()
overridePendingTransition(R.anim.onesignal_fade_in, R.anim.onesignal_fade_out)
finishActivity()
}

private fun shouldShowSettings(permission: String): Boolean {
Expand Down
Loading