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.

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

writeFile() not truncating file contents on Android 10 #837

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SilverInfinity
Copy link

Fix for #700
As of Android 10, passing just "w" will no longer truncate the file after writing. You must also explicitly pass the truncate option. Previous versions of android treated "w" and "wt" as the same.

Changes:

  • send write/truncate option as the mode to openOutputStream on Android when append is false.

@Evyraad
Copy link

Evyraad commented Feb 24, 2020

@SilverInfinity hello.
Are you sure that we can use "wt" mode? I'm asking because I can't find this mode in Android documentation. But your fix works.

@yohannpoli
Copy link

yohannpoli commented Feb 24, 2020

Any news on that PR please ? Can you please merge it?

@SilverInfinity
Copy link
Author

SilverInfinity commented Feb 26, 2020

@Iyoss
You're right, it's not directly in the documentation. I came across it while trying to find more detail on the modes.
ContentResolver.openOutputStream creates an AssetFileDescriptor using openAssetFileDescriptor, then calls openOutputStream on that.
The openAssetFileDescriptor method's mode parameter references ContentProvider#openAssetFile for available parse modes.
Here, I found a bit more description:

mode: String: Access mode for the file. May be "r" for read-only access, "w" for write-only access (erasing whatever data is currently in the file), "wa" for write-only access to append to any existing data, "rw" for read and write access on any existing data, and "rwt" for read and write access that truncates any existing file. This value must never be null.

However, this appears to not be correct, as on android 10, passing "w" for write-only access is not in fact erasing whatever data is currently in the file.

This method linked out to ContentProvider.openFile which mention's ParcelFileDescriptor.parseMode(String)
Its here that I found all of the modes listed:

mode String: The string representation of the file mode. Can be "r", "w", "wt", "wa", "rw" or "rwt".

That, coupled with this android bug ticket, which mention's needing to pass 't' in the open mode is where I got the "wt" file mode.
I poked around the Android source code for different versions, and found that before Android 10, "w" and "wt" were explicitly being parsed exactly the same in ParcelFileDescriptor, resulting in a bitmask with both MODE_WRITE_ONLY and MODE_TRUNCATE to open the file. Then in Android 10, there was a change and 'w' and 't' are now parsed individually.

To answer your question, looking back on it, I am not entirely sure. The documentation on its own did not appear to be entirely clear on it, or contradictory in places. I am sure that it works for absolute file paths, such as files stored in RNFSDocumentDirectoryPath as is the use case for my application where i noticed the issue. Also tested on multiple versions of android. However I haven't tested it with content uris. If it is a content uri, then it looks like the Content Resolver or Content Provider could be overwritten with a custom implementation? I am not sure what to do in such a case.

I can change "wt" to "rwt". "rwt" also fixes the issue, and is explicitly mentioned in the android documentation. However, that may require adding READ_EXTERNAL_STORAGE permissions where it was previously not required. I was trying to avoid that.

@SilverInfinity
Copy link
Author

As I have explained above, Would you like me to edit this PR to use "rwt" or leave it as "wt"? Both options solve the issue.

I have been using a forked branch in production for 2 months now, and have seen no issues in using "wt". However our use case is very simple, only writing txt and image files to the document directory.
I don't have the time to test other scenarios myself. Since "rwt" is explicitly mentioned in the documentation, if you would rather use that, then I will update this PR.

@itinance
Copy link
Owner

Anything we can do here to find a final solution for your discussion, guys?

@creambyemute
Copy link

@itinance I think the solution proposed here and in #890 fits.

In our project we now have to use forks of redux-persist-fs-storage and react-native-fs due to the Android 10 problem. Otherwise itt pretty much rendered our application unusable.

@JordanDuncan
Copy link

JordanDuncan commented Oct 6, 2020

I can verify that this fixes an issue I was having on an app that was using redux-persist-fs.

Any time a reducers content was reduced in size and commited to disk on android 10, the app would be unable to rehydrate any data for that reducer as it left non-valid JSON in the persistors file.

This patch has fixed the android 10 issue and works as expected on previous versions (physically tested on 9 and 8).

It would be ideal to get this merged, currently using patch-package in my repo to apply the fix.

@exotexot
Copy link

exotexot commented Nov 17, 2020

I also would like to see this merged.
Using the "wt" method as mentioned here.

@gnardini
Copy link

Just one more person sharing that they need to use 2 forks because of this, please merge 😄

@sh-zam
Copy link

sh-zam commented Feb 10, 2021

Hello @SilverInfinity

Disclaimer: I found this issue while researching. I don't use this library or react native.

As I have explained above, Would you like me to edit this PR to use "rwt" or leave it as "wt"?

We used "wt", after sometime users reported that they can no longer save the files. This specifically happen to files from Google Drive. The error we were getting is: java.io.FileNotFoundException: Unsupported mode: wt. The problem was fixed by switching to "rwt".

Again, I don't know this library but I felt like reporting it as it seemed relevant :)

@ashughes
Copy link

@sh-zam Thanks for the heads up about "wt" not working with Google Drive. I was able to reproduce this as well.

I just filed a bug for this on the Android issue tracker. Hopefully, someone will follow up with more guidance and updated documentation.

FYI, I'm an Android developer, not a react native developer or user of this library. I also got here while researching this issue.

@FeXd
Copy link

FeXd commented Mar 23, 2021

I just spent waaaaaaay too much on a bug caused by this. Can we please have an update on gettings this approved?

@gaodeng
Copy link

gaodeng commented May 22, 2021

any news?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet