From 27059fb834038713fa42d326cce7431e31acc190 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 9 Aug 2021 18:56:58 +0200 Subject: [PATCH] Ensure Directory.Delete works fine even if user has no ListDirectory permissions (#56996) --- .../tests/Directory/Delete.Windows.cs | 30 +++++++++++++++++++ .../tests/Directory/Delete.cs | 2 +- .../tests/System.IO.FileSystem.Tests.csproj | 4 ++- .../src/System/IO/FileSystem.Windows.cs | 11 +++++-- 4 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 src/libraries/System.IO.FileSystem/tests/Directory/Delete.Windows.cs diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/Delete.Windows.cs b/src/libraries/System.IO.FileSystem/tests/Directory/Delete.Windows.cs new file mode 100644 index 0000000000000..007e13a0b9c43 --- /dev/null +++ b/src/libraries/System.IO.FileSystem/tests/Directory/Delete.Windows.cs @@ -0,0 +1,30 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Text; +using Xunit; +using Microsoft.DotNet.XUnitExtensions; +using System.Security.Principal; +using System.Security.AccessControl; + +namespace System.IO.Tests +{ + public partial class Directory_Delete_str_bool : Directory_Delete_str + { + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public void RecursiveDelete_NoListDirectoryPermission() // https://github.com/dotnet/runtime/issues/56922 + { + string parentPath = GetTestFilePath(); + var parent = Directory.CreateDirectory(parentPath); + var ac = parent.GetAccessControl(); + ac.SetAccessRule(new FileSystemAccessRule(WindowsIdentity.GetCurrent().User, FileSystemRights.ListDirectory, AccessControlType.Deny)); + parent.SetAccessControl(ac); + + var subDir = parent.CreateSubdirectory("subdir"); + File.Create(Path.Combine(subDir.FullName, GetTestFileName())).Dispose(); + Delete(subDir.FullName, recursive: true); + Assert.False(subDir.Exists); + } + } +} diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/Delete.cs b/src/libraries/System.IO.FileSystem/tests/Directory/Delete.cs index 00d5c46cef7ea..b264ec4c3b60e 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/Delete.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/Delete.cs @@ -217,7 +217,7 @@ public void Unix_NotFoundDirectory_ReadOnlyVolume() #endregion } - public class Directory_Delete_str_bool : Directory_Delete_str + public partial class Directory_Delete_str_bool : Directory_Delete_str { #region Utilities diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index 0decd976d8e53..227d528f25f49 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -1,4 +1,4 @@ - + true true @@ -75,6 +75,7 @@ + @@ -94,6 +95,7 @@ + diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index 191106a8de229..b92de0ad38ae9 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -186,7 +186,10 @@ public static void RemoveDirectory(string fullPath, bool recursive) } Interop.Kernel32.WIN32_FIND_DATA findData = default; - GetFindData(fullPath, isDirectory: true, ref findData); + // FindFirstFile($path) (used by GetFindData) fails with ACCESS_DENIED when user has no ListDirectory rights + // but FindFirstFile($path/*") (used by RemoveDirectoryRecursive) works fine in such scenario. + // So we ignore it here and let RemoveDirectoryRecursive throw if FindFirstFile($path/*") fails with ACCESS_DENIED. + GetFindData(fullPath, isDirectory: true, ignoreAccessDenied: true, ref findData); if (IsNameSurrogateReparsePoint(ref findData)) { // Don't recurse @@ -200,7 +203,7 @@ public static void RemoveDirectory(string fullPath, bool recursive) RemoveDirectoryRecursive(fullPath, ref findData, topLevel: true); } - private static void GetFindData(string fullPath, bool isDirectory, ref Interop.Kernel32.WIN32_FIND_DATA findData) + private static void GetFindData(string fullPath, bool isDirectory, bool ignoreAccessDenied, ref Interop.Kernel32.WIN32_FIND_DATA findData) { using SafeFindHandle handle = Interop.Kernel32.FindFirstFile(Path.TrimEndingDirectorySeparator(fullPath), ref findData); if (handle.IsInvalid) @@ -209,6 +212,8 @@ private static void GetFindData(string fullPath, bool isDirectory, ref Interop.K // File not found doesn't make much sense coming from a directory. if (isDirectory && errorCode == Interop.Errors.ERROR_FILE_NOT_FOUND) errorCode = Interop.Errors.ERROR_PATH_NOT_FOUND; + if (isDirectory && errorCode == Interop.Errors.ERROR_ACCESS_DENIED && ignoreAccessDenied) + return; throw Win32Marshal.GetExceptionForWin32Error(errorCode, fullPath); } } @@ -530,7 +535,7 @@ internal static void CreateSymbolicLink(string path, string pathToTarget, bool i private static unsafe string? GetFinalLinkTarget(string linkPath, bool isDirectory) { Interop.Kernel32.WIN32_FIND_DATA data = default; - GetFindData(linkPath, isDirectory, ref data); + GetFindData(linkPath, isDirectory, ignoreAccessDenied: false, ref data); // The file or directory is not a reparse point. if ((data.dwFileAttributes & (uint)FileAttributes.ReparsePoint) == 0 ||