apache / cordova-plugin-file

Apache Cordova File Plugin
https://cordova.apache.org/
Apache License 2.0
740 stars 757 forks source link

getDirectory and Android 13 #606

Closed petr-vytlacil closed 7 months ago

petr-vytlacil commented 7 months ago

Is any way ho use getDirectory on Android 13?

Example usage

fileSystem.root.getDirectory( 'Download', { create: true }, (dirEntry) => { console.log('createFile') log.info('saveBlob2File -> requestFileSystem', 'createFile') createFile(dirEntry, fileName, blob) }, (error) => { log.error(logSource, 'Directory Download could not be created.') log.error(logSource, error) const notificationObj = { message: logSource + ' | fileSystem.root.getDirectory | ' + ': error: ' + error.message, timeMark: new Date().getTime() } store.dispatch('appNotifications/setMessage', notificationObj) updateCircularProgressVisible(false) })

In FileUtils is else if (action.equals("getDirectory")) { where check permission but its for old Android API.

private void getWritePermission(String rawArgs, int action, CallbackContext callbackContext) { int requestCode = pendingRequests.createRequest(rawArgs, action, callbackContext); PermissionHelper.requestPermission(this, requestCode, Manifest.permission.WRITE_EXTERNAL_STORAGE); }

THX

breautek commented 7 months ago

Yes, I believe this needs to be API guarded...

API 29 and later doesn't need WRITE_EXTERNAL_STORAGE because scoped storage is in effect which removes the permission requirement. So on API 29 or later, I think we can simply return true...

https://github.com/apache/cordova-plugin-file/blob/5480a3e2932b5fec0d5554ba7123bfd510203a30/src/android/FileUtils.java#L616

private boolean hasWritePermission() {
-    return PermissionHelper.hasPermission(this, Manifest.permission.WRITE_EXTERNAL_STORAGE);
+   if (android.os.Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) {
+       return PermissionHelper.hasPermission(this, Manifest.permission.WRITE_EXTERNAL_STORAGE);
+   }
+
+   return true;
}

Honestly this whole permission checking can be further improved, since it seems like it's also requesting permissions on internal storage usage, which isn't necessary since API 19.

breautek commented 7 months ago

I believe #581 fixes this issue

But it tests for TIRAMISU (API 33) instead of P (API 29), which I think is fine because if we test for P like I originally suggested, it means we will actually lose out on the implicit read grant which is still necessary for API 29-32.

It would be great if you can give PR #581 a test to see if it solves your issue.

erisu commented 7 months ago

Should be resolved by https://github.com/apache/cordova-plugin-file/pull/608