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.

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

Jump to bottom

bug: Android - Filesystem Plugin - Permission request cancellation results in endless loop #4170

Closed
kaunstdadenga opened this issue Feb 8, 2021 · 4 comments
Closed

bug: Android - Filesystem Plugin - Permission request cancellation results in endless loop #4170

kaunstdadenga opened this issue Feb 8, 2021 · 4 comments
Labels
needs reproduction needs reproducible example to illustrate the issue platform: android

Comments

@kaunstdadenga
Copy link

Bug Report

Capacitor Version

latest Dependencies:

@capacitor/cli: 2.4.6
@capacitor/core: 2.4.6
@capacitor/android: 2.4.6
@capacitor/electron: 2.4.6
@capacitor/ios: 2.4.6

Installed Dependencies:

@capacitor/cli 2.4.5
@capacitor/android 2.4.6
@capacitor/ios 2.4.5
@capacitor/core 2.4.5
@capacitor/electron not installed

Platform(s)

Android

Current Behavior

When Android neither accepts or denies the permission request but instead cancels it, Filesystem does not recognise it as cancelled and neither finds a deny and restores the saved call. When processing the saved call Filesystem recognises that there is no permission for the Filesystem and request the permission again, which results in an endless loop and the app crashes.

Expected Behavior

As there are no methods to check for permissions on the capacitor Filessystem plugin, it should manage the permission dialogs correctly according to Android implementation.

Code Reproduction

Unfortunately I cannot provide a simple code to reproduce this behaviour as our project is quite large already. Additionally this problem only occurs when a modal is dismissed where the modal and the underlying view are trying to access some files and the User declined all the Filesystem Permissions to this part.

But I have pinpointed the Problem in the Android SDK (see Additional Context)

Other Technical Details

npm --version output: 6.14.8

node --version output: v14.13.1

pod --version output (iOS issues only):

Additional Context

To pinpoint the issue I looked into the Filesystem and Android Code and found the exact problem and already fixed it locally.

If you take a look at the Capacitor Filesystem.java v4.6 @ Line 664-674:

for (int i = 0; i < grantResults.length; i++) {
  int result = grantResults[i];
  String perm = permissions[i];
  Logger.debug(getLogTag(), "Check permission: "+ perm +" = "+ result);
  if(result == PackageManager.PERMISSION_DENIED) {
    Logger.debug(getLogTag(), "User denied storage permission: " + perm);
    savedCall.error(PERMISSION_DENIED_ERROR);
    this.freeSavedCall();
    return;
  }
}

There the grantResults Array will be iterated and checked if any result contains "denied". If a denied permission is found the saved call is freed and an error is returned to the caller.

But if I take a look at the Android Activity.java SDK 28 @ line 5082 - 5084 (onRequestPermissionsResult)

// Dispatch the callback with empty arrays which means a cancellation.
onRequestPermissionsResult(requestCode, new String[0], new int[0]);
return;

Android also has the Option to cancel a request which is represented as an empty array.
If this occurs Filesystem won't iterate over the array and therefore does not find a permission denied and continues.

To catch this error locally in the filesystem in the node_modules, I simply added an additional check in the Filesystem logic before the for loop gets executed:

if (grantResults.length == 0) {
  Logger.debug(getLogTag(), "Permission results array is empty which means a cancellation.");
  savedCall.error(PERMISSION_DENIED_ERROR);
  this.freeSavedCall();
  return;
}

There I check if the array has a length of 0. If this is the case I handle the call as if Android would have denied the permission. For me this was enough, but it would be even nicer if there would be a different Error like "PERMISSION_CANCELLED_ERROR".
@jcesarmobile jcesarmobile added the needs reproduction needs reproducible example to illustrate the issue label Feb 8, 2021
@Ionitron
Copy link
Collaborator

Ionitron commented Feb 8, 2021

This issue may need more information before it can be addressed. In particular, it will need a reliable Code Reproduction that demonstrates the issue.

Please see the Contributing Guide for how to create a Code Reproduction.

Thanks!
Ionitron 💙

@Ionitron Ionitron added the needs reply needs reply from the user label Feb 8, 2021
@kaunstdadenga
Copy link
Author

Reproduction

Here I put together a quick and dirty sample where the bug is reproduceable: https://github.com/kaunstdadenga/capacitor-filesystem-bug-sample
It is the Ionic Sample App with the necessary code of our app to reproduce the bug.:

  • Simply click on the icon in the first tab page
  • a modal is opened where u can sign
  • click on the checkmark in the top right to save the signature
  • The permission request will pop up followed by an app crash

Documentation

Here is the official Android documentation where it is state in the "Note:" paragraph that it is possible that the request result might be empty, which should be treated as cancellation: https://developer.android.com/reference/androidx/core/app/ActivityCompat.OnRequestPermissionsResultCallback

Stacktrace

I have added the full stacktrace to the git repo as it would exceed the comments maximum characters. https://github.com/kaunstdadenga/capacitor-filesystem-bug-sample/blob/master/stacktrace.txt

Here is a part of the stacktrace of the endless loop:

2021-02-09 13:25:36.741 8822-8899/com.example.app W/Activity: Can reqeust only one set of permissions at a time
2021-02-09 13:25:36.742 8822-8899/com.example.app D/Capacitor/Filesystem: handling request perms result
2021-02-09 13:25:36.742 8822-8899/com.example.app V/Capacitor/Filesystem: Permission 'android.permission.READ_EXTERNAL_STORAGE' denied. Asking user for it.
2021-02-09 13:25:36.742 8822-8899/com.example.app W/Activity: Can reqeust only one set of permissions at a time
2021-02-09 13:25:36.743 8822-8899/com.example.app D/Capacitor/Filesystem: handling request perms result
2021-02-09 13:25:36.744 8822-8899/com.example.app V/Capacitor/Filesystem: Permission 'android.permission.READ_EXTERNAL_STORAGE' denied. Asking user for it.
2021-02-09 13:25:36.744 8822-8899/com.example.app W/Activity: Can reqeust only one set of permissions at a time
2021-02-09 13:25:36.745 8822-8899/com.example.app D/Capacitor/Filesystem: handling request perms result
2021-02-09 13:25:36.749 8822-8899/com.example.app E/Capacitor: Serious error executing plugin
    java.lang.reflect.InvocationTargetException
        at java.lang.reflect.Method.invoke(Native Method)
        at com.getcapacitor.PluginHandle.invoke(PluginHandle.java:99)
        at com.getcapacitor.Bridge$1.run(Bridge.java:542)
        at android.os.Handler.handleCallback(Handler.java:789)
        at android.os.Handler.dispatchMessage(Handler.java:98)
        at android.os.Looper.loop(Looper.java:164)
        at android.os.HandlerThread.run(HandlerThread.java:65)
     Caused by: java.lang.StackOverflowError: stack size 1037KB
        at java.util.Collections.emptyList(Collections.java:4467)
        at java.lang.Throwable.<init>(Throwable.java:220)
        at java.lang.Exception.<init>(Exception.java:66)
        at org.json.JSONException.<init>(JSONException.java:47)
        at org.json.JSONObject.get(JSONObject.java:392)
        at org.json.JSONObject.getBoolean(JSONObject.java:413)
        at com.getcapacitor.CapConfig.getBoolean(CapConfig.java:102)
        at com.getcapacitor.Logger.shouldLog(Logger.java:100)
        at com.getcapacitor.Logger.verbose(Logger.java:40)
        at com.getcapacitor.plugin.Filesystem.isStoragePermissionGranted(Filesystem.java:622)
        at com.getcapacitor.plugin.Filesystem.readFile(Filesystem.java:172)
        at com.getcapacitor.plugin.Filesystem.handleRequestPermissionsResult(Filesystem.java:673)
        at com.getcapacitor.Bridge.onRequestPermissionsResult(Bridge.java:763)
        at com.getcapacitor.BridgeActivity.onRequestPermissionsResult(BridgeActivity.java:206)
        at android.app.Activity.requestPermissions(Activity.java:4455)
        at androidx.core.app.ActivityCompat.requestPermissions(ActivityCompat.java:502)
        at com.getcapacitor.Plugin.pluginRequestPermissions(Plugin.java:277)
        at com.getcapacitor.plugin.Filesystem.isStoragePermissionGranted(Filesystem.java:623)
        at com.getcapacitor.plugin.Filesystem.readFile(Filesystem.java:172)
        at com.getcapacitor.plugin.Filesystem.handleRequestPermissionsResult(Filesystem.java:673)
        at com.getcapacitor.Bridge.onRequestPermissionsResult(Bridge.java:763)
        at com.getcapacitor.BridgeActivity.onRequestPermissionsResult(BridgeActivity.java:206)
        at android.app.Activity.requestPermissions(Activity.java:4455)
        at androidx.core.app.ActivityCompat.requestPermissions(ActivityCompat.java:502)
        at com.getcapacitor.Plugin.pluginRequestPermissions(Plugin.java:277)
        at com.getcapacitor.plugin.Filesystem.isStoragePermissionGranted(Filesystem.java:623)
        at com.getcapacitor.plugin.Filesystem.readFile(Filesystem.java:172)
        at com.getcapacitor.plugin.Filesystem.handleRequestPermissionsResult(Filesystem.java:673)
        at com.getcapacitor.Bridge.onRequestPermissionsResult(Bridge.java:763)
        at com.getcapacitor.BridgeActivity.onRequestPermissionsResult(BridgeActivity.java:206)
        at android.app.Activity.requestPermissions(Activity.java:4455)
        at androidx.core.app.ActivityCompat.requestPermissions(ActivityCompat.java:502)
        at com.getcapacitor.Plugin.pluginRequestPermissions(Plugin.java:277)
        at com.getcapacitor.plugin.Filesystem.isStoragePermissionGranted(Filesystem.java:623)
        at com.getcapacitor.plugin.Filesystem.readFile(Filesystem.java:172)
        at com.getcapacitor.plugin.Filesystem.handleRequestPermissionsResult(Filesystem.java:673)
        at com.getcapacitor.Bridge.onRequestPermissionsResult(Bridge.java:763)
        at com.getcapacitor.BridgeActivity.onRequestPermissionsResult(BridgeActivity.java:206)
        at android.app.Activity.requestPermissions(Activity.java:4455)
        at androidx.core.app.ActivityCompat.requestPermissions(ActivityCompat.java:502)
        at com.getcapacitor.Plugin.pluginRequestPermissions(Plugin.java:277)
        at com.getcapacitor.plugin.Filesystem.isStoragePermissionGranted(Filesystem.java:623)
        at com.getcapacitor.plugin.Filesystem.readFile(Filesystem.java:172)
        at com.getcapacitor.plugin.Filesystem.handleRequestPermissionsResult(Filesystem.java:673)
        at com.getcapacitor.Bridge.onRequestPermissionsResult(Bridge.java:763)
        at com.getcapacitor.BridgeActivity.onRequestPermissionsResult(BridgeActivity.java:206)
        at android.app.Activity.requestPermissions(Activity.java:4455)
2021-02-09 13:25:36.751 8822-8899/com.example.app E/Capacitor:     at androidx.core.app.ActivityCompat.requestPermissions(ActivityCompat.java:502)
        at com.getcapacitor.Plugin.pluginRequestPermissions(Plugin.java:277)
        at com.getcapacitor.plugin.Filesystem.isStoragePermissionGranted(Filesystem.java:623)
        at com.getcapacitor.plugin.Filesystem.readFile(Filesystem.java:172)
        at com.getcapacitor.plugin.Filesystem.handleRequestPermissionsResult(Filesystem.java:673)
        at com.getcapacitor.Bridge.onRequestPermissionsResult(Bridge.java:763)
        at com.getcapacitor.BridgeActivity.onRequestPermissionsResult(BridgeActivity.java:206)
        at android.app.Activity.requestPermissions(Activity.java:4455)
        at androidx.core.app.ActivityCompat.requestPermissions(ActivityCompat.java:502)
        at com.getcapacitor.Plugin.pluginRequestPermissions(Plugin.java:277)
        at com.getcapacitor.plugin.Filesystem.isStoragePermissionGranted(Filesystem.java:623)
        at com.getcapacitor.plugin.Filesystem.readFile(Filesystem.java:172)
        at com.getcapacitor.plugin.Filesystem.handleRequestPermissionsResult(Filesystem.java:673)
        at com.getcapacitor.Bridge.onRequestPermissionsResult(Bridge.java:763)
        at com.getcapacitor.BridgeActivity.onRequestPermissionsResult(BridgeActivity.java:206)
        at android.app.Activity.requestPermissions(Activity.java:4455)
        at androidx.core.app.ActivityCompat.requestPermissions(ActivityCompat.java:502)
        at com.getcapacitor.Plugin.pluginRequestPermissions(Plugin.java:277)
        at com.getcapacitor.plugin.Filesystem.isStoragePermissionGranted(Filesystem.java:623)
        at com.getcapacitor.plugin.Filesystem.readFile(Filesystem.java:172)
        at com.getcapacitor.plugin.Filesystem.handleRequestPermissionsResult(Filesystem.java:673)
        at com.getcapacitor.Bridge.onRequestPermissionsResult(Bridge.java:763)
        at com.getcapacitor.BridgeActivity.onRequestPermissionsResult(BridgeActivity.java:206)
        at android.app.Activity.requestPermissions(Activity.java:4455)
        at androidx.core.app.ActivityCompat.requestPermissions(ActivityCompat.java:502)
        at com.getcapacitor.Plugin.pluginRequestPermissions(Plugin.java:277)
        at com.getcapacitor.plugin.Filesystem.isStoragePermissionGranted(Filesystem.java:623)
        at com.getcapacitor.plugin.Filesystem.readFile(Filesystem.java:172)
        at com.getcapacitor.plugin.Filesystem.handleRequestPermissionsResult(Filesystem.java:673)
        at com.getcapacitor.Bridge.onRequestPermissionsResult(Bridge.java:763)
        at com.getcapacitor.BridgeActivity.onRequestPermissionsResult(BridgeActivity.java:206)
        at android.app.Activity.requestPermissions(Activity.java:4455)
        at androidx.core.app.ActivityCompat.requestPermissions(ActivityCompat.java:502)
        at com.getcapacitor.Plugin.pluginRequestPermissions(Plugin.java:277)
        at com.getcapacitor.plugin.Filesystem.isStoragePermissionGranted(Filesystem.java:623)
        at com.getcapacitor.plugin.Filesystem.readFile(Filesystem.java:172)
        at com.getcapacitor.plugin.Filesystem.handleRequestPermissionsResult(Filesystem.java:673)
        at com.getcapacitor.Bridge.onRequestPermissionsResult(Bridge.java:763)
        at com.getcapacitor.BridgeActivity.onRequestPermissionsResult(BridgeActivity.java:206)
        at android.app.Activity.requestPermissions(Activity.java:4455)
        at androidx.core.app.ActivityCompat.requestPermissions(ActivityCompat.java:502)
        at com.getcapacitor.Plugin.pluginRequestPermissions(Plugin.java:277)
        at com.getcapacitor.plugin.Filesystem.isStoragePermissionGranted(Filesystem.java:623)
        at com.getcapacitor.plugin.Filesystem.readFile(Filesystem.java:172)
        at com.getcapacitor.plugin.Filesystem.handleRequestPermissionsResult(Filesystem.java:673)
        at com.getcapacitor.Bridge.onRequestPermissionsResult(Bridge.java:763)
        at com.getcapacitor.BridgeActivity.onRequestPermissionsResult(BridgeActivity.java:206)
        at android.app.Activity.requestPermissions(Activity.java:4455)
        at androidx.core.app.ActivityCompat.requestPermissions(ActivityCompat.java:502)
        at com.getcapacitor.Plugin.pluginRequestPermissions(Plugin.java:277)
        at com.getcapacitor.plugin.Filesystem.isStoragePermissionGranted(Filesystem.java:623)

@Ionitron Ionitron removed the needs reply needs reply from the user label Feb 9, 2021
@jcesarmobile jcesarmobile mentioned this issue Feb 9, 2021
Merged
@jcesarmobile
Copy link
Member

Thanks for the sample app, the pr was merged, closing.

@ionitron-bot
Copy link

ionitron-bot bot commented Nov 11, 2022

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Capacitor, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Nov 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
needs reproduction needs reproducible example to illustrate the issue platform: android
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jcesarmobile @Ionitron @kaunstdadenga