android-password-store / Android-Password-Store

Android application compatible with ZX2C4's Pass command line application
https://passwordstore.app
GNU General Public License v3.0
2.58k stars 265 forks source link

[BUG] Internal crash when using existing external repository #730

Closed Hazegard closed 4 years ago

Hazegard commented 4 years ago

Describe the bug When setting an external repository, using the top bar back arrow bypasses the permission pop up dialog, which result in an internal crash. Pressing the system back button after the crash screen makes the permission dialog appears.

Pressing the system back button instead of the top bar back arrow does not crash.

To Reproduce Steps to reproduce the behavior:

  1. Get your local git repository on your phone
  2. Click on 'Use Local Directory'
  3. Click on 'SD-CARD', then 'OK'
  4. Select your repository
  5. Click on 'Allow' (I think it is only for Android 10+)
  6. Click on your back button
  7. Click on the top arrow to go back (not the back button)

Using the system back arrow does bring the permission dialog.

Expected behavior The storage permission pop up should appear, then the main screen should appear.

Screenshots image

Crash Log

kotlin.KotlinNullPointerException
    at com.zeapo.pwdstore.utils.PasswordRepository$Companion.initialize(PasswordRepository.kt:175)
    at com.zeapo.pwdstore.PasswordStore.createRepository(PasswordStore.kt:368)
    at com.zeapo.pwdstore.PasswordStore.initializeRepositoryInfo(PasswordStore.kt:411)
    at com.zeapo.pwdstore.PasswordStore.onActivityResult(PasswordStore.kt:673)
    at android.app.Activity.dispatchActivityResult(Activity.java:8131)
    at android.app.ActivityThread.deliverResults(ActivityThread.java:4838)
    at android.app.ActivityThread.handleSendResult(ActivityThread.java:4886)
    at android.app.servertransaction.ActivityResultItem.execute(ActivityResultItem.java:51)
    at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
    at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
    at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2016)
    at android.os.Handler.dispatchMessage(Handler.java:107)
    at android.os.Looper.loop(Looper.java:214)
    at android.app.ActivityThread.main(ActivityThread.java:7356)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:491)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)

Device information (please complete the following information):

Additional context The issue is, I think, because onActivityResult is called before onResume when using the top back arrow, so initializeRepositoryInfo is called before the permission dialog. Given that the app does not have the permission, dir in PasswordRepository L.175 does not exist.

On the other hand, when using the system back button, no resultCode is set, which makes the app to bypass onActivityResult (initializeRepositoryInfo is then not called) , so onResume can show the permission dialog, and then everything works fine.

I think actually that the non crashing path is also wrong, because setResult is not called when using this button, so both path should be fixed:

I might have the time to make a fix for this later this week :grinning:

msfjarvis commented 4 years ago

Thanks for the detailed report and analysis! I'll look into getting a fix out with the point release that's scheduled in the next couple days.