hpoul / file_picker_writable

Flutter plugin to choose files which can be read, referenced and written back at a later time.
https://pub.dev/packages/file_picker_writable
MIT License
17 stars 13 forks source link

Android fix crash when requesting persistable permissions (mostly for ACTION_VIEW intent) #1 #2

Closed amake closed 4 years ago

amake commented 4 years ago

This is my current solution to #1. I would love for there to be a better way :/

hpoul commented 4 years ago

thanks for the PR, to be honest i was thinking more or less the same thing, when I read your issue.. i guess if the app starting the intent does not provide a persistable permission ,there is no way to get it anyway.. so the best thing we could do is catch the exception?

but I would still use the "identifier" = fileUri .. no matter if we have a persistable permission or not.. (because it's still possible that we are able to write into it, if it's a public place on the sd card or something)

amake commented 4 years ago

The reason I chose this path is because of my use case: I want to offer a "recently used files" view in my app, so I only want to remember files that I can open later. If identifier is present despite failing to get persistent permissions, then I would need a separate field in order to determine whether it's "safe" to remember the file.

Otherwise I will end up showing the user a file that they can't open.

Would you prefer a persistable bool field instead of a null identifier? If so I am happy to make the change.

hpoul commented 4 years ago

i'm just not sure how to know if we can access the file later.. I guess it's possible to check the uri scheme? because i'm pretty sure if it's a file:/// URI pointing to a location on the SD card, the app should be able to read it at all times (when it has the READ_EXTERNAL_STORAGE permission)

amake commented 4 years ago

Starting in Android 7 you basically never have file:// URIs; instead you have content:// URIs (see here). The permissions on these are not necessarily persistable: you can access the URI while your app is open, but on next launch access will fail.

So it is necessary to know if takePersistableUriPermission succeeded or not.

hpoul commented 4 years ago

makes sense.. i lean toward a boolean flag, because I think it would still be possible to read the file again using the "identifier" within the same session? not sure how long the "temporary permission" lasts?

maybe there is a way to ask for permissions at a later time.. the best thing I could find would be using the document chooser with EXTRA_INITIAL_URI

although it's a bit awkward API wise.. i guess I would keep Map<String, String> for now and just use "true" and do the type conversion in dart.. because i've been thinking about giving pigeon a try, so no need to do a lot of cleanup now..

btw. I figured out why this worked for me.. i always used the app itself to save a file (e.g. a .codeux file in the example app).. and then used the Files app to open those files.. that's why my app still had persistable permission for the file :-) I could only reproduce the crash once i've moved the file in the Files app ..(or created the file through some other means)

amake commented 4 years ago

makes sense.. i lean toward a boolean flag, because I think it would still be possible to read the file again using the "identifier" within the same session? not sure how long the "temporary permission" lasts?

Sure, that sounds good. The permission lasts while the receiving process lives, so you can reopen as much as you want until you e.g. kill your app and relaunch it.

although it's a bit awkward API wise.. i guess I would keep Map<String, String> for now and just use "true" and do the type conversion in dart.. because i've been thinking about giving pigeon a try, so no need to do a lot of cleanup now..

OK, sounds good. I can update the PR with this.

btw. I figured out why this worked for me.. i always used the app itself to save a file (e.g. a .codeux file in the example app).. and then used the Files app to open those files.. that's why my app still had persistable permission for the file :-) I could only reproduce the crash once i've moved the file in the Files app ..(or created the file through some other means)

Yeah makes sense. My use case is quite different: my app is a viewer and never creates its own files, so I ran into this right away.

hpoul commented 4 years ago

OK, sounds good. I can update the PR with this.

👍️ would be great, just make it a Map<String, String> for now with like hasPersistablePermission or something.. (ie. meaning that the identifier is probably still identifying that file, just that it can't be accessed without acquiring permissions again) , i'll later try out if pigeon could make the messenger calls typesafe.

fwiw, on iOS it is called security scoped bookmark so hard to unify that naming.. but i'm also not completely sure if on iOS acquiring a security scoped bookmark could actually fail..

amake commented 4 years ago

I force-pushed my changes last night. Let me know if you want the wording or anything changed.

The iOS one can fail but maybe only if you didn’t set your Info.plist entries correctly, so we can consider that a programmer error.

hpoul commented 4 years ago

ups. missed your push.. thanks.. i've merged it, thanks (it's now on pub as 1.1.1+2)