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

Add folder cleanup support to CleanupInstaller #5090

Merged
merged 15 commits into from
May 2, 2022

Conversation

daguiler
Copy link
Contributor

Summary

The CleanupInstaller class supports a glob parameter that works well, but leaves an empty folder structure behind after deleting all files.
This pull request adds support for a new <folder> element in the package schema that allows for recursive folder deletion.

Usage:

<dotnetnuke type="Package" version="5.0">
    <packages>
        <package name="..." type="Provider" version="02.00.00">
            <friendlyName>...</friendlyName>
            ...
            <components>
                ...
                <component type="Cleanup">
                    <files>
                        ...
                    </files>
                    <folder>
                        <path>Providers/HtmlEditorProviders/MyProvider/1.0.0</path>
                    </folder>
                    <folder>
                        <path>Providers/HtmlEditorProviders/MyProvider/1.0.1</path>
                    </folder>
                    <folder>
                        <path>Providers/HtmlEditorProviders/MyProvider/1.0.2</path>
                    </folder>
                    ...
                </component>
            </components>
        </package>
    </packages>
</dotnetnuke>

The value of path is validated so that it can only reference valid paths inside the virtual directory.


var appPath = this.applicationStatusInfo.ApplicationMapPath;
var fullPath = Path.Combine(appPath, normalized);
if (!normalized.StartsWith(appPath, StringComparison.InvariantCultureIgnoreCase) || fullPath == appPath)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a feeling we need to be more protective with this validation test as it would be very easy to corrupt an installation with this.

Values such as

  • /DesktopModules
  • /Portals/0
  • /Bin

Or otherwise, and since this is a recursive delete, that doesn't expect folders to be empty I think we need to prevent at least our critical directories from being cleaned?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good point, although critical files could still get deleted using the glob parameter.
Anyway, I have changed the original indiscriminate delete now with a new method that only deletes empty directories. This avoids the risk of any files being deleted by CleanupInstaller.

@bdukes bdukes added this to the 9.11.0 milestone Apr 26, 2022
@bdukes bdukes changed the base branch from develop to release/9.11.0 April 26, 2022 19:24
Copy link
Contributor

@valadas valadas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work, thanks!

@valadas valadas merged commit 206ed97 into dnnsoftware:release/9.11.0 May 2, 2022
@daguiler daguiler deleted the folder-cleanup branch May 2, 2022 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants