Skip to content

Rename is still deleting files, cf. bug #1576 #4445

@knakamura8

Description

@knakamura8

Encountered the exact same issue on a colleague's device, as issue #1576

Rename operation can silently fail, and will delete the file, as described here:

#1576 (comment) (comment 506373020)
#1576 (comment) (comment 526959369)

Do not have time to write a test/fix, but it appears as though only ShellNotRunningException is being handled(?), and merely silently warned in the log? What about IOException or SecurityException? These types of file system operations are not always safe on Android (many used to have to be move + delete to ensure integrity), and, though they typically only fail across mount points or things of that nature, there are other situations in which they may be raised.

Looking at the logs posted in that ticket, I noticed avc: denied { read } which would raise as IOException and/or SecurityException. It's been a bit, but I think this is related to SELinux and how scoped storage permissions are handled. These exceptions aren't being caught, so Amaze thinks it has read access to the file it is about to rename, when it does not. Fortunately, that means that if the exception is properly handled, then Amaze can fallback automatically to a safe rename routine (i.e. copy + hash check), or fail gracefully, because read access and security permission can be interrogated by Amaze prior to making the file system rename call.

Regardless, deleting a user's file when they simply wish to rename it is a situation that should obviously never occur when the user is using a file manager.

I believe what is happening is that users here can encounter some IOException, SecurityException, or other type, and this exception is not being caught or handled (see the log analysis part I typed earlier). For an operation as critical as this when handling user files, all exceptions should be caught. Better yet, perhaps have a user option for "safe rename" that always performs a copy first to ensure that any Android implementation of the rename routine will not delete the original file. It does not appear to be Amaze handling this delete, but some Android implementations of rename that are performing this delete.

Because of this, I think that having a slower "safe rename" does not need to be the default (though an option in advanced settings for the user would be nice), but there should be better exception handling to catch file IO errors and SELinux errors prior to applying potentially destructive operations on user files. But if an error is raised, then perform the safe rename, i.e. with the program flow here being copying the original file to a newly named file, then performing a hash check with the original file prior to deleting the original file. As with any other operation, if this proves unsuccessful, display a popup for the user saying the exception type and reasoning as to why the file cannot be renamed.

Also, to be clear, this isn't really the fault of Amaze, these types of "basic file system operations that should be atomic and rollbackable but aren't" have plagued Android since its inception. Gingerbread and Honeycomb had some nasty bugs in Android file system operations. Regardless, I think this is still a rather critical bug.

Oh and I suppose technically for version of Android 10+, there could be interfacing with the storage access framework, but then there would need to be multiple codepaths to maintain, so I think it is preferable to keep things simple and maintain legacy, safe compatibility.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area-FileOperationsRelated to file operations (e.g. moving, copying, renaming).Crash-IntermittentThe crash is an intermittent issue, hence low priorityIssue-BugRelated unexpected behavior or something worth investigating.Issue-Severe (high)Showstopper issues that require immediate attentionhelp wantedAnyone can help out!

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions