diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/Delete.cs b/src/libraries/System.IO.FileSystem/tests/Directory/Delete.cs index b264ec4c3b60e2..1e12662b971ca0 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/Delete.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/Delete.cs @@ -290,5 +290,34 @@ public void RecursiveDelete_ShouldThrowIOExceptionIfContainedFileInUse() } Assert.True(testDir.Exists); } + + [ConditionalFact(nameof(CanCreateSymbolicLinks))] + public void RecursiveDeletingDoesntFollowLinks() + { + var target = GetTestFilePath(); + Directory.CreateDirectory(target); + + var fileInTarget = Path.Combine(target, GetTestFileName()); + File.WriteAllText(fileInTarget, ""); + + var linkParent = GetTestFilePath(); + Directory.CreateDirectory(linkParent); + + var linkPath = Path.Combine(linkParent, GetTestFileName()); + Assert.NotNull(Directory.CreateSymbolicLink(linkPath, target)); + + // Both the symlink and the target exist + Assert.True(Directory.Exists(target), "target should exist"); + Assert.True(Directory.Exists(linkPath), "linkPath should exist"); + Assert.True(File.Exists(fileInTarget), "fileInTarget should exist"); + + // Delete the parent folder of the symlink. + Delete(linkParent, true); + + // Target should still exist + Assert.True(Directory.Exists(target), "target should still exist"); + Assert.False(Directory.Exists(linkPath), "linkPath should no longer exist"); + Assert.True(File.Exists(fileInTarget), "fileInTarget should exist"); + } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs index e23ae9e9e28a8c..05b782147a8e3a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Diagnostics; +using System.IO.Enumeration; using System.Text; using Microsoft.Win32.SafeHandles; @@ -433,69 +434,66 @@ public static void MoveDirectory(string sourceFullPath, string destFullPath) public static void RemoveDirectory(string fullPath, bool recursive) { - var di = new DirectoryInfo(fullPath); - if (!di.Exists) + // Delete the directory. + // If we're recursing, don't throw when it is not empty, and perform a recursive remove. + if (!RemoveEmptyDirectory(fullPath, topLevel: true, throwWhenNotEmpty: !recursive)) { - throw Interop.GetExceptionForIoErrno(Interop.Error.ENOENT.Info(), fullPath, isDirectory: true); + Debug.Assert(recursive); + + RemoveDirectoryRecursive(fullPath); } - RemoveDirectoryInternal(di, recursive, throwOnTopLevelDirectoryNotFound: true); } - private static void RemoveDirectoryInternal(DirectoryInfo directory, bool recursive, bool throwOnTopLevelDirectoryNotFound) + private static void RemoveDirectoryRecursive(string fullPath) { Exception? firstException = null; - if ((directory.Attributes & FileAttributes.ReparsePoint) != 0) + try { - DeleteFile(directory.FullName); - return; - } + var fse = new FileSystemEnumerable<(string, bool)>(fullPath, + static (ref FileSystemEntry entry) => + { + // Don't report symlinks to directories as directories. + bool isRealDirectory = !entry.IsSymbolicLink && entry.IsDirectory; + return (entry.ToFullPath(), isRealDirectory); + }, + EnumerationOptions.Compatible); - if (recursive) - { - try + foreach ((string childPath, bool isDirectory) in fse) { - foreach (string item in Directory.EnumerateFileSystemEntries(directory.FullName)) + try { - if (!ShouldIgnoreDirectory(Path.GetFileName(item))) + if (isDirectory) { - try - { - var childDirectory = new DirectoryInfo(item); - if (childDirectory.Exists) - { - RemoveDirectoryInternal(childDirectory, recursive, throwOnTopLevelDirectoryNotFound: false); - } - else - { - DeleteFile(item); - } - } - catch (Exception exc) - { - if (firstException != null) - { - firstException = exc; - } - } + RemoveDirectoryRecursive(childPath); + } + else + { + DeleteFile(childPath); } } - } - catch (Exception exc) - { - if (firstException != null) + catch (Exception ex) { - firstException = exc; + firstException ??= ex; } } + } + catch (Exception exc) + { + firstException ??= exc; + } - if (firstException != null) - { - throw firstException; - } + if (firstException != null) + { + throw firstException; } - if (Interop.Sys.RmDir(directory.FullName) < 0) + RemoveEmptyDirectory(fullPath); + } + + private static bool RemoveEmptyDirectory(string fullPath, bool topLevel = false, bool throwWhenNotEmpty = true) + { + if (Interop.Sys.RmDir(fullPath) < 0) { Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); switch (errorInfo.Error) @@ -504,17 +502,40 @@ private static void RemoveDirectoryInternal(DirectoryInfo directory, bool recurs case Interop.Error.EPERM: case Interop.Error.EROFS: case Interop.Error.EISDIR: - throw new IOException(SR.Format(SR.UnauthorizedAccess_IODenied_Path, directory.FullName)); // match Win32 exception + throw new IOException(SR.Format(SR.UnauthorizedAccess_IODenied_Path, fullPath)); // match Win32 exception case Interop.Error.ENOENT: - if (!throwOnTopLevelDirectoryNotFound) + // When we're recursing, don't throw for items that go missing. + if (!topLevel) { - return; + return true; + } + goto default; + case Interop.Error.ENOTDIR: + // When the top-level path is a symlink to a directory, delete the link. + // In other cases, throw because we expect path to be a real directory. + if (topLevel) + { + if (!DirectoryExists(fullPath)) + { + throw Interop.GetExceptionForIoErrno(Interop.Error.ENOENT.Info(), fullPath, isDirectory: true); + } + + DeleteFile(fullPath); + return true; + } + goto default; + case Interop.Error.ENOTEMPTY: + if (!throwWhenNotEmpty) + { + return false; } goto default; default: - throw Interop.GetExceptionForIoErrno(errorInfo, directory.FullName, isDirectory: true); + throw Interop.GetExceptionForIoErrno(errorInfo, fullPath, isDirectory: true); } } + + return true; } /// Determines whether the specified directory name should be ignored.