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

Stop onActivityResult from proceeding when intent is null to prevent app from crashing #352

Merged

Conversation

AlgirdasVZ
Copy link
Contributor

Partially fixes #130

Description

There are at least several cases where the app crashes in onActivityResult method when received data intent is null (more about this - #130). The cause of the crash is that AuthorizationResponse.fromIntent method (imported from net.openid.appauth) has a null checker method, which throws NullPointerException and the user is presented with a native crash info screen. I'm proposing to add a check for data intent being null, and terminate the authorization process if it is (reject the the promise with error message and return from onActivityResult method). As the data intent being null will always end up in crashing the app, there is no point to continue onActivityResult method. Terminating the authorization process instead would be much more user-friendly solution.

Steps to verify

One of the scenarios to reproduce the crash:

1. Open the app
2. Open the idp web login screen in the app
3. Put the app in background
4. Start the app from the device launcher

Prerequisites: android:launchMode="singleTask" in AndroidManifest.xml

@AlgirdasVZ AlgirdasVZ changed the title Stopped onActivityResult from proceeding when intent is null; Stop onActivityResult from proceeding when intent is null to prevent app from crashing Jul 11, 2019
@kadikraman kadikraman added this to the Next Major Release 🚀 milestone Jul 15, 2019
@kadikraman kadikraman merged commit 10a5c1a into FormidableLabs:master Jul 15, 2019
jasondibenedetto pushed a commit to mango-chutney/react-native-app-auth that referenced this pull request Aug 27, 2019
@mklekowski
Copy link

Can it be released as a patch to 4.4.0?

@kadikraman
Copy link
Contributor

Hi @mklekowski - I'd like to avoid maintaining two branches of the library - it'll get messy quick!

I'd use patch-package for this, since the change is so small:

  1. open node_modules/react-native-app-auth/android/src/main/java/com/rnappauth/RNAppAuthModule.java and mirror the changes in this PR
  2. run npx patch-package some-package (this will create a .patch file)
  3. commit the patch to your source control
  4. in package.json add a postistall script:
 "scripts": {
+  "postinstall": "patch-package"
 }
  1. add the library as a dependency
yarn add patch-package postinstall-postinstall

or

npm i patch-package

This basically adds a patch to the library every time you do an install. Once you can upgrade to v5 of this library you can simply delete the patch file and the package.

@kadikraman
Copy link
Contributor

Released in v5.0.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash on Android : dataIntent must not be null
3 participants