fix(android)!: avoid showing permissions prompt & migrate write location

[sterlingwes]

Overview

We were looking to this library for a way to share a base64-encoded PDF and noticed a couple things:

• in API 29, getExternalStorageDirectory is deprecated
• a cache storage location seems more appropriate for writing files since they're temporary in nature
• the permissions prompt isn't necessary since API 19 to these locations

This maybe solves #843

Test Plan

I've currently only tested my happy path - but if this seems like an OK approach, I'll test all the other methods available in the sample app.

BREAKING CHANGE: write location & permission prompt behaviour changed

[MateusAndrade]

Nice! Except for the suggestions, it looks great! There is any change that we should be aware since we are changing how the library handles files on android?

[MateusAndrade]

Thanks! Could you update your base branch with the master? Except from that LGTM!

[sterlingwes]

Thanks @MateusAndrade - I've tested all the other scenarios in the sample app and noticed the "Share to IG Story" one doesn't appear to be working. I suspect it's something to do with the FileProvider, but looking into it

[MateusAndrade]

Thanks @MateusAndrade - I've tested all the other scenarios in the sample app and noticed the "Share to IG Story" one doesn't appear to be working. I suspect it's something to do with the FileProvider, but looking into it

There is anything that I can help you with?

[sterlingwes]

I think I've narrowed it down to pre-existing behaviour. I checked out 1a80835 from before my changes and it behaves the same:

I notice in the logcat output an error with exif parsing which may indicate the problem:

2020-09-18 08:53:05.690 2035-4820/? I/ActivityTaskManager: START u0 {act=com.instagram.share.ADD_TO_STORY dat=content://com.example.rnshare.fileprovider/rnshare_sdcard/storage/emulated/0/Android/data/com.example/cache/Download/background.png typ=image/png flg=0x10000001 pkg=com.instagram.android cmp=com.instagram.android/com.instagram.share.handleractivity.CustomStoryShareHandlerActivity} from uid 10157
2020-09-18 08:53:05.722 3366-3450/com.example I/System.out: SHARE SINGLE METHOD
2020-09-18 08:53:05.728 3366-3450/com.example I/System.out: com.instagram.android
2020-09-18 08:53:06.043 2491-3263/? W/ExifInterface: Invalid image: ExifInterface got an unsupported image format file(ExifInterface supports JPEG and some RAW image formats only) or a corrupted JPEG file to ExifInterface.
    java.io.IOException: Invalid byte order: 1a0a
        at android.media.ExifInterface.readByteOrder(ExifInterface.java:3121)
        at android.media.ExifInterface.isOrfFormat(ExifInterface.java:2437)
        at android.media.ExifInterface.getMimeType(ExifInterface.java:2315)
        at android.media.ExifInterface.loadAttributes(ExifInterface.java:1753)
        at android.media.ExifInterface.initForFilename(ExifInterface.java:2297)
        at android.media.ExifInterface.<init>(ExifInterface.java:1384)
        at X.8GS.A00(:185)
        at X.8bO.A00(:12)
        at X.8bQ.run(:28)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
        at java.lang.Thread.run(Thread.java:919)
        at X.0Pv.run(:5)

It is passing the png mimetype with the intent but maybe this error indicates instagram is performing some default behaviour or doesn't recognize that mimetype

---

In any case, doesn't seem related to my changes

To your earlier question tho: I think the biggest thing to callout about the deprecated method change (getExternalStorageDirectory -> getExternalCacheDirectory) is that we're no longer placing our written shared files in a storage location outside of the app's scope. This has the benefit of being coupled to the cache storage location for the app, which is intentionally opaque so that the system can manage it or the user can easily clear the app cache. I think that's appropriate for our usage here since users aren't directly involved in the naming or save location of the files so it behaves more like a temporary cache. That said, if users of this library were relying on this behaviour it could be considered a breaking change. I don't see anywhere in the docs where we publicize this behaviour so I think it's safe to consider it non-breaking / internal.

[sterlingwes]

Actually this may be breaking if they had defined the FileProvider paths, since it requires a different child attribute:

<?xml version="1.0" encoding="utf-8"?>
<paths xmlns:android="http://schemas.android.com/apk/res/android">
    <external-cache-path name="cache" path="Download/" />
</paths>

I'm unable to find a case in the example app where this was required, though.

[MateusAndrade]

@sterlingwes we can go on merging this?

[sterlingwes]

@sterlingwes we can go on merging this?

I'll defer to you on this - I think this should be marked as a breaking change & have commensurate docs on how to migrate any usage of FileProvider in the android manifest

[sterlingwes]

I can update if you feel that's appropriate, otherwise feel free to merge I'm satisfied with my manual testing @MateusAndrade

[MateusAndrade]

I can update if you feel that's appropriate, otherwise, feel free to merge I'm satisfied with my manual testing @MateusAndrade

This would be amazing! I can merge this with a breaking change and a version 4.0.0 will be published. Would you mind updating our docs pointing that change?

[sterlingwes]

ok good to go on my end if you are @MateusAndrade 👍

[MateusAndrade]

Nice!!!!! Thanks a lot, @sterlingwes! 🚀 🚀 🚀 🚀 🚀 🚀

[MateusAndrade]

Nice!!!!! Thanks a lot, @sterlingwes! 🚀 🚀 🚀 🚀 🚀 🚀

[MateusAndrade]

🎉 This PR is included in version 3.8.4 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[MateusAndrade]

For anybody looking after 4.0.0 due to a mistake merging this, I triggered a fix that ended without updating this to a major version. I fixed this pushing some commits forcing semantic-release to trigger a v4.0.0, which can be seen here: https://github.com/react-native-community/react-native-share/releases/tag/v4.0.0

[marcorm]

Hi all, is there a way to force v4 to keep saving files into Download directory?

[MateusAndrade]

Hi all, is there a way to force v4 to keep saving files into Download directory?

Did you try adding this config to your FileProvider: https://react-native-share.github.io/react-native-share/docs/migrate-v3-to-v4#getexternalstoragedirectory---getexternalcachedir?

[imdaniele]

Hi all, is there a way to force v4 to keep saving files into Download directory?

Did you try adding this config to your FileProvider: https://react-native-share.github.io/react-native-share/docs/migrate-v3-to-v4#getexternalstoragedirectory---getexternalcachedir?

Hi,
by adding that config to FileProvider on Manifest I can't open that file by using an external file browser.

Is there a way to save the file in the public Download directory so the user could open it using any file browser?

[sterlingwes]

@marcorm @imdaniele can you confirm which APIs you're using to share / save the files? Are they base64'd? Which external app(s) specifically are trying to access the file and unable to do so?

Specific reproduction steps would help in ensuring this workflow is in the sample app and can be tested

[imdaniele]

Hi @sterlingwes ,
I'm just using

const base64Data = `data:application/pdf;base64,${base64Doc}`
await Share.open({ url: base64Data, type: 'application/pdf' })

without a FileProvider.
I also tried using a FileProvider as suggested in the guide, but the result is the same.

The shared file can't be listed as a media file, so it's not visible to the user.

In the previous version the

Environment.getExternalStorageDirectory()

was accessible by the user (in the "Download" directory), but with the current release files inside

getExternalCacheDir()

are only visible to the application.

Returns absolute path to application-specific directory on the primary shared/external storage device where the application can place cache files it owns. These files are internal to the application, and not typically visible to the user as media.
https://developer.android.com/reference/kotlin/android/content/Context?hl=en#getexternalcachedir

[sterlingwes]

Thanks @imdaniele for the extra info!

It sounds like you might have been relying on an internal implementation detail of this lib? Where the file is saved in order to facilitate the share sheet is a byproduct of the main API use case, which is to share a file with another application. Or at least that's how I understand this lib's set of APIs.

I'll defer to the core maintainers here but I feel like this should be opened as a separate issue proposing an API surface change. Perhaps Share.open can take an option that specifies the desired save destination?