miguelpruivo / flutter_file_picker

File picker plugin for Flutter, compatible with mobile (iOS & Android), Web, Desktop (Mac, Linux, Windows) platforms with Flutter Go support.
MIT License
1.3k stars 648 forks source link

(OS Error: Permission denied, errno = 13) on Android Q #169

Closed lifenautjoe closed 4 years ago

lifenautjoe commented 4 years ago

Describe the bug

We've received and seen several crashes related to permission denied trying to open a file that was picked on Android Q devices.

Looking for solutions, came across this https://github.com/flutter/flutter/issues/31122#issuecomment-485429597

where it's claimed that

WRITE_EXTERNAL_STORAGE is being deprecated in Q.

and might the cause for issues.

Perhaps something to update in regards to how permissions are asked to the user on Android devices?

The strange thing is that users CAN pick the file with the modal, it's after we programatically try to access it's contents, we get the exception.

Issue details

  1. Platform Android
  2. Q
  3. IMAGE
  4. Try to pick an image from your SD card.

Error Log

FileSystemException: FileSystemException: Cannot open file, path = '/storage/emulated/0/WhatsApp/Media/WhatsApp Images/IMG-20180519-WA0002.jpg' (OS Error: Permission denied, errno = 13)
  File "file_impl.dart", line 643, in _File.throwIfError
  File "file_impl.dart", line 487, in _File.openSync
  File "file_impl.dart", line 547, in _File.readAsBytesSync
  File "media.dart", line 64, in MediaService.pickImage
  File "<asynchronous suspension>"
  File "edit_user_profile.dart", line 360, in OBEditUserProfileModalState._showImageBottomSheet.<fn>.<fn>
  File "<asynchronous suspension>"
  File "ink_well.dart", line 701, in _InkResponseState._handleTap
  File "ink_well.dart", line 783, in _InkResponseState.build.<fn>
  File "recognizer.dart", line 182, in GestureRecognizer.invokeCallback
  File "tap.dart", line 486, in TapGestureRecognizer.handleTapUp
  File "tap.dart", line 282, in BaseTapGestureRecognizer._checkUp
  File "tap.dart", line 236, in BaseTapGestureRecognizer.acceptGesture
  File "arena.dart", line 156, in GestureArenaManager.sweep
  File "binding.dart", line 222, in GestureBinding.handleEvent
  File "binding.dart", line 198, in GestureBinding.dispatchEvent
  File "binding.dart", line 156, in GestureBinding._handlePointerEvent
  File "binding.dart", line 102, in GestureBinding._flushPointerEventQueue
  File "binding.dart", line 86, in GestureBinding._handlePointerDataPacket
  File "zone.dart", line 1136, in _rootRunUnary
  File "zone.dart", line 1029, in _CustomZone.runUnary
  File "zone.dart", line 931, in _CustomZone.runUnaryGuarded
  File "hooks.dart", line 273, in _invoke1
  File "hooks.dart", line 182, in _dispatchPointerDataPacket

Flutter Version details

[✓] Flutter (Channel master, v1.10.15-pre.307, on Mac OS X 10.14.6 18G95, locale
    en-NL)
    • Flutter version 1.10.15-pre.307 at
      /Users/lifenautjoe/Documents/code/libraries/flutter
    • Framework revision 2cedd559bb (4 days ago), 2019-10-29 00:43:06 -0400
    • Engine revision 419f5d594a
    • Dart version 2.6.0 (build 2.6.0-dev.8.2 e1fce75301)

[✓] Android toolchain - develop for Android devices (Android SDK version 28.0.3)
    • Android SDK at /Users/lifenautjoe/Library/Android/sdk
    • Android NDK location not configured (optional; useful for native profiling
      support)
    • Platform android-29, build-tools 28.0.3
    • Java binary at: /Applications/Android
      Studio.app/Contents/jre/jdk/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1248-b01)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 11.0)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 11.0, Build version 11A420a
    • CocoaPods version 1.8.3

[✓] Android Studio (version 3.3)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin version 33.4.1
    • Dart plugin version 182.5215
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1248-b01)

[✓] IntelliJ IDEA Ultimate Edition (version 2018.3.5)
    • IntelliJ at /Applications/IntelliJ IDEA.app
    • Flutter plugin version 33.3.2
    • Dart plugin version 183.5912.23

[✓] Connected device (1 available)
    • 🥑 • 1d548d079169fde4d329284ea2448a5b8982eaf3 • ios • iOS 13.1.3
miguelpruivo commented 4 years ago

Hi @lifenautjoe, this is a no-issue and actually what is happening is that the picker is working as it should by allowing users to pick files from wherever they are allowed to (and that's why it works). What you can't do tho, is writing files from directories that you don't have permissions, such as that one '/storage/emulated/0/WhatsApp/Media/WhatsApp which is from WhatsApp app.

Even though you are free to pick the files you want and use it to display some data, for example, as a developer, if you are going to write some file, you need to make sure that you write/move the files to your app's content folder or some cache directory.

Regarding the WRITE_EXTERNAL_STORAGE being deprecated on Q, the plugin doesn't have it at all. It's on the wiki because older SDK Android versions may actually need that, but it won't hurt if you have it as well. 🙂

I'm closing this as it doesn't represents an issue, but feel free to keep the discussion here if you still have any question.

Thank you.

lifenautjoe commented 4 years ago

Hmm we're not trying to write to the file nor the directory it's in, we're actually just trying to read it with pickedImage.readAsBytesSync() as seen here

https://github.com/OkunaOrg/okuna-app/blob/release/0.0.55/lib/services/media.dart#L64

this library is used on the showImagePicker just before this. It shows a bottom sheet asking to select source, and returns the picked file

I'll try copying the file somewhere else before even trying to read it (?)

miguelpruivo commented 4 years ago

@lifenautjoe that's probably because that ImageConverter is writing on the same directory from which is picked and it can't. It's up to the developer to decide where to write the files and since you are creating a copy (converted), you should place it in a temp directory or your own app content directory.

Like I said, FilePicker gives you the file as it is, this is, from its original directory (whenever possible) but that doesn't mean that you can actually write on it and you shouldn't assume that because there are a lot of folders on Android that can be accessible to read but not write.

In iOS you won't have this issue because the native picker delegate already forces a cache copy to the app's content.

lifenautjoe commented 4 years ago

hah, didn't think about that one, will give it a try :-)

You should get a buymeacoffee page Miguel, we'd be happy to send some coffees for your time and help!

miguelpruivo commented 4 years ago

No need to thank me @lifenautjoe, always glad to help! 🙂 Just because of you, I've created one here ☕️ 😄

Thank you!

lifenautjoe commented 4 years ago

Cool! I've sent some coffee's your way ☕️ .

In regards to the issue we're having, we've changed the code to intermediately copy the file to a directory we have permissions with getApplicationDocumentsDirectory(), still seeing the same issues.

FileSystemException: FileSystemException: Cannot copy file to '/data/user/0/social.openbook.app/cache/mediaCache/fd3e9541-7ef2-40b9-85af-18373f02e208VID_20191102_164450.mp4', path = '/storage/emulated/0/DCIM/Camera/VID_20191102_164450.mp4' (OS Error: Permission denied, errno = 13)
  File "create_post.dart", line 609, in OBSavePostModalState._onError
  File "async_patch.dart", line 43, in _AsyncAwaitCompleter.start
  File "create_post.dart", line 593, in OBSavePostModalState._onError
  File "create_post.dart", line 410, in OBSavePostModalState._getImagePostActions.<fn>
  File "async_patch.dart", line 78, in _asyncErrorWrapperHelper.<fn>
  File "zone.dart", line 1144, in _rootRunBinary
  File "zone.dart", line 1037, in _CustomZone.runBinary
  File "future_impl.dart", line 151, in _FutureListener.handleError
  File "future_impl.dart", line 690, in Future._propagateToListeners.handleError
  File "future_impl.dart", line 711, in Future._propagateToListeners
  File "future_impl.dart", line 530, in Future._completeError
  File "async_patch.dart", line 36, in _AsyncAwaitCompleter.completeError
  File "media.dart", line 0, in MediaService.pickVideo
  File "async_patch.dart", line 71, in _asyncThenWrapperHelper.<fn>
  File "zone.dart", line 1132, in _rootRunUnary
  File "zone.dart", line 1029, in _CustomZone.runUnary
  File "future_impl.dart", line 137, in _FutureListener.handleValue
  File "future_impl.dart", line 678, in Future._propagateToListeners.handleValueCallback
  File "future_impl.dart", line 707, in Future._propagateToListeners
  File "future_impl.dart", line 522, in Future._completeWithValue
  File "async_patch.dart", line 30, in _AsyncAwaitCompleter.complete
  File "async_patch.dart", line 288, in _completeOnAsyncReturn
  File "media.dart", line 0, in MediaService._getTempPath
  File "async_patch.dart", line 71, in _asyncThenWrapperHelper.<fn>
  File "zone.dart", line 1132, in _rootRunUnary
  File "zone.dart", line 1029, in _CustomZone.runUnary
  File "future_impl.dart", line 137, in _FutureListener.handleValue
  File "future_impl.dart", line 678, in Future._propagateToListeners.handleValueCallback
  File "future_impl.dart", line 707, in Future._propagateToListeners
  File "future_impl.dart", line 522, in Future._completeWithValue
  File "future_impl.dart", line 552, in Future._asyncComplete.<fn>
  File "zone.dart", line 1124, in _rootRun
  File "zone.dart", line 1021, in _CustomZone.run
  File "zone.dart", line 923, in _CustomZone.runGuarded
  File "zone.dart", line 963, in _CustomZone.bindCallbackGuarded.<fn>
  File "schedule_microtask.dart", line 41, in _microtaskLoop
  File "schedule_microtask.dart", line 50, in _startMicrotaskLoop

With the code

    final tempPath = await _getTempPath();

    final String processedImageUuid = _uuid.v4();
    String imageExtension = basename(pickedImage.path);

    // The image picker gives us the real image, lets copy it into a temp path
    pickedImage =
        pickedImage.copySync('$tempPath/$processedImageUuid$imageExtension');

I've found this issue on the image picker package which might be related https://github.com/flutter/flutter/issues/41459

People suggest removing some legacy flag as seen here https://github.com/flutter/flutter/issues/41459#issuecomment-536323793 or downgrading to target sdk 28 instead of the latest 29 as a workaround.

If it's really a bug, perhaps an interesting thread to follow for the fix on this library too.

miguelpruivo commented 4 years ago

Hi, thank you @lifenautjoe 🙌

This is really weird, there must be something missing there, if this was really an issue with the plugin, it should have affected a lot of other users already. 🤔

Could you provide the full steps and details to replicate? I’ll try to make it happen on my machine. Also, does it happens in the emulator or only in some real devices? If you could make it happen on any emulator, please, give me the details/steps of it as well.

Thank you!

lifenautjoe commented 4 years ago

Hi Miguel, it'll happen if you're on Android Q | 10.

And it will be solved by adding the following on the manifest file.

    <application
            android:requestLegacyExternalStorage="true"

Reading a bit more, looks like read external storage and write external storage permissions are being deprecated.

https://commonsware.com/blog/2019/06/07/death-external-storage-end-saga.html

And instead, there's scoped external storage with its respective methods : getExternalFilesDir(), getExternalCacheDir(), getExternalMediaDirs(), getExternalCacheDirs(), and getExternalFilesDirs() for storing files instead of the getTemporaryDirectory and getApplicationDocumentsDir

There's a new release of the path_provider plugin which chooses between the old and new directories but for some reason its breaking people's apps as well...

https://github.com/flutter/flutter/issues/35783

miguelpruivo commented 4 years ago

@lifenautjoe yes, I'm familiar with it being deprecated because they want us to always handle files using URI instead of real paths, and in a way, that's safer and much more easier that just covering all file path possibilities between SDK [16, 29]. However, for Flutter users, that's not very convenient as they need a lot of times real paths to display content on Flutter's side, that's why I've been keeping it.

If this is a thing, I don't bother add that flag to the File Picker package as well by leaving the developer with another thing to not miss. However, I'd like to replicate it here first. Could you make it happen on an emulator and provide me with the full steps?

Thank you once more.

miguelpruivo commented 4 years ago

@lifenautjoe ok, after digging a bit more around this, it seems that the issue is indeed with the path_provider package and not this one. So, I'll break it down for you for better understanding:

  1. You are picking the files with file_picker which gives you the file path without any issue, right? So, file_picker gets its job done.
  2. Now, you are trying to make a copy of the original file to a temporary directory, and, because of that, you are using path_provider or any other path utility plugin.
  3. An exception is thrown due to no writing permissions (remember that with the file_picker we are not writing anything here, right?)
  4. You added the android:requestLegacyExternalStorage="true", which is required to use the SDK scope, and it works because it's picking the old path (without the social.openbook.app). That flag is only affecting the path_provider, thus, making it work.

TL;DR: Sometimes is hard to delegate the issue to the right plugin, specially when you are using multiple plugins for one operation. However, like I said, if this was really an issue with file_picker I would have probably found it already when testing on Android Q or other users reported it, because it's a very common use case.

Again, after further investigation, I'm closing this, but it's always good to keep on track of this issues. If there's still anything that you didn't get, just let me know.

Thank you, I guess I'll get some coffee now before they cool down! 👀 ☕️

lifenautjoe commented 4 years ago

Thank you for the nice breakdown of the issue and help as usual Miguel!

Good rest of the weekend! ☕️ ☀️

ovidiu-anghelina commented 4 years ago

Guys, I'm not convinced that you have found any long-term solution to the real problem here.

After doing a lot of reading on what Android 10's new scoped storage implies, I came to the following conclusions:

The app I'm working on has 2 use cases that require refactoring for scoped storage compatibility:

I would recommend the following to make file_picker compliant with scoped storage:

miguelpruivo commented 4 years ago

@ovidiu-anghelina thank you for the huge feedback. I agree with both of your suggestions and I may added those in a near future, just need to have the time. 🙂

ovidiu-anghelina commented 4 years ago

Is it maybe worth reopening this issue in order to improve its visibility and make it easier to keep track of it?

miguelpruivo commented 4 years ago

Sure. I’m already wiring up some solution for this @ovidiu-anghelina.

Veteraanikoodari commented 4 years ago

Hi Miguel, it'll happen if you're on Android Q | 10.

And it will be solved by adding the following on the manifest file.

... <application android:requestLegacyExternalStorage="true"

Can confirm. Had the exact same issue and the suggested (temporary) solution worked perfectly. It was really surprising to see that I could pick a file, query about it's existence but exception (Error 13) )was thrown as soon as I tried to actually read the contents of the file. (Android 10. OnePlus 7Pro).

BTW: Miguel, thx for great plugin! It rocks!

devjeff commented 4 years ago

One additional comment from my observations: Within your plugin you use the method "Environment.getExternalStorageDirectory()". According to the [docs](https://developer.android.com/reference/android/os/Environment.html#getExternalStorageDirectory()), the method is depcreacted and the path returned by this method is not accessible by apps. Thus, even reading files from this folder using Flutter/Dart file methods will fail.

In case an image from the Camera folder is selected via file_picker, the internally resolved URI is 'content://media/external/images/media' (via MediaStore.Images.Media.EXTERNAL_CONTENT_URI) and the actually returned path is '/storage/emulated/0/DCIM/Camera/.jpg (via context.getContentResolver())'. Using 'File(path).existsSync()' in Flutter for the returned path yields false. From my point of view, this is also related to the scoped storage changes. Apparently the actual file path (even from an externally visible file) is no longer accessible.

It is really a big pain now to use files access in Flutter in combination with the scoped storage changes. I hope, you'll be able to update your plugin to help cover these issues. Thanks in advance.

miguelpruivo commented 4 years ago

Does this still happen with V2 embedded support? (file_picker: 1.5.0+2)

Thank you.

dokinkon commented 4 years ago

@miguelpruivo It still happen on my Android10 device (pixel3) with file_picker 1.5.0+2 java.io.FileNotFoundException: /storage/emulated/0/SendAnywhere/Square_LDAC_v1.1.8_DFU_1068.HEX: open failed: EACCES (Permission denied)

miguelpruivo commented 4 years ago

@dokinkon ok, might have forgot something that should fix your issue. Please, add the following

android:requestLegacyExternalStorage="true"

to your <application> of your Android manifest file.

It should look like this:

<application
        android:name="your app bundle"
        android:label="your app name"
        android:requestLegacyExternalStorage="true"

And let me know if it worked for you. If so, this should be added onto the Wiki.

This happens because Android 10 (Q) or above, require new scoped file system and won't work with legacy unless this flag is added.

Anyway, don't forget to flutter clean before running after adding this.

Expecting to hear a feedback from you soon.

Thanks!

miguelpruivo commented 4 years ago

@dokinkon have you already tried? Nevertheless, added it to the docs.

Let me know so we can further close this issue. Thank you!

ovideozn commented 4 years ago

@miguelpruivo Sorry but how does adding the legacy flag help with closing this issue? As mentioned before, the legacy flag is only a temporary workaround, which will stop working as soon as Android 11 is released, regardless of the target SDK version. Any solution relying on the legacy flag is just a temporary solution.

miguelpruivo commented 4 years ago

@ovideozn no one knows if it will be deprecated on Android 11 or not, sometimes deprecated API's work until later versions. However, a problem at a time. Flutter needs absolute paths to handle Files, there's no way a Dart File will use Android URI's.

I've been drafting a solution for it by wrapping it in a custom class that could manage both absolute paths and URI's, but iOS will always behave differently and that's one more thing for the dev to handle a less abstraction.

For now, having the flag will solve this issue as it offers a concrete solution or temporary workaround, if you prefer. I'll soon open a new issue regarding this with the motivation for scoped storage handling.

ovideozn commented 4 years ago

@miguelpruivo Fair enough.

Would the solution I mentioned above not work though?

Copy all selected files to the scoped cache dir before returning the paths of the copied files, in the same way you currently do for remote Uris using getUriFromRemote. This way Flutter devs can continue to use file paths across Android and iOS and not have to deal with Android Uris.

and

Offer an alternative of returning Uris instead of paths for those that don't want the files to be automatically copied to the cache folder.

I still think this is better than creating a custom class that wraps everything, as it would either have to extend File, which would require long term maintenance, or it wouldn't extend File in which case it will be difficult to work with in projects that already use Files extensively.

miguelpruivo commented 4 years ago

@ovideozn first one will work, however, I'm not sure if some devs will starting open issues because the paths are not real but copies instead — which already happened when URI can't be resolved to a path and a temporary copy has to be made.

Second one, will match my purposed solution and that's probably the way to go. But I can agree that having files through a URI only and an optional method, such as getAndroidLegacyPath() or so, can be more verbose.

Thanks for the input!

miguelpruivo commented 4 years ago

Closing in favor of #234.

braysonjohn148 commented 4 years ago

FAILURE: Build failed with an exception.

miguelpruivo commented 4 years ago

@braysonjohn148 you are using requestLegscyStorage flag for a project that probably isn't targeting sdk 29.

Either change your target to it or remove the flag from your manifest file.