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

Possible foundation changes desired for usb drive fix. #204

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Xavron
Copy link

@Xavron Xavron commented Nov 16, 2023

Issue related #192

This pull request can remain open or accepted into the code if wanting the desired changes. If left open or accepted, I will slowly switch over more classes in time (years) or someone else can jump in. The code is set up with one class done as an example to go on for switching more over. The rest of the classes can be switched over time.

This pull request seeks to improve the foundation on which to deal with the USB drive fix. The code in the CmdDELE class is more readable, improved, and slimmed down like it once was when there was only File in use.

The tests I've run on it are successful.

I've only implemented it as a trial example to see if it is good to continue with. Other ways are indeed possible.

Example block side by side:

This pull request

        if (file == null) {
            errString = "550 Invalid name or chroot violation\r\n";
        } else {
            final String path = FileUtil.getScopedClientPath(param, null, null);
            if (file.violatesChroot(this, path)) {
                errString = "550 Invalid name or chroot violation\r\n";
            } else if (file.isDirectory()) {
                errString = "550 Can't DELE a directory\r\n";
            } else if (!file.delete(App.getAppContext())) {
                errString = "450 Error deleting file\r\n";
            }
        }

Current

        if (storeFile == null || storeFile.getOb() == null) {
            errString = "550 Invalid name or chroot violation\r\n";
        } else {
            final boolean isDocumentFile = storeFile.getOb() instanceof DocumentFile;
            final boolean isFile = !isDocumentFile;
            final String path = FileUtil.getScopedClientPath(param, null, null);
            if ((isDocumentFile && violatesChroot((DocumentFile) storeFile.getOb(), path))
                    || (isFile && violatesChroot((File) storeFile.getOb()))) {
                errString = "550 Invalid name or chroot violation\r\n";
            } else if (storeFile.isDirectory()) {
                errString = "550 Can't DELE a directory\r\n";
            } else if ((isDocumentFile && !((DocumentFile) storeFile.getOb()).delete())
                    || (isFile && !FileUtil.deleteFile((File) storeFile.getOb(), App.getAppContext()))) {
                errString = "450 Error deleting file\r\n";
            }
        }

Before DocumentFile

        if (violatesChroot(storeFile)) {
            errString = "550 Invalid name or chroot violation\r\n";
        } else if (storeFile.isDirectory()) {
            errString = "550 Can't DELE a directory\r\n";
        } else if (!FileUtil.deleteFile(storeFile, App.getAppContext())) {
            errString = "450 Error deleting file\r\n";
        }

@Xavron
Copy link
Author

Xavron commented Jul 4, 2024

I see a thumbs up =)

Will look into this more in time.

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.

1 participant