MoveFile crash when the destination file already exists

Juan_Antonio · August 29, 2022, 11:23pm

MoveFile ASD/perro_50.png to ASD/animal.png works (the first time)

MoveFile ASD/gato_50.png to ASD/animal.png crash app because the animal.png file already exists.

Juan_Antonio · August 30, 2022, 11:24am

One solution would be to delete the destination file before moving it.
BTW, we can delete a file that doesn't exist without getting an error.

movefile_bug2

TIMAI2 · August 30, 2022, 11:28am

I have not looked at the sources, but when a file is created (such as with move) there should be a flag/option set (to "false") to allow an existing file to be overwritten.

Anke · August 30, 2022, 11:29am

Yes, this is another bug. You must

delete the existing file beforehand
or use one of the File extensions (from @Taifun or @vknow360).

Btw, the same with Niotron.

Anke · August 30, 2022, 12:27pm

Yes, but for whatever reason, that's not the case.

I tried TaifunFile with the async move block (and this works).

ewpatton · August 30, 2022, 1:03pm

I was just working on some file related issues yesterday so I'll try to see if we can get this taken care of.

ewpatton · August 30, 2022, 1:41pm

@Juan_Antonio Which version of Android are you testing on? There is different logic for Android 8 and later versus earlier versions of Android.

Anke · August 30, 2022, 2:01pm

I checked it now on Android 5, 9 and 11 (Companion).
Only on Android 5 it works.

Anke · August 30, 2022, 2:06pm

On Android 8.1.0 it does not work.

Juan_Antonio · August 30, 2022, 2:19pm

Android 9

Anke · August 30, 2022, 2:33pm

Why?

grafik955×151 5.97 KB

ewpatton · August 30, 2022, 3:02pm

A lot of the Java NIO functionality was added in Android O (SDK 26). The benefit of this functionality is that it can defer to the OS and filesystem level to perform the operation in the most effective way possible.

For earlier versions where we don't have this benefit, we've implemented the fallback logic needed to make it work. To do it safely we have to copy the file first in case that we need to roll things back in case of failure during the operation. Otherwise, we can end up in a situation where the target file is deleted but the move part fails and now data have been lost (which ideally should never be the case).

Anke · August 30, 2022, 3:29pm

Ok, but I've done it hundreds of times on dozens of devices and all Android versions with the TaifunFile extension and never had any problems.

ewpatton · August 30, 2022, 6:25pm

It's expected that the success condition is the most prevalent though. I think we'd be all hard pressed if moving files in any context was bound to fail more than succeed. The real challenge is accounting for the corner cases that might come up which is what the code for older Android versions represents. However, everyone has reported using a newer version of Android so the Java NIO path is the one that's taken. The issue was that we were not passing the necessary flag to overwrite the destination, and, as was pointed out, this could be worked around by deleting the destination first.

In any case, the change that fixes this issue is now an open PR on GitHub: