From 257575fad582c5b24429906ba1d19b444e3c8f94 Mon Sep 17 00:00:00 2001 From: Osvay Soma Date: Tue, 13 Oct 2026 10:14:12 +0200 Subject: [PATCH 1/5] Delete a Physical folder through handles, so no link swapped in can steer it outside the root Refs #70 --- AGENTS.md | 5 +- CHANGELOG.md | 7 + .../FolderDelete.cs | 333 ++++++++++ .../PhysicalFileProviderOptions.cs | 7 +- src/WriteableFileProviders.Physical/README.md | 9 +- .../UnixFolders.cs | 73 +++ .../WindowsFolders.cs | 145 +++++ .../WriteableFileProviders.Physical.csproj | 3 +- .../WriteablePhysicalFileProvider.cs | 49 +- .../PhysicalFolderDeleteTests.cs | 599 ++++++++++++++++++ 10 files changed, 1217 insertions(+), 13 deletions(-) create mode 100644 src/WriteableFileProviders.Physical/FolderDelete.cs create mode 100644 src/WriteableFileProviders.Physical/UnixFolders.cs create mode 100644 src/WriteableFileProviders.Physical/WindowsFolders.cs create mode 100644 tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs diff --git a/AGENTS.md b/AGENTS.md index d652974..f49832a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -125,8 +125,9 @@ Each package has a test project of its own, `tests/WriteableFileProviders. new(EntryKind.Missing, null, error); +} + +// The calls a folder delete makes to the system, through handles: an entry is always opened relative to its folder's +// handle and without following a link, so a link swapped in can never take the delete elsewhere. They report raw +// facts; what the facts mean is decided by the rules below and by FolderDelete, which run on every system. +internal abstract class FolderSystem +{ + // The system this process runs on, where its calls and their flags are known: Linux on x64, Arm and Arm64, macOS, + // Windows. Null elsewhere: there a folder is deleted by its path, as .NET does. + public static FolderSystem? Current { get; } = + OperatingSystem.IsWindows() ? new WindowsFolders() + : UnixTable.For(OperatingSystem.IsLinux(), OperatingSystem.IsMacOS(), RuntimeInformation.ProcessArchitecture) is { } table ? new UnixFolders(table) + : null; + + // A folder by its path, following links, for the walk to start from: its handle, or null and the error number. + public abstract SafeHandle? OpenPath(string path, out int error); + + // The entry 'name' of 'folder', without following a link. 'forRemoval' asks for the access a removal needs (Windows + // asks for it as it opens); a folder on the way is only looked through. + public abstract Entry Open(SafeHandle folder, string name, bool forRemoval); + + // Removes an entry Open returned: 0, or the error number. 'classic' asks Windows for its delete without POSIX + // semantics; elsewhere it changes nothing. + public abstract int Remove(SafeHandle folder, string name, Entry entry, bool classic); + + // What an error number means. + public abstract Failure Classify(int error); + + // Whether a delete refused with this error number is tried again the classic way. + public virtual bool FallsBack(int error) => false; + + // The path to list a folder by. + public virtual string ListingPath(string path) => path; + + // The system's words for an error number. + public virtual string Describe(int error) => Marshal.GetPInvokeErrorMessage(error); +} + +// The flags and error numbers of the C library calls, which differ by system and, on Linux, by architecture. From the +// kernel's and Darwin's headers. +internal sealed record UnixTable(int Directory, int NoFollow, int PathOnly, int CloseOnExec, int RemoveDirectory, int Loop, int NotEmpty) +{ + // The same on Linux and macOS. + public const int NoEntry = 2; + public const int AccessDenied = 13; + public const int NotPermitted = 1; + public const int NotADirectory = 20; + public const int Invalid = 22; + + // Linux on x64 (and the architectures that use the generic values); O_PATH opens a folder without reading it. + public static UnixTable Linux { get; } = new(0x10000, 0x20000, 0x200000, 0x80000, 0x200, 40, 39); + + // Linux on Arm and Arm64, whose O_DIRECTORY and O_NOFOLLOW differ. + public static UnixTable LinuxArm { get; } = new(0x4000, 0x8000, 0x200000, 0x80000, 0x200, 40, 39); + + // macOS, on Intel and Apple silicon; it has no O_PATH, so a folder is opened to read (O_RDONLY, 0). + public static UnixTable MacOS { get; } = new(0x100000, 0x100, 0, 0x1000000, 0x80, 62, 66); + + // The table of a system, by its architecture as this process runs (an x64 process emulated on Arm is x64); null + // where the values are not known. + public static UnixTable? For(bool linux, bool macOS, Architecture architecture) => + (linux, macOS, architecture) switch + { + (true, _, Architecture.X64) => Linux, + (true, _, Architecture.Arm or Architecture.Arm64) => LinuxArm, + (_, true, Architecture.X64 or Architecture.Arm64) => MacOS, + _ => null, + }; + + // Opens a folder: without reading it where the system can. + public int OpenFolder => PathOnly | Directory | CloseOnExec; + + // Opens an entry only if it is a folder of its own: a link fails (ENOTDIR or ELOOP), a file fails (ENOTDIR). + public int OpenEntry => OpenFolder | NoFollow; + + // Not a folder on the way (ENOTDIR) is as gone as nothing there. + public Failure Classify(int error) => + error is NoEntry or NotADirectory ? Failure.Gone + : error is AccessDenied or NotPermitted ? Failure.Refused + : error == NotEmpty ? Failure.NotEmpty + : Failure.Other; +} + +// What Windows reports, in numbers that do not depend on the architecture. +internal static class WindowsRules +{ + public const uint DirectoryAttribute = 0x10; + public const uint ReparsePointAttribute = 0x400; + + // A reparse tag that names another entry (a symbolic link, a junction, a mount point) has this bit. + public const uint NameSurrogate = 0x20000000; + + public const int FileNotFound = 2; + public const int PathNotFound = 3; + public const int AccessDenied = 5; + public const int InvalidFunction = 1; + public const int NotSupported = 50; + public const int InvalidParameter = 87; + public const int DirectoryNotEmpty = 145; + + // A name surrogate is a link, whatever else it is, as .NET's recursive delete reads it; other reparse points (cloud + // placeholders, deduplicated files) hold their own content. + public static EntryKind KindOf(uint attributes, uint tag) => + (attributes & ReparsePointAttribute) != 0 && (tag & NameSurrogate) != 0 ? EntryKind.Link + : (attributes & DirectoryAttribute) != 0 ? EntryKind.Folder + : EntryKind.Other; + + public static Failure Classify(int error) => + error switch + { + FileNotFound or PathNotFound => Failure.Gone, + AccessDenied => Failure.Refused, + DirectoryNotEmpty => Failure.NotEmpty, + _ => Failure.Other, + }; + + // A file system without POSIX deletes (FAT, exFAT, some network file systems) refuses them so; DeleteFile then + // deletes the classic way, and so does this. + public static bool FallsBack(int error) => error is InvalidParameter or NotSupported or InvalidFunction; + + // The extended form of a full path, which Windows takes as it is, past MAX_PATH and with any name: \\?\C:\a, and + // \\?\UNC\server\share\a for \\server\share\a. + public static string ExtendedPath(string path) => + path.StartsWith(@"\\", StringComparison.Ordinal) ? @"\\?\UNC\" + path[2..] : @"\\?\" + path; +} + +// Deletes a folder and everything in it through the handles of FolderSystem. The names come from listing each folder +// by its path; every entry is opened relative to its folder's handle without following a link and removed through +// that handle or its own, so a listing that a rename or a swapped link made read another folder can only leave names +// out, which then fails the delete (the folder is not empty), never reach outside the folder. Like .NET's recursive +// delete, it goes on past an entry it cannot delete, skips one gone meanwhile, and throws the first failure at the end. +internal sealed class FolderDelete(FolderSystem system, Action? opening, bool classicDeletes) +{ + private static readonly EnumerationOptions _listing = new() { AttributesToSkip = 0, IgnoreInaccessible = false }; + + // Deletes the folder at root/segments, whose provider path is 'path': true when something was deleted, false when + // nothing was there by the time it was opened. With 'refuseLinks', the walk starts at the root and opens each + // folder on the way without following a link, and a link there or at the target throws 'refused' for its path + // under the root; without, the folder that holds the target is opened by its path, following links as every + // member does. + public bool Delete(ResolvedPath path, string root, IReadOnlyList segments, bool refuseLinks, Func refused) + { + var fullPath = Path.Join([root, .. segments]); + var parent = refuseLinks ? WalkDown(path, root, segments, refused) : OpenPath(path, Path.GetDirectoryName(fullPath)!); + if (parent is null) + return false; + using (parent) + { + var name = segments[^1]; + opening?.Invoke(fullPath); + var target = system.Open(parent, name, forRemoval: true); + using (target.Handle) + { + if (target.Error != 0) + return system.Classify(target.Error) == Failure.Gone ? false : throw Failed(path.ToString(), target.Error); + switch (target.Kind) + { + case EntryKind.Missing: + return false; + case EntryKind.Link when refuseLinks: + throw refused(string.Join('/', segments)); + case EntryKind.Folder: + DeleteContents(target.Handle!, fullPath, path.ToString()); + break; + } + // A link is removed as itself; a file put where the folder was is what the path names now. + var error = Remove(parent, name, target); + if (error != 0 && system.Classify(error) != Failure.Gone) + throw Failed(path.ToString(), error); + return true; + } + } + } + + // The folder that holds the target, reached from the root one folder at a time without following a link: null when + // a folder on the way is gone or is not a folder. + private SafeHandle? WalkDown(ResolvedPath path, string root, IReadOnlyList segments, Func refused) + { + // The root may itself be a link: it is opened by its path. + var folder = OpenPath(path, root); + for (var i = 0; folder is not null && i < segments.Count - 1; i++) + { + opening?.Invoke(Path.Join([root, .. segments.Take(i + 1)])); + Entry entry; + using (folder) + entry = system.Open(folder, segments[i], forRemoval: false); + if (entry.Error != 0) + return system.Classify(entry.Error) == Failure.Gone ? null : throw Failed(path.ToString(), entry.Error); + if (entry.Kind == EntryKind.Folder) + { + folder = entry.Handle; + continue; + } + entry.Handle?.Dispose(); + return entry.Kind == EntryKind.Link ? throw refused(string.Join('/', segments.Take(i + 1))) : null; + } + return folder; + } + + // A folder by its path: null when it is gone or is not a folder. + private SafeHandle? OpenPath(ResolvedPath path, string folderPath) + { + var handle = system.OpenPath(folderPath, out var error); + if (handle is not null) + return handle; + return system.Classify(error) == Failure.Gone ? null : throw Failed(path.ToString(), error); + } + + // Empties the folder whose handle is 'folder' and whose path is 'fullPath' ('relative' under the root, for messages). + private void DeleteContents(SafeHandle folder, string fullPath, string relative) + { + Exception? first = null; + IEnumerator? names = null; + try + { + while (Next(fullPath, ref names, ref first)) + { + var name = names.Current; + var childPath = Path.Join(fullPath, name); + var childRelative = relative + "/" + name; + opening?.Invoke(childPath); + var entry = system.Open(folder, name, forRemoval: true); + using (entry.Handle) + { + if (entry.Error != 0) + { + if (system.Classify(entry.Error) != Failure.Gone) + first ??= Failed(childRelative, entry.Error); + continue; + } + if (entry.Kind == EntryKind.Missing) + continue; + if (entry.Kind == EntryKind.Folder) + { + try + { + DeleteContents(entry.Handle!, childPath, childRelative); + } + catch (Exception e) when (e is IOException or UnauthorizedAccessException) + { + // Not empty: it stays, and the delete goes on with the rest. + first ??= e; + continue; + } + } + var error = Remove(folder, name, entry); + if (error != 0 && system.Classify(error) != Failure.Gone) + first ??= Failed(childRelative, error); + } + } + } + finally + { + names?.Dispose(); + } + if (first is not null) + throw first; + } + + // The next name of the folder, listed by its path as the walk goes: false at the end, or when the listing fails, + // which is recorded. + private bool Next(string fullPath, [NotNullWhen(true)] ref IEnumerator? names, ref Exception? first) + { + try + { + names ??= new FileSystemEnumerable(system.ListingPath(fullPath), (ref FileSystemEntry entry) => entry.FileName.ToString(), _listing).GetEnumerator(); + return names.MoveNext(); + } + catch (Exception e) when (e is IOException or UnauthorizedAccessException) + { + // The folder could not be listed, or no longer can: what is left of it fails its own removal. + first ??= e; + return false; + } + } + + private int Remove(SafeHandle folder, string name, Entry entry) + { + var error = system.Remove(folder, name, entry, classicDeletes); + if (error != 0 && !classicDeletes && system.FallsBack(error)) + error = system.Remove(folder, name, entry, classic: true); + return error; + } + + private Exception Failed(string relative, int error) + { + var message = $"'{relative}' could not be deleted: {system.Describe(error)}"; + return system.Classify(error) == Failure.Refused ? new UnauthorizedAccessException(message) : new IOException(message); + } +} diff --git a/src/WriteableFileProviders.Physical/PhysicalFileProviderOptions.cs b/src/WriteableFileProviders.Physical/PhysicalFileProviderOptions.cs index 2a7aceb..97ab5bd 100644 --- a/src/WriteableFileProviders.Physical/PhysicalFileProviderOptions.cs +++ b/src/WriteableFileProviders.Physical/PhysicalFileProviderOptions.cs @@ -46,9 +46,10 @@ public override string? RootDirectoryPath /// /// /// The check looks at the path, one entry at a time from the root, just before each member acts, not atomically - /// with it: a process that can create links inside the root can still swap one in between. A recursive delete of a - /// folder checks the folder's own path, then goes into each subfolder by its path, so a link swapped in while it - /// runs can steer it into a folder elsewhere and delete what is there. On Linux and macOS, an entry on the way that + /// with it: a process that can create links inside the root can still swap one in between. A folder delete is the + /// exception where the system is known (Linux on x64, Arm and Arm64, macOS, Windows): it opens each folder on the way + /// from the root without following a link, so a link swapped in there is refused, and removes a link inside the + /// folder as itself, never going into it. On Linux and macOS, an entry on the way that /// the process may not look up ends the check, as the system then stops the member there too (a read finds nothing, /// a write is refused); on Windows, which can let a process through a folder it may not read, it throws /// . Watch is unchanged: with the polling watcher, a change behind diff --git a/src/WriteableFileProviders.Physical/README.md b/src/WriteableFileProviders.Physical/README.md index af8f894..cbdf934 100644 --- a/src/WriteableFileProviders.Physical/README.md +++ b/src/WriteableFileProviders.Physical/README.md @@ -71,10 +71,15 @@ None. The provider reads and writes with the permissions of the process. out such names, and on Linux and macOS, names holding `:` or `\`. - **Symbolic links and junctions** inside the root are followed, wherever they point, unless you set `RefuseLinks`. Without it, a recursive listing lists a linked folder but doesn't walk into it. `RefuseLinks` checks the path just - before each operation, so a process that can create links inside the root can still swap one in between; a recursive - folder delete can even be steered into a folder elsewhere by a link swapped in while it runs. Hard links and mounts on + before each operation, so a process that can create links inside the root can still swap one in between, except for + a folder delete, which opens every folder from the root without following a link. Hard links and mounts on Linux and macOS aren't visible as links, and other reparse points (cloud placeholders, Windows container layers) aren't links. The polling `Watch` can still report a change behind a linked folder. +- **A folder delete never leaves the folder.** It opens each entry in its folder without following a link and removes + it there, so a link inside, even one swapped in while it runs, is removed as itself. It goes on past what it can't + delete and throws the first failure. On Windows a folder holding a junction is deleted. On macOS the folder above the + deleted one must be readable. Where the system's calls aren't known (Linux other than on x64 and Arm), the delete + goes by path, as .NET's does. - **No content types.** `UpdateFileOptions.ContentType` is ignored; `ContentType` is null. - **`Watch` works** like ASP.NET Core's `PhysicalFileProvider`: a glob relative to the root. Hidden and dot-prefixed files aren't watched. Where file system events don't arrive (some network and container mounts), set diff --git a/src/WriteableFileProviders.Physical/UnixFolders.cs b/src/WriteableFileProviders.Physical/UnixFolders.cs new file mode 100644 index 0000000..599ae4b --- /dev/null +++ b/src/WriteableFileProviders.Physical/UnixFolders.cs @@ -0,0 +1,73 @@ +using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; + +namespace WriteableFileProviders.Physical; + +// A folder delete's calls on Linux and macOS, through the C library: openat with O_NOFOLLOW and O_DIRECTORY opens an +// entry only if it is a folder of its own, readlinkat tells a link from a file, and unlinkat removes an entry from the +// folder whose handle it is given. open and openat are declared with their fixed arguments only: O_CREAT is never set, +// so the mode they would take is never read. +internal sealed partial class UnixFolders(UnixTable table) : FolderSystem +{ + public override SafeHandle? OpenPath(string path, out int error) + { + var fd = Open(path, table.OpenFolder); + error = fd < 0 ? Marshal.GetLastPInvokeError() : 0; + return fd < 0 ? null : new SafeFileHandle(fd, ownsHandle: true); + } + + public override Entry Open(SafeHandle folder, string name, bool forRemoval) + { + var fd = With(folder, dirfd => OpenAt(dirfd, name, table.OpenEntry)); + if (fd >= 0) + return new Entry(EntryKind.Folder, new SafeFileHandle(fd, ownsHandle: true), 0); + var error = Marshal.GetLastPInvokeError(); + if (error == UnixTable.NoEntry) + return new Entry(EntryKind.Missing, null, 0); + if (error != UnixTable.NotADirectory && error != table.Loop) + return Entry.Failed(error); + // Not a folder of its own (a link gives ENOTDIR on Linux, ELOOP or ENOTDIR on macOS): a link has a target to read. + byte target = 0; + if (With(folder, dirfd => ReadLinkAt(dirfd, name, ref target, 1)) >= 0) + return new Entry(EntryKind.Link, null, 0); + error = Marshal.GetLastPInvokeError(); + return error == UnixTable.Invalid ? new Entry(EntryKind.Other, null, 0) + : error == UnixTable.NoEntry ? new Entry(EntryKind.Missing, null, 0) + : Entry.Failed(error); + } + + public override int Remove(SafeHandle folder, string name, Entry entry, bool classic) => + With(folder, dirfd => UnlinkAt(dirfd, name, entry.Kind == EntryKind.Folder ? table.RemoveDirectory : 0)) == 0 + ? 0 + : Marshal.GetLastPInvokeError(); + + public override Failure Classify(int error) => table.Classify(error); + + // Runs a call with the file descriptor a handle holds, which stays open until the call returns. + private static T With(SafeHandle handle, Func call) + { + var added = false; + handle.DangerousAddRef(ref added); + try + { + return call((int)handle.DangerousGetHandle()); + } + finally + { + if (added) + handle.DangerousRelease(); + } + } + + [LibraryImport("libc", EntryPoint = "open", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] + private static partial int Open(string path, int flags); + + [LibraryImport("libc", EntryPoint = "openat", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] + private static partial int OpenAt(int folder, string name, int flags); + + [LibraryImport("libc", EntryPoint = "readlinkat", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] + private static partial nint ReadLinkAt(int folder, string name, ref byte buffer, nuint size); + + [LibraryImport("libc", EntryPoint = "unlinkat", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] + private static partial int UnlinkAt(int folder, string name, int flags); +} diff --git a/src/WriteableFileProviders.Physical/WindowsFolders.cs b/src/WriteableFileProviders.Physical/WindowsFolders.cs new file mode 100644 index 0000000..1a9a2f8 --- /dev/null +++ b/src/WriteableFileProviders.Physical/WindowsFolders.cs @@ -0,0 +1,145 @@ +using System.Diagnostics.CodeAnalysis; +using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; + +namespace WriteableFileProviders.Physical; + +// A folder delete's calls on Windows: NtCreateFile opens an entry relative to its folder's handle, with +// FILE_OPEN_REPARSE_POINT so a link is opened as itself; its attributes and reparse tag say what it is (WindowsRules); +// it is deleted through its own handle, with POSIX semantics unless asked otherwise (FolderDelete decides when). Only +// the calls are here, each with no branch but its own success: everything they report is read by shared code. +[ExcludeFromCodeCoverage(Justification = "Runs only on Windows, where CI runs the tests without coverage; the decisions on what it reports are shared code, covered on Linux.")] +internal sealed unsafe partial class WindowsFolders : FolderSystem +{ + private const uint Delete = 0x10000; + private const uint ReadAttributes = 0x80; + private const uint Synchronize = 0x100000; + private const uint ShareAll = 0x7; + private const uint OpenExisting = 3; + private const uint BackupSemantics = 0x02000000; + private const uint OpenDisposition = 1; + private const uint SynchronousIo = 0x20; + private const uint ForBackupIntent = 0x4000; + private const uint OpenReparsePoint = 0x200000; + private const uint CaseInsensitive = 0x40; + private const int AttributeTagInfo = 9; + private const int DispositionInfo = 4; + private const int DispositionInfoEx = 21; + private const uint DispositionDelete = 0x1; + private const uint DispositionPosixSemantics = 0x2; + + public override SafeHandle? OpenPath(string path, out int error) + { + var handle = CreateFile(WindowsRules.ExtendedPath(path), ReadAttributes | Synchronize, ShareAll, 0, OpenExisting, BackupSemantics, 0); + error = handle.IsInvalid ? Marshal.GetLastPInvokeError() : 0; + if (!handle.IsInvalid) + return handle; + handle.Dispose(); + return null; + } + + public override Entry Open(SafeHandle folder, string name, bool forRemoval) + { + var handle = OpenRelative(folder, name, (forRemoval ? Delete : 0) | ReadAttributes | Synchronize, out var error); + if (handle is null) + return Entry.Failed(error); + uint* info = stackalloc uint[2]; + if (GetFileInformationByHandleEx(handle, AttributeTagInfo, info, 8) == 0) + { + error = Marshal.GetLastPInvokeError(); + handle.Dispose(); + return Entry.Failed(error); + } + return new Entry(WindowsRules.KindOf(info[0], info[1]), handle, 0); + } + + public override int Remove(SafeHandle folder, string name, Entry entry, bool classic) + { + var flags = DispositionDelete | DispositionPosixSemantics; + byte delete = 1; + var done = classic + ? SetFileInformationByHandle(entry.Handle!, DispositionInfo, &delete, 1) + : SetFileInformationByHandle(entry.Handle!, DispositionInfoEx, &flags, 4); + return done != 0 ? 0 : Marshal.GetLastPInvokeError(); + } + + public override Failure Classify(int error) => WindowsRules.Classify(error); + + public override bool FallsBack(int error) => WindowsRules.FallsBack(error); + + public override string ListingPath(string path) => WindowsRules.ExtendedPath(path); + + // Opens 'name' in the folder whose handle is given, as itself: a link, a junction or a mount point is not followed. + private static SafeFileHandle? OpenRelative(SafeHandle folder, string name, uint access, out int error) + { + var added = false; + folder.DangerousAddRef(ref added); + try + { + fixed (char* chars = name) + { + var length = (ushort)(name.Length * sizeof(char)); + var objectName = new UnicodeString { Length = length, MaximumLength = length, Buffer = chars }; + var attributes = new ObjectAttributes + { + Length = sizeof(ObjectAttributes), + RootDirectory = folder.DangerousGetHandle(), + ObjectName = &objectName, + Attributes = CaseInsensitive, + }; + IoStatusBlock status; + var result = NtCreateFile(out var handle, access, &attributes, &status, null, 0, ShareAll, OpenDisposition, + SynchronousIo | ForBackupIntent | OpenReparsePoint, null, 0); + error = result >= 0 ? 0 : (int)RtlNtStatusToDosError(result); + return result >= 0 ? new SafeFileHandle(handle, ownsHandle: true) : null; + } + } + finally + { + if (added) + folder.DangerousRelease(); + } + } + + [StructLayout(LayoutKind.Sequential)] + private struct UnicodeString + { + public ushort Length; + public ushort MaximumLength; + public char* Buffer; + } + + [StructLayout(LayoutKind.Sequential)] + private struct ObjectAttributes + { + public int Length; + public nint RootDirectory; + public UnicodeString* ObjectName; + public uint Attributes; + public void* SecurityDescriptor; + public void* SecurityQualityOfService; + } + + [StructLayout(LayoutKind.Sequential)] + private struct IoStatusBlock + { + public nint Status; + public nint Information; + } + + [LibraryImport("kernel32.dll", EntryPoint = "CreateFileW", StringMarshalling = StringMarshalling.Utf16, SetLastError = true)] + private static partial SafeFileHandle CreateFile(string name, uint access, uint share, nint security, uint disposition, uint flags, nint template); + + [LibraryImport("ntdll.dll")] + private static partial int NtCreateFile(out nint handle, uint access, ObjectAttributes* attributes, IoStatusBlock* status, long* allocationSize, + uint fileAttributes, uint share, uint disposition, uint options, void* extendedAttributes, uint extendedAttributesLength); + + [LibraryImport("ntdll.dll")] + private static partial uint RtlNtStatusToDosError(int status); + + [LibraryImport("kernel32.dll", SetLastError = true)] + private static partial int GetFileInformationByHandleEx(SafeHandle handle, int informationClass, void* information, uint size); + + [LibraryImport("kernel32.dll", SetLastError = true)] + private static partial int SetFileInformationByHandle(SafeHandle handle, int informationClass, void* information, uint size); +} diff --git a/src/WriteableFileProviders.Physical/WriteableFileProviders.Physical.csproj b/src/WriteableFileProviders.Physical/WriteableFileProviders.Physical.csproj index 8135325..0fdcc6e 100644 --- a/src/WriteableFileProviders.Physical/WriteableFileProviders.Physical.csproj +++ b/src/WriteableFileProviders.Physical/WriteableFileProviders.Physical.csproj @@ -5,7 +5,8 @@ Read, write, list and watch files in a folder on the local disk through IWriteableFileProvider: atomic writes and appends, conditional writes, byte-range and seekable reads, change notifications. The same API as the Azure Blob, Amazon S3, MinIO, Google Cloud Storage, Box and SharePoint providers. Native AOT compatible. $(PackageTags);local;disk;local-storage;physical;aot;native-aot;trimming README.md - + true diff --git a/src/WriteableFileProviders.Physical/WriteablePhysicalFileProvider.cs b/src/WriteableFileProviders.Physical/WriteablePhysicalFileProvider.cs index 58b2cf4..b9807e5 100644 --- a/src/WriteableFileProviders.Physical/WriteablePhysicalFileProvider.cs +++ b/src/WriteableFileProviders.Physical/WriteablePhysicalFileProvider.cs @@ -95,6 +95,21 @@ namespace WriteableFileProviders.Physical; /// that died during a write left behind. /// /// +/// Deleting a folder goes through handles: each entry is opened in its folder without following a link and +/// removed there, so a link inside the folder, even one swapped in while the delete runs, is removed as itself and never +/// gone into, and the delete never reaches outside the folder it opened. The folder that holds it is opened by its path, +/// links followed, unless is set: then every folder from the root +/// down is opened the same way, and a link swapped in on the way is refused. The names come from listing each folder by +/// its path, so a rename or a link swapped in meanwhile can only make the delete miss names, and fail. Like .NET's +/// recursive delete, it goes on past an entry it cannot delete, skips one gone meanwhile and throws the first failure +/// at the end; a folder that still has entries after it (a file another process holds open, on a file system without +/// POSIX deletes) throws . On Windows a folder holding a junction is deleted, the junction as +/// itself. On macOS the folder that holds the deleted one, and with RefuseLinks every folder on the way, must be +/// readable. This is how a folder is deleted on Linux on x64, Arm and Arm64, macOS and Windows; elsewhere it is deleted +/// by its path, as .NET does, and a link swapped in while that runs can steer it elsewhere. A file is deleted by its +/// path. +/// +/// /// Access. Where the operating system refuses the process access to a file or a folder that a member opens, /// lists, writes, moves or deletes, the member throws , a read or a listing as /// well as a write. A path the system will not look up at all, such as one inside a folder the process may not enter, @@ -226,6 +241,18 @@ public WriteablePhysicalFileProvider(PhysicalFileProviderOptions options) : base // the tests change the file there. internal Action? Probed { get; init; } + // How a folder is deleted: through handles, where the system and its flags are known (Linux on x64, Arm and Arm64, + // macOS, Windows), else by its path, as .NET does (null). Settable so the tests run the path delete and a fake system. + internal FolderSystem? Folders { get; init; } = FolderSystem.Current; + + // Called with the full path of each entry a folder delete is about to open, the folders on the way included: the + // tests swap an entry for a link there. + internal Action? Opening { get; init; } + + // Whether a folder delete deletes each entry the classic way from the start, as it does where the file system + // refuses POSIX deletes (Windows): the tests run that way on NTFS, which never refuses them. + internal bool ClassicDeletes { get; init; } + /// True on Windows and macOS, whose file systems match names without regard to case by default. protected override bool IsCaseInsensitive => IgnoresCase; @@ -433,12 +460,20 @@ protected override Task DeleteCoreAsync(ResolvedPath path, CancellationTok cancellationToken.ThrowIfCancellationRequested(); var fullPath = FullPath(path); if (File.Exists(fullPath)) + { File.Delete(fullPath); - else if (Directory.Exists(fullPath)) - Directory.Delete(fullPath, recursive: true); - else + return Task.FromResult(true); + } + if (!Directory.Exists(fullPath)) return Task.FromResult(false); - return Task.FromResult(true); + if (Folders is not { } folders) + { + Directory.Delete(fullPath, recursive: true); + return Task.FromResult(true); + } + // Through handles, so a link swapped in while the delete runs can never take it elsewhere. + var walk = new FolderDelete(folders, Opening, ClassicDeletes); + return Task.FromResult(walk.Delete(path, _root, path.RelativeSegments, _refuseLinks, link => RefusedLink(path, link))); } /// Renames the file; the destination is taken atomically, so an existing one is never replaced. @@ -730,7 +765,7 @@ private string FullPath(ResolvedPath path) private void RefuseLinksOn(ResolvedPath path) { if (LinkOnTheWay(path.RelativeSegments) is { } link) - throw new ArgumentException($"The path '{path}' goes through '{link}', a link, which this provider refuses (RefuseLinks).", nameof(path)); + throw RefusedLink(path, link); } // The first entry on the way to these segments under the root that is a link, as its path under the root, or null. @@ -1166,6 +1201,10 @@ public async Task CopyFromStartAsync(Stream output, CancellationToken cancellati public ValueTask DisposeAsync() => _file?.DisposeAsync() ?? default; } + // A path that names or goes through 'link', its path under the root, with RefuseLinks. + private static ArgumentException RefusedLink(ResolvedPath path, string link) => + new($"The path '{path}' goes through '{link}', a link, which this provider refuses (RefuseLinks).", nameof(path)); + private static FileAlreadyExistsException AlreadyExists(ResolvedPath path, Exception? innerException = null) => new($"The file '{path}' already exists.", innerException); diff --git a/tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs b/tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs new file mode 100644 index 0000000..4dedf84 --- /dev/null +++ b/tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs @@ -0,0 +1,599 @@ +using System.Diagnostics; +using System.Runtime.InteropServices; +using System.Runtime.Versioning; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace WriteableFileProviders.Physical.Tests; + +/// +/// Physical's folder delete through handles: a link swapped in while it runs is removed as itself, never entered, and +/// with a link swapped in on the way is refused; the errors, the +/// rules each system's answers are read by, and the delete by path where the system is not known. +/// +[TestClass] +public sealed class PhysicalFolderDeleteTests +{ + private string _base = ""; + private string _root = ""; + private string _outside = ""; + + [TestInitialize] + public void Start() + { + _base = Path.Combine(Path.GetTempPath(), "wfp-delete-" + Guid.NewGuid().ToString("N")[..12]); + _root = Directory.CreateDirectory(Path.Combine(_base, "root")).FullName; + _outside = Directory.CreateDirectory(Path.Combine(_base, "outside")).FullName; + File.WriteAllText(Path.Combine(_outside, "keep.txt"), "outside"); + Directory.CreateDirectory(Path.Combine(_outside, "deep")); + File.WriteAllText(Path.Combine(_outside, "deep", "keep.txt"), "outside, deeper"); + } + + [TestCleanup] + public void End() + { + if (!Directory.Exists(_base)) + return; + // .NET's own recursive delete fails at a junction: the links go first, as themselves. + var links = new DirectoryInfo(_base).EnumerateFileSystemInfos("*", new EnumerationOptions { RecurseSubdirectories = true, AttributesToSkip = 0 }) + .Where(entry => entry.LinkTarget is not null).ToList(); + foreach (var link in links) + link.Delete(); + Directory.Delete(_base, recursive: true); + } + + private WriteablePhysicalFileProvider Provider(bool refuseLinks = false, Action? opening = null) => + new(new PhysicalFileProviderOptions { RootDirectoryPath = _root, RefuseLinks = refuseLinks }) { Opening = opening }; + + private string Disk(params string[] segments) => Path.Combine([_root, .. segments]); + + private void Write(string text, params string[] segments) + { + Directory.CreateDirectory(Path.GetDirectoryName(Disk(segments))!); + File.WriteAllText(Disk(segments), text); + } + + private void AssertOutsideIsIntact() + { + Assert.AreEqual("outside", File.ReadAllText(Path.Combine(_outside, "keep.txt"))); + Assert.AreEqual("outside, deeper", File.ReadAllText(Path.Combine(_outside, "deep", "keep.txt"))); + } + + // A link to a folder: a symbolic link, or on Windows a junction, which needs no privilege there. + private static void FolderLink(string link, string target) + { + if (!OperatingSystem.IsWindows()) + { + Directory.CreateSymbolicLink(link, target); + return; + } + using var mklink = Process.Start(new ProcessStartInfo("cmd.exe", ["/c", "mklink", "/J", link, target]) { UseShellExecute = false, RedirectStandardOutput = true })!; + mklink.WaitForExit(); + Assert.AreEqual(0, mklink.ExitCode, "mklink /J"); + } + + // A symbolic link to a file: false on Windows without Developer Mode or elevation, which it needs there. + private static bool TryFileLink(string link, string target) + { + try + { + File.CreateSymbolicLink(link, target); + return true; + } + catch (Exception e) when (OperatingSystem.IsWindows() && e is IOException or UnauthorizedAccessException) + { + return false; + } + } + + // ----- Links swapped in while the delete runs ----- + + [TestMethod] + [DataRow(false)] + [DataRow(true)] + public async Task ASubfolderSwappedForALinkMeanwhile_IsRemovedAsALink_NeverEntered(bool refuseLinks) + { + Write("inner", "dir", "sub", "inner.txt"); + Write("other", "dir", "other.txt"); + var swapped = false; + using var provider = Provider(refuseLinks, opening: path => + { + if (path != Disk("dir", "sub") || swapped) + return; + swapped = true; + Directory.Move(Disk("dir", "sub"), Disk("moved")); + FolderLink(Disk("dir", "sub"), _outside); + }); + Assert.IsTrue(await provider.DeleteAsync("dir")); + Assert.IsTrue(swapped); + Assert.IsFalse(Path.Exists(Disk("dir"))); + AssertOutsideIsIntact(); + Assert.AreEqual("inner", File.ReadAllText(Disk("moved", "inner.txt")), "The folder moved away is not the delete's"); + } + + [TestMethod] + public async Task WithRefuseLinks_AFolderOnTheWaySwappedForALink_IsRefused() + { + Write("x", "a", "b", "x.txt"); + Directory.CreateDirectory(Path.Combine(_outside, "b")); + File.WriteAllText(Path.Combine(_outside, "b", "y.txt"), "outside"); + var swapped = false; + using var provider = Provider(refuseLinks: true, opening: path => + { + if (path != Disk("a") || swapped) + return; + swapped = true; + Directory.Move(Disk("a"), Disk("a-moved")); + FolderLink(Disk("a"), _outside); + }); + var e = await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("a/b")); + StringAssert.Contains(e.Message, "RefuseLinks"); + StringAssert.Contains(e.Message, "'a'"); + Assert.AreEqual("outside", File.ReadAllText(Path.Combine(_outside, "b", "y.txt"))); + Assert.AreEqual("x", File.ReadAllText(Disk("a-moved", "b", "x.txt"))); + } + + [TestMethod] + public async Task WithRefuseLinks_TheTargetSwappedForALink_IsRefused() + { + Write("x", "dir", "x.txt"); + using var provider = Provider(refuseLinks: true, opening: path => + { + if (path != Disk("dir") || Directory.Exists(Disk("dir-moved"))) + return; + Directory.Move(Disk("dir"), Disk("dir-moved")); + FolderLink(Disk("dir"), _outside); + }); + var e = await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); + StringAssert.Contains(e.Message, "RefuseLinks"); + Assert.IsTrue(Path.Exists(Disk("dir")), "The link stays"); + AssertOutsideIsIntact(); + } + + [TestMethod] + public async Task WithRefuseLinks_AFolderOnTheWaySwappedForAFile_DeletesNothing() + { + Write("x", "a", "b", "x.txt"); + using var provider = Provider(refuseLinks: true, opening: path => + { + if (path != Disk("a") || Directory.Exists(Disk("a-moved"))) + return; + Directory.Move(Disk("a"), Disk("a-moved")); + File.WriteAllText(Disk("a"), "a file"); + }); + Assert.IsFalse(await provider.DeleteAsync("a/b")); + Assert.AreEqual("x", File.ReadAllText(Disk("a-moved", "b", "x.txt"))); + } + + [TestMethod] + public async Task ATargetFolderReplacedByAFileSinceTheProbe_IsDeleted() + { + Write("x", "dir", "x.txt"); + using var provider = Provider(opening: path => + { + if (path != Disk("dir") || File.Exists(Disk("dir"))) + return; + Directory.Delete(Disk("dir"), recursive: true); + File.WriteAllText(Disk("dir"), "a file now"); + }); + Assert.IsTrue(await provider.DeleteAsync("dir")); + Assert.IsFalse(Path.Exists(Disk("dir"))); + } + + [TestMethod] + public async Task AnEntryTurnedFromAFolderIntoAFileMeanwhile_IsDeleted() + { + Write("x", "dir", "sub", "x.txt"); + using var provider = Provider(opening: path => + { + if (path != Disk("dir", "sub") || File.Exists(Disk("dir", "sub"))) + return; + Directory.Delete(Disk("dir", "sub"), recursive: true); + File.WriteAllText(Disk("dir", "sub"), "a file now"); + }); + Assert.IsTrue(await provider.DeleteAsync("dir")); + Assert.IsFalse(Path.Exists(Disk("dir"))); + } + + [TestMethod] + public async Task AnEntryGoneBetweenTheListingAndItsOpen_IsSkipped() + { + Write("x", "dir", "x.txt"); + Write("y", "dir", "y.txt"); + using var provider = Provider(opening: path => + { + if (path == Disk("dir", "x.txt")) + File.Delete(path); + }); + Assert.IsTrue(await provider.DeleteAsync("dir")); + Assert.IsFalse(Path.Exists(Disk("dir"))); + } + + [TestMethod] + public async Task ATargetGoneBeforeItIsOpened_DeletesNothing() + { + Write("x", "dir", "x.txt"); + using var provider = Provider(opening: path => + { + if (path == Disk("dir")) + Directory.Delete(path, recursive: true); + }); + Assert.IsFalse(await provider.DeleteAsync("dir")); + } + + // ----- Links that were there ----- + + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public async Task ALinkAtTheTarget_IsRemoved_ItsTargetKept(bool targetInside) + { + Write("inside", "real", "x.txt"); + FolderLink(Disk("link"), targetInside ? Disk("real") : _outside); + using var provider = Provider(); + Assert.IsTrue(await provider.DeleteAsync("link")); + Assert.IsFalse(Path.Exists(Disk("link"))); + Assert.AreEqual("inside", File.ReadAllText(Disk("real", "x.txt"))); + AssertOutsideIsIntact(); + } + + [TestMethod] + public async Task WithRefuseLinks_ALinkAtTheTarget_IsRefused() + { + FolderLink(Disk("link"), _outside); + using var provider = Provider(refuseLinks: true); + await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("link")); + Assert.IsTrue(Path.Exists(Disk("link"))); + AssertOutsideIsIntact(); + } + + [TestMethod] + [DataRow(false)] + [DataRow(true)] + public async Task AFolderHoldingLinks_IsDeleted_TheirTargetsKept(bool refuseLinks) + { + Write("real", "real.txt"); + Write("x", "dir", "x.txt"); + FolderLink(Disk("dir", "folder-link"), _outside); + var fileLinks = TryFileLink(Disk("dir", "file-link.txt"), Disk("real.txt")) && TryFileLink(Disk("dir", "dangling"), Disk("nothing")); + using var provider = Provider(refuseLinks); + Assert.IsTrue(await provider.DeleteAsync("dir")); + Assert.IsFalse(Path.Exists(Disk("dir"))); + Assert.AreEqual("real", File.ReadAllText(Disk("real.txt")), fileLinks ? "A link to a file is removed as itself" : ""); + AssertOutsideIsIntact(); + } + + // ----- What the system refuses ----- + + [TestMethod] + public async Task AFileOpenForReadingInside_IsDeleted() + { + Write("x", "dir", "x.txt"); + Write("y", "dir", "sub", "y.txt"); + using var provider = Provider(); + using (new FileStream(Disk("dir", "sub", "y.txt"), FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete)) + Assert.IsTrue(await provider.DeleteAsync("dir")); + Assert.IsFalse(Path.Exists(Disk("dir"))); + } + + [TestMethod] + [OSCondition(OperatingSystems.Windows)] + public async Task OnWindows_AReadOnlyFile_IsRefused_AndTheRestDeleted() + { + Write("other", "dir", "other.txt"); + Write("kept", "dir", "read-only.txt"); + File.SetAttributes(Disk("dir", "read-only.txt"), FileAttributes.ReadOnly); + try + { + using var provider = Provider(); + var e = await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); + StringAssert.Contains(e.Message, "'dir/read-only.txt'"); + Assert.IsFalse(File.Exists(Disk("dir", "other.txt")), "The rest is deleted"); + Assert.IsTrue(File.Exists(Disk("dir", "read-only.txt"))); + } + finally + { + File.SetAttributes(Disk("dir", "read-only.txt"), FileAttributes.Normal); + } + } + + [TestMethod] + [OSCondition(OperatingSystems.Linux | OperatingSystems.OSX)] + [UnsupportedOSPlatform("windows")] + public async Task OnLinuxAndMacOS_AFolderThatMayNotBeChanged_IsRefused_AndTheRestDeleted() + { + Write("other", "dir", "other.txt"); + Write("x", "dir", "locked", "x.txt"); + // Readable and searchable, so its entry can be found, but not writable: x.txt cannot be removed from it. + File.SetUnixFileMode(Disk("dir", "locked"), UnixFileMode.UserRead | UnixFileMode.UserExecute); + try + { + if (Environment.UserName == "root") + Assert.Inconclusive("The tests run as root, whom file permissions do not stop."); + using var provider = Provider(); + var e = await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); + StringAssert.Contains(e.Message, "'dir/locked/x.txt'"); + Assert.IsFalse(File.Exists(Disk("dir", "other.txt")), "The rest is deleted"); + Assert.IsTrue(File.Exists(Disk("dir", "locked", "x.txt"))); + } + finally + { + File.SetUnixFileMode(Disk("dir", "locked"), UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + } + } + + [TestMethod] + [OSCondition(OperatingSystems.Linux | OperatingSystems.OSX)] + [UnsupportedOSPlatform("windows")] + public async Task OnLinuxAndMacOS_AFolderThatMayNotBeSearched_IsRefused() + { + Write("x", "dir", "closed", "x.txt"); + // Readable, so it lists, but not searchable: its entries cannot be opened. + File.SetUnixFileMode(Disk("dir", "closed"), UnixFileMode.UserRead | UnixFileMode.UserWrite); + try + { + if (Environment.UserName == "root") + Assert.Inconclusive("The tests run as root, whom file permissions do not stop."); + using var provider = Provider(); + await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); + } + finally + { + File.SetUnixFileMode(Disk("dir", "closed"), UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + } + } + + // ----- The other ways a folder is deleted ----- + + [TestMethod] + public async Task WhereTheSystemIsNotKnown_AFolderIsDeletedByItsPath() + { + Write("x", "dir", "sub", "x.txt"); + using var provider = new WriteablePhysicalFileProvider(_root) { Folders = null }; + Assert.IsTrue(await provider.DeleteAsync("dir")); + Assert.IsFalse(Path.Exists(Disk("dir"))); + } + + [TestMethod] + public async Task TheClassicDelete_DeletesTheSameTree() + { + Write("x", "dir", "x.txt"); + Write("y", "dir", "sub", "y.txt"); + FolderLink(Disk("dir", "link"), _outside); + using var provider = new WriteablePhysicalFileProvider(_root) { ClassicDeletes = true }; + Assert.IsTrue(await provider.DeleteAsync("dir")); + Assert.IsFalse(Path.Exists(Disk("dir"))); + AssertOutsideIsIntact(); + } + + [TestMethod] + [OSCondition(OperatingSystems.Windows)] + public async Task OnWindows_TheClassicDelete_LeavesAFolderWithAFileOpenInIt_AndSaysSo() + { + // Without POSIX deletes, a file open elsewhere keeps its name until it is closed, so its folder is not empty. + Write("x", "dir", "x.txt"); + using var provider = new WriteablePhysicalFileProvider(_root) { ClassicDeletes = true }; + using (new FileStream(Disk("dir", "x.txt"), FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete)) + { + var e = await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); + StringAssert.Contains(e.Message, "'dir'"); + } + Assert.IsFalse(File.Exists(Disk("dir", "x.txt")), "The file goes once it is closed"); + } + + // ----- Errors, through a system that reports what a disk can't be made to ----- + + [TestMethod] + public async Task ARemovalRefused_IsUnauthorizedAccess_AndTheRestDeleted() + { + Write("x", "dir", "x.txt"); + Write("y", "dir", "y.txt"); + var folders = new ScriptedFolders(FolderSystem.Current!) { RemoveErrors = { ["x.txt"] = WindowsRules.AccessDenied } }; + using var provider = new WriteablePhysicalFileProvider(_root) { Folders = folders }; + var e = await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); + StringAssert.Contains(e.Message, "'dir/x.txt'"); + Assert.IsFalse(File.Exists(Disk("dir", "y.txt"))); + } + + [TestMethod] + [DataRow(WindowsRules.InvalidParameter, true)] + [DataRow(WindowsRules.NotSupported, true)] + [DataRow(WindowsRules.InvalidFunction, true)] + [DataRow(WindowsRules.AccessDenied, false)] + public async Task APosixDeleteRefused_IsTriedTheClassicWay_OnlyForTheFileSystemsThatRefuseIt(int error, bool fallsBack) + { + Write("x", "dir", "x.txt"); + var folders = new ScriptedFolders(FolderSystem.Current!) { RemoveErrors = { ["x.txt"] = error } }; + using var provider = new WriteablePhysicalFileProvider(_root) { Folders = folders }; + if (fallsBack) + { + Assert.IsTrue(await provider.DeleteAsync("dir")); + CollectionAssert.Contains(folders.Removed, ("x.txt", true)); + } + else + { + await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); + CollectionAssert.DoesNotContain(folders.Removed, ("x.txt", true)); + } + } + + [TestMethod] + public async Task AFolderStillNotEmpty_IsIOException() + { + Write("x", "dir", "sub", "x.txt"); + var folders = new ScriptedFolders(FolderSystem.Current!) { RemoveErrors = { ["sub"] = WindowsRules.DirectoryNotEmpty } }; + using var provider = new WriteablePhysicalFileProvider(_root) { Folders = folders }; + var e = await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); + StringAssert.Contains(e.Message, "'dir/sub'"); + } + + [TestMethod] + public async Task AnEntryThatCannotBeOpened_FailsTheDelete_AndTheRestGoes() + { + Write("x", "dir", "x.txt"); + Write("y", "dir", "y.txt"); + var folders = new ScriptedFolders(FolderSystem.Current!) { OpenErrors = { ["x.txt"] = WindowsRules.AccessDenied } }; + using var provider = new WriteablePhysicalFileProvider(_root) { Folders = folders }; + await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); + Assert.IsTrue(File.Exists(Disk("dir", "x.txt"))); + Assert.IsFalse(File.Exists(Disk("dir", "y.txt"))); + } + + [TestMethod] + public async Task AFolderThatCannotBeListed_FailsTheDelete() + { + Write("x", "dir", "x.txt"); + var folders = new ScriptedFolders(FolderSystem.Current!) { ListsNothing = true }; + using var provider = new WriteablePhysicalFileProvider(_root) { Folders = folders }; + await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); + Assert.IsTrue(File.Exists(Disk("dir", "x.txt"))); + } + + [TestMethod] + [DataRow(WindowsRules.PathNotFound, false)] + [DataRow(WindowsRules.AccessDenied, true)] + public async Task TheStart_GoneDeletesNothing_RefusedThrows(int error, bool throws) + { + Write("x", "a", "b", "x.txt"); + foreach (var refuseLinks in new[] { false, true }) + { + // Without RefuseLinks the folder that holds the target is opened by its path; with it, the root is, then + // each folder on the way is opened in its parent; then the target in its folder. + foreach (var folders in new[] + { + new ScriptedFolders(FolderSystem.Current!) { OpenPathError = error }, + new ScriptedFolders(FolderSystem.Current!) { OpenErrors = { ["a"] = error } }, + new ScriptedFolders(FolderSystem.Current!) { OpenErrors = { ["b"] = error } }, + }) + { + if (!refuseLinks && folders.OpenErrors.ContainsKey("a")) + continue; + using var provider = new WriteablePhysicalFileProvider(new PhysicalFileProviderOptions { RootDirectoryPath = _root, RefuseLinks = refuseLinks }) { Folders = folders }; + if (throws) + await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("a/b"), $"RefuseLinks {refuseLinks}"); + else + Assert.IsFalse(await provider.DeleteAsync("a/b"), $"RefuseLinks {refuseLinks}"); + Assert.IsTrue(File.Exists(Disk("a", "b", "x.txt"))); + } + } + } + + [TestMethod] + public async Task TheTargetRefusingItsRemoval_Throws() + { + Write("x", "dir", "x.txt"); + var folders = new ScriptedFolders(FolderSystem.Current!) { RemoveErrors = { ["dir"] = WindowsRules.AccessDenied } }; + using var provider = new WriteablePhysicalFileProvider(_root) { Folders = folders }; + var e = await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); + StringAssert.Contains(e.Message, "'dir'"); + } + + [TestMethod] + public async Task TheTargetGoneAsItIsRemoved_StillCountsAsDeleted() + { + Write("x", "dir", "x.txt"); + var folders = new ScriptedFolders(FolderSystem.Current!) { RemoveErrors = { ["dir"] = WindowsRules.FileNotFound } }; + using var provider = new WriteablePhysicalFileProvider(_root) { Folders = folders }; + Assert.IsTrue(await provider.DeleteAsync("dir")); + } + + // ----- The rules ----- + + [TestMethod] + public void UnixTable_IsPickedBySystemAndArchitecture() + { + Assert.AreSame(UnixTable.Linux, UnixTable.For(linux: true, macOS: false, Architecture.X64)); + Assert.AreSame(UnixTable.LinuxArm, UnixTable.For(linux: true, macOS: false, Architecture.Arm64)); + Assert.AreSame(UnixTable.LinuxArm, UnixTable.For(linux: true, macOS: false, Architecture.Arm)); + Assert.AreSame(UnixTable.MacOS, UnixTable.For(linux: false, macOS: true, Architecture.Arm64)); + Assert.AreSame(UnixTable.MacOS, UnixTable.For(linux: false, macOS: true, Architecture.X64)); + Assert.IsNull(UnixTable.For(linux: true, macOS: false, Architecture.S390x), "An architecture whose values aren't known"); + Assert.IsNull(UnixTable.For(linux: true, macOS: false, Architecture.RiscV64)); + Assert.IsNull(UnixTable.For(linux: false, macOS: false, Architecture.X64), "Another system"); + Assert.IsNotNull(FolderSystem.Current, "Every system CI runs on deletes through handles"); + } + + [TestMethod] + public void UnixTable_ComposesItsFlags() + { + Assert.AreEqual(0x200000 | 0x10000 | 0x80000, UnixTable.Linux.OpenFolder); + Assert.AreEqual(0x200000 | 0x10000 | 0x80000 | 0x20000, UnixTable.Linux.OpenEntry); + Assert.AreEqual(0x200000 | 0x4000 | 0x80000 | 0x8000, UnixTable.LinuxArm.OpenEntry); + Assert.AreEqual(0x100000 | 0x1000000 | 0x100, UnixTable.MacOS.OpenEntry); + Assert.AreEqual(Failure.Gone, UnixTable.Linux.Classify(UnixTable.NoEntry)); + Assert.AreEqual(Failure.Gone, UnixTable.Linux.Classify(UnixTable.NotADirectory)); + Assert.AreEqual(Failure.Refused, UnixTable.Linux.Classify(UnixTable.AccessDenied)); + Assert.AreEqual(Failure.Refused, UnixTable.MacOS.Classify(UnixTable.NotPermitted)); + Assert.AreEqual(Failure.NotEmpty, UnixTable.Linux.Classify(39)); + Assert.AreEqual(Failure.NotEmpty, UnixTable.MacOS.Classify(66)); + Assert.AreEqual(Failure.Other, UnixTable.MacOS.Classify(39), "39 is another error on macOS"); + } + + [TestMethod] + public void WindowsRules_ReadWhatWindowsReports() + { + const uint directory = WindowsRules.DirectoryAttribute; + const uint reparse = WindowsRules.ReparsePointAttribute; + Assert.AreEqual(EntryKind.Folder, WindowsRules.KindOf(directory, 0)); + Assert.AreEqual(EntryKind.Other, WindowsRules.KindOf(0x20, 0)); + Assert.AreEqual(EntryKind.Link, WindowsRules.KindOf(directory | reparse, 0xA0000003), "A junction or a mount point"); + Assert.AreEqual(EntryKind.Link, WindowsRules.KindOf(reparse, 0xA000000C), "A symbolic link to a file"); + Assert.AreEqual(EntryKind.Folder, WindowsRules.KindOf(directory | reparse, 0x9000001A), "A cloud placeholder holds its own content"); + Assert.AreEqual(EntryKind.Other, WindowsRules.KindOf(reparse, 0x80000013), "A deduplicated file"); + Assert.AreEqual(Failure.Gone, WindowsRules.Classify(WindowsRules.FileNotFound)); + Assert.AreEqual(Failure.Gone, WindowsRules.Classify(WindowsRules.PathNotFound)); + Assert.AreEqual(Failure.Refused, WindowsRules.Classify(WindowsRules.AccessDenied)); + Assert.AreEqual(Failure.NotEmpty, WindowsRules.Classify(WindowsRules.DirectoryNotEmpty)); + Assert.AreEqual(Failure.Other, WindowsRules.Classify(32)); + Assert.IsTrue(WindowsRules.FallsBack(WindowsRules.InvalidParameter)); + Assert.IsFalse(WindowsRules.FallsBack(WindowsRules.AccessDenied)); + } + + [TestMethod] + public void WindowsRules_GiveAPathItsExtendedForm() + { + Assert.AreEqual(@"\\?\C:\data\files", WindowsRules.ExtendedPath(@"C:\data\files")); + Assert.AreEqual(@"\\?\C:\", WindowsRules.ExtendedPath(@"C:\")); + Assert.AreEqual(@"\\?\UNC\server\share\files", WindowsRules.ExtendedPath(@"\\server\share\files")); + } + + // The system as it is, but for what a test scripts by entry name: an error opening it, an error removing it (the + // POSIX way; the classic way is the system's own), a folder by path that cannot be opened, and listings that read + // nowhere. It reads every error by Windows' rules, whatever the system. + private sealed class ScriptedFolders(FolderSystem system) : FolderSystem + { + public Dictionary OpenErrors { get; } = []; + + public Dictionary RemoveErrors { get; } = []; + + public int? OpenPathError { get; init; } + + public bool ListsNothing { get; init; } + + public List<(string Name, bool Classic)> Removed { get; } = []; + + public override SafeHandle? OpenPath(string path, out int error) + { + if (OpenPathError is { } scripted) + { + error = scripted; + return null; + } + return system.OpenPath(path, out error); + } + + public override Entry Open(SafeHandle folder, string name, bool forRemoval) => + OpenErrors.TryGetValue(name, out var error) ? Entry.Failed(error) : system.Open(folder, name, forRemoval); + + public override int Remove(SafeHandle folder, string name, Entry entry, bool classic) + { + Removed.Add((name, classic)); + return !classic && RemoveErrors.TryGetValue(name, out var error) ? error : system.Remove(folder, name, entry, classic); + } + + public override Failure Classify(int error) => WindowsRules.Classify(error); + + public override bool FallsBack(int error) => WindowsRules.FallsBack(error); + + public override string ListingPath(string path) => ListsNothing ? Path.Combine(path, "nowhere") : system.ListingPath(path); + } +} From 77a96a951916251fa2463fcb8911905ba0288d72 Mon Sep 17 00:00:00 2001 From: Osvay Soma Date: Tue, 13 Oct 2026 11:41:53 +0200 Subject: [PATCH 2/5] Log step 3 in the 2027.1.1 plan Refs #70 --- docs/plans/2026-10-13-2027-1-1.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/plans/2026-10-13-2027-1-1.md b/docs/plans/2026-10-13-2027-1-1.md index 0cf9ceb..1769e2f 100644 --- a/docs/plans/2026-10-13-2027-1-1.md +++ b/docs/plans/2026-10-13-2027-1-1.md @@ -466,4 +466,12 @@ expectation (the winner is done before the loser retries); the locked restore of them, so Darwin's limit (32) and Linux's (40) are pinned on the kernel. The docs name the jobs, and the Native AOT job that the release docs had left out. Code review: round 1, no blocker or major, five fixes (two on the maintainer's yes); round 2 clean. On the maintainer's choice, both jobs become required checks on `main` after the merge (P8's - ruleset question). + ruleset question). Merged as #75; `main`'s ruleset now requires both. +- **2026-10-13, step 3 (#70):** `FolderDelete` (the walk, what an entry is, the errors, and the rules: `UnixTable`, + `WindowsRules`), `UnixFolders` and `WindowsFolders`, written before step 1 merged and rebased onto it. As built: the + seam is `FolderSystem`, an abstract class whose `Current` is the system's or null, and the provider's `Folders` + property (null deletes by path) stands for the plan's `DeletesByHandle`; the classic-delete switch is + `ClassicDeletes`; the seam's `Open` returns an entry already classified by shared rules (`WindowsRules.KindOf`, and + `UnixFolders`, measured on Linux), so `WindowsFolders` keeps only the calls; a folder that cannot be listed records + the failure like any other and the walk throws it at the end. Run on Windows and on Linux in a container, Physical's + coverage on Linux 98.48% of lines and 95.00% of branches before the review. From 068fbf04e4c5f0c408914bf36a0ab33f85b63a8d Mon Sep 17 00:00:00 2001 From: Osvay Soma Date: Tue, 13 Oct 2026 12:23:56 +0200 Subject: [PATCH 3/5] Keep every listed name inside its folder, and let a folder delete be cancelled Some file systems (FUSE, some NFS servers) list '.' and '..' as files, which .NET then reports: opened in the folder's handle, '..' would have been its parent and '.' the folder again. The walk now skips them and refuses a name holding a separator, and lists through the system seam so a test can hand it such names. A subfolder gone between its open and its listing is skipped, and Windows' refusal to remove a folder already deleted counts as gone. The walk checks the cancellation token between entries. A test holds folders on the way open without delete sharing, which fails if the walk asks for delete access there. The README says where the walk runs and that elsewhere a delete by path can still be steered; unused classifications go. Refs #70 --- CHANGELOG.md | 4 +- docs/plans/2026-10-13-2027-1-1.md | 10 +- .../FolderDelete.cs | 104 ++++++++---- src/WriteableFileProviders.Physical/README.md | 15 +- .../UnixFolders.cs | 8 +- .../WriteablePhysicalFileProvider.cs | 13 +- .../PhysicalFolderDeleteTests.cs | 149 ++++++++++++++++-- ...iteableFileProviders.Physical.Tests.csproj | 5 + 8 files changed, 239 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e510f97..e6d5dd7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,9 @@ version's section as its GitHub release notes. opened the same way and a link swapped in there is refused. On Windows a folder holding a junction is now deleted (before, `UnauthorizedAccessException`). A folder that still has entries after the delete (a file another process holds open on a file system without POSIX deletes) throws `IOException`, where it returned true. On macOS the folder - that holds the deleted one, and with `RefuseLinks` every folder on the way, must be readable. + that holds the deleted one, and with `RefuseLinks` every folder on the way, must be readable. The delete can now be + cancelled: it stops between entries, the folder partly deleted. Elsewhere (Linux on other architectures, other + systems) a folder is deleted by its path, as before. ## 2027.1.0 (2026-10-13) diff --git a/docs/plans/2026-10-13-2027-1-1.md b/docs/plans/2026-10-13-2027-1-1.md index 1769e2f..fb46975 100644 --- a/docs/plans/2026-10-13-2027-1-1.md +++ b/docs/plans/2026-10-13-2027-1-1.md @@ -474,4 +474,12 @@ expectation (the winner is done before the loser retries); the locked restore of `ClassicDeletes`; the seam's `Open` returns an entry already classified by shared rules (`WindowsRules.KindOf`, and `UnixFolders`, measured on Linux), so `WindowsFolders` keeps only the calls; a folder that cannot be listed records the failure like any other and the walk throws it at the end. Run on Windows and on Linux in a container, Physical's - coverage on Linux 98.48% of lines and 95.00% of branches before the review. + coverage on Linux 98.48% of lines and 95.00% of branches before the review. Code review, deep tier (the maintainer's + choice), round 1: no blocker; three majors, all fixed on the maintainer's yes: a listing that names `.` or `..` as + files (FUSE, some NFS servers) would have opened the folder's parent, or itself, through its handle, so a listed name + is now only ever an entry of the folder (`.` and `..` skipped, a separator refused), with listing behind the seam so a + test can hand it those names (F1); the R3 test (a folder on the way held open without delete sharing), shown to fail + when the walk asks for delete access there (F2); the README's account of where the walk runs and the fallback (F3). + Minors: a subfolder gone between its open and its listing is skipped (F4); the walk is cancellable between entries + (F5); `Failure.NotEmpty` and the table's `ENOTEMPTY` go, so a folder not empty is "anything else" (F6, a departure + from D6's table); one representation of gone (F7). F8, dropping `ClassicDeletes`, was declined: the plan names it. diff --git a/src/WriteableFileProviders.Physical/FolderDelete.cs b/src/WriteableFileProviders.Physical/FolderDelete.cs index ed2e8bb..24ff616 100644 --- a/src/WriteableFileProviders.Physical/FolderDelete.cs +++ b/src/WriteableFileProviders.Physical/FolderDelete.cs @@ -7,8 +7,8 @@ namespace WriteableFileProviders.Physical; // What an entry of a folder is, opened without following a link. internal enum EntryKind { - // Nothing has the name (any more). - Missing, + // None: the entry could not be opened (Entry.Failed). + Unknown, // A folder of its own, not a link to one. Folder, @@ -29,10 +29,7 @@ internal enum Failure // The system refused the access. Refused, - // A folder still has entries. - NotEmpty, - - // Anything else. + // Anything else, a folder that still has entries included. Other, } @@ -40,7 +37,7 @@ internal enum Failure // the system's error number when it could not be opened. internal readonly record struct Entry(EntryKind Kind, SafeHandle? Handle, int Error) { - public static Entry Failed(int error) => new(EntryKind.Missing, null, error); + public static Entry Failed(int error) => new(EntryKind.Unknown, null, error); } // The calls a folder delete makes to the system, through handles: an entry is always opened relative to its folder's @@ -75,13 +72,22 @@ internal abstract class FolderSystem // The path to list a folder by. public virtual string ListingPath(string path) => path; + // The names in a folder, read from its path as the walk goes; opening it is where a folder that can't be listed + // fails. + public virtual IEnumerator List(string path) => + new FileSystemEnumerable(ListingPath(path), (ref FileSystemEntry entry) => entry.FileName.ToString(), + _listing).GetEnumerator(); + + private static readonly EnumerationOptions _listing = new() { AttributesToSkip = 0, IgnoreInaccessible = false }; + // The system's words for an error number. public virtual string Describe(int error) => Marshal.GetPInvokeErrorMessage(error); } // The flags and error numbers of the C library calls, which differ by system and, on Linux, by architecture. From the // kernel's and Darwin's headers. -internal sealed record UnixTable(int Directory, int NoFollow, int PathOnly, int CloseOnExec, int RemoveDirectory, int Loop, int NotEmpty) +internal sealed record UnixTable( + int Directory, int NoFollow, int PathOnly, int CloseOnExec, int RemoveDirectory, int Loop) { // The same on Linux and macOS. public const int NoEntry = 2; @@ -91,13 +97,13 @@ internal sealed record UnixTable(int Directory, int NoFollow, int PathOnly, int public const int Invalid = 22; // Linux on x64 (and the architectures that use the generic values); O_PATH opens a folder without reading it. - public static UnixTable Linux { get; } = new(0x10000, 0x20000, 0x200000, 0x80000, 0x200, 40, 39); + public static UnixTable Linux { get; } = new(0x10000, 0x20000, 0x200000, 0x80000, 0x200, 40); // Linux on Arm and Arm64, whose O_DIRECTORY and O_NOFOLLOW differ. - public static UnixTable LinuxArm { get; } = new(0x4000, 0x8000, 0x200000, 0x80000, 0x200, 40, 39); + public static UnixTable LinuxArm { get; } = new(0x4000, 0x8000, 0x200000, 0x80000, 0x200, 40); // macOS, on Intel and Apple silicon; it has no O_PATH, so a folder is opened to read (O_RDONLY, 0). - public static UnixTable MacOS { get; } = new(0x100000, 0x100, 0, 0x1000000, 0x80, 62, 66); + public static UnixTable MacOS { get; } = new(0x100000, 0x100, 0, 0x1000000, 0x80, 62); // The table of a system, by its architecture as this process runs (an x64 process emulated on Arm is x64); null // where the values are not known. @@ -116,11 +122,10 @@ internal sealed record UnixTable(int Directory, int NoFollow, int PathOnly, int // Opens an entry only if it is a folder of its own: a link fails (ENOTDIR or ELOOP), a file fails (ENOTDIR). public int OpenEntry => OpenFolder | NoFollow; - // Not a folder on the way (ENOTDIR) is as gone as nothing there. - public Failure Classify(int error) => + // Not a folder on the way (ENOTDIR) is as gone as nothing there; the same numbers on Linux and macOS. + public static Failure Classify(int error) => error is NoEntry or NotADirectory ? Failure.Gone : error is AccessDenied or NotPermitted ? Failure.Refused - : error == NotEmpty ? Failure.NotEmpty : Failure.Other; } @@ -153,7 +158,6 @@ public static Failure Classify(int error) => { FileNotFound or PathNotFound => Failure.Gone, AccessDenied => Failure.Refused, - DirectoryNotEmpty => Failure.NotEmpty, _ => Failure.Other, }; @@ -169,22 +173,27 @@ public static string ExtendedPath(string path) => // Deletes a folder and everything in it through the handles of FolderSystem. The names come from listing each folder // by its path; every entry is opened relative to its folder's handle without following a link and removed through -// that handle or its own, so a listing that a rename or a swapped link made read another folder can only leave names -// out, which then fails the delete (the folder is not empty), never reach outside the folder. Like .NET's recursive -// delete, it goes on past an entry it cannot delete, skips one gone meanwhile, and throws the first failure at the end. +// that handle or its own. A name is only ever one entry of the folder: '.' and '..', which some file systems list as +// files, are skipped, and a name holding a separator is refused, so a listing that a rename or a swapped link made +// read another folder can only leave names out, which then fails the delete (the folder is not empty), never reach +// outside the folder. Like .NET's recursive delete, it goes on past an entry it cannot delete, skips one gone +// meanwhile, and throws the first failure at the end. Cancelled, it stops between entries, the folder partly deleted. internal sealed class FolderDelete(FolderSystem system, Action? opening, bool classicDeletes) { - private static readonly EnumerationOptions _listing = new() { AttributesToSkip = 0, IgnoreInaccessible = false }; + private static readonly char[] _separators = [Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar]; // Deletes the folder at root/segments, whose provider path is 'path': true when something was deleted, false when // nothing was there by the time it was opened. With 'refuseLinks', the walk starts at the root and opens each // folder on the way without following a link, and a link there or at the target throws 'refused' for its path // under the root; without, the folder that holds the target is opened by its path, following links as every // member does. - public bool Delete(ResolvedPath path, string root, IReadOnlyList segments, bool refuseLinks, Func refused) + public bool Delete(ResolvedPath path, string root, IReadOnlyList segments, bool refuseLinks, + Func refused, CancellationToken cancellationToken) { var fullPath = Path.Join([root, .. segments]); - var parent = refuseLinks ? WalkDown(path, root, segments, refused) : OpenPath(path, Path.GetDirectoryName(fullPath)!); + var parent = refuseLinks + ? WalkDown(path, root, segments, refused, cancellationToken) + : OpenPath(path, Path.GetDirectoryName(fullPath)!); if (parent is null) return false; using (parent) @@ -198,17 +207,15 @@ public bool Delete(ResolvedPath path, string root, IReadOnlyList segment return system.Classify(target.Error) == Failure.Gone ? false : throw Failed(path.ToString(), target.Error); switch (target.Kind) { - case EntryKind.Missing: - return false; case EntryKind.Link when refuseLinks: throw refused(string.Join('/', segments)); case EntryKind.Folder: - DeleteContents(target.Handle!, fullPath, path.ToString()); + DeleteContents(target.Handle!, fullPath, path.ToString(), cancellationToken); break; } // A link is removed as itself; a file put where the folder was is what the path names now. var error = Remove(parent, name, target); - if (error != 0 && system.Classify(error) != Failure.Gone) + if (error != 0 && !IsGone(error, fullPath)) throw Failed(path.ToString(), error); return true; } @@ -217,12 +224,18 @@ public bool Delete(ResolvedPath path, string root, IReadOnlyList segment // The folder that holds the target, reached from the root one folder at a time without following a link: null when // a folder on the way is gone or is not a folder. - private SafeHandle? WalkDown(ResolvedPath path, string root, IReadOnlyList segments, Func refused) + private SafeHandle? WalkDown(ResolvedPath path, string root, IReadOnlyList segments, + Func refused, CancellationToken cancellationToken) { // The root may itself be a link: it is opened by its path. var folder = OpenPath(path, root); for (var i = 0; folder is not null && i < segments.Count - 1; i++) { + if (cancellationToken.IsCancellationRequested) + { + folder.Dispose(); + cancellationToken.ThrowIfCancellationRequested(); + } opening?.Invoke(Path.Join([root, .. segments.Take(i + 1)])); Entry entry; using (folder) @@ -250,7 +263,8 @@ public bool Delete(ResolvedPath path, string root, IReadOnlyList segment } // Empties the folder whose handle is 'folder' and whose path is 'fullPath' ('relative' under the root, for messages). - private void DeleteContents(SafeHandle folder, string fullPath, string relative) + private void DeleteContents(SafeHandle folder, string fullPath, string relative, + CancellationToken cancellationToken) { Exception? first = null; IEnumerator? names = null; @@ -258,7 +272,18 @@ private void DeleteContents(SafeHandle folder, string fullPath, string relative) { while (Next(fullPath, ref names, ref first)) { + cancellationToken.ThrowIfCancellationRequested(); var name = names.Current; + // A folder's own entries, which some file systems list as files (FUSE, NFS): opened in the folder, '..' + // would be its parent and '.' itself. + if (name is "." or "..") + continue; + if (name.AsSpan().IndexOfAny(_separators) >= 0) + { + first ??= new IOException( + $"'{relative}' could not be emptied: its listing named '{name}', which is not the name of an entry."); + continue; + } var childPath = Path.Join(fullPath, name); var childRelative = relative + "/" + name; opening?.Invoke(childPath); @@ -271,13 +296,11 @@ private void DeleteContents(SafeHandle folder, string fullPath, string relative) first ??= Failed(childRelative, entry.Error); continue; } - if (entry.Kind == EntryKind.Missing) - continue; if (entry.Kind == EntryKind.Folder) { try { - DeleteContents(entry.Handle!, childPath, childRelative); + DeleteContents(entry.Handle!, childPath, childRelative, cancellationToken); } catch (Exception e) when (e is IOException or UnauthorizedAccessException) { @@ -287,7 +310,7 @@ private void DeleteContents(SafeHandle folder, string fullPath, string relative) } } var error = Remove(folder, name, entry); - if (error != 0 && system.Classify(error) != Failure.Gone) + if (error != 0 && !IsGone(error, childPath)) first ??= Failed(childRelative, error); } } @@ -301,22 +324,33 @@ private void DeleteContents(SafeHandle folder, string fullPath, string relative) } // The next name of the folder, listed by its path as the walk goes: false at the end, or when the listing fails, - // which is recorded. + // which is recorded, but for a folder gone since it was opened, whose removal then finds nothing to do. private bool Next(string fullPath, [NotNullWhen(true)] ref IEnumerator? names, ref Exception? first) { try { - names ??= new FileSystemEnumerable(system.ListingPath(fullPath), (ref FileSystemEntry entry) => entry.FileName.ToString(), _listing).GetEnumerator(); + names ??= system.List(fullPath); return names.MoveNext(); } catch (Exception e) when (e is IOException or UnauthorizedAccessException) { - // The folder could not be listed, or no longer can: what is left of it fails its own removal. - first ??= e; + if (e is not DirectoryNotFoundException || names is not null) + first ??= e; return false; } } + // Whether a removal failed because the entry is gone: as the system says, or, where it answers that the access is + // refused (Windows, for a folder another process deleted while this one held it), because nothing has the name any + // more. Only the answer depends on it, never what is deleted. + private bool IsGone(int error, string fullPath) => + system.Classify(error) switch + { + Failure.Gone => true, + Failure.Refused => (int)new FileInfo(fullPath).Attributes == -1, + _ => false, + }; + private int Remove(SafeHandle folder, string name, Entry entry) { var error = system.Remove(folder, name, entry, classicDeletes); diff --git a/src/WriteableFileProviders.Physical/README.md b/src/WriteableFileProviders.Physical/README.md index cbdf934..d502cff 100644 --- a/src/WriteableFileProviders.Physical/README.md +++ b/src/WriteableFileProviders.Physical/README.md @@ -72,14 +72,17 @@ None. The provider reads and writes with the permissions of the process. - **Symbolic links and junctions** inside the root are followed, wherever they point, unless you set `RefuseLinks`. Without it, a recursive listing lists a linked folder but doesn't walk into it. `RefuseLinks` checks the path just before each operation, so a process that can create links inside the root can still swap one in between, except for - a folder delete, which opens every folder from the root without following a link. Hard links and mounts on + a folder delete on Linux (x64, Arm, Arm64), macOS and Windows, which opens every folder from the root without + following a link. Hard links and mounts on Linux and macOS aren't visible as links, and other reparse points (cloud placeholders, Windows container layers) aren't links. The polling `Watch` can still report a change behind a linked folder. -- **A folder delete never leaves the folder.** It opens each entry in its folder without following a link and removes - it there, so a link inside, even one swapped in while it runs, is removed as itself. It goes on past what it can't - delete and throws the first failure. On Windows a folder holding a junction is deleted. On macOS the folder above the - deleted one must be readable. Where the system's calls aren't known (Linux other than on x64 and Arm), the delete - goes by path, as .NET's does. +- **A folder delete never leaves the folder** on Linux (x64, Arm, Arm64), macOS and Windows. It opens each entry in its + folder without following a link and removes it there, so a link inside, even one swapped in while it runs, is + removed as itself. It goes on past what it can't delete and throws the first failure; cancelled, it stops between + entries, the folder partly deleted. On Windows a folder holding a junction is deleted. On macOS the folder that holds + the deleted one, and with `RefuseLinks` every folder on the way, must be readable. Elsewhere (Linux on other + architectures, other systems) the delete goes by path, as .NET's does, and a link swapped in while it runs can still + steer it outside the folder. - **No content types.** `UpdateFileOptions.ContentType` is ignored; `ContentType` is null. - **`Watch` works** like ASP.NET Core's `PhysicalFileProvider`: a glob relative to the root. Hidden and dot-prefixed files aren't watched. Where file system events don't arrive (some network and container mounts), set diff --git a/src/WriteableFileProviders.Physical/UnixFolders.cs b/src/WriteableFileProviders.Physical/UnixFolders.cs index 599ae4b..417fee9 100644 --- a/src/WriteableFileProviders.Physical/UnixFolders.cs +++ b/src/WriteableFileProviders.Physical/UnixFolders.cs @@ -22,8 +22,6 @@ public override Entry Open(SafeHandle folder, string name, bool forRemoval) if (fd >= 0) return new Entry(EntryKind.Folder, new SafeFileHandle(fd, ownsHandle: true), 0); var error = Marshal.GetLastPInvokeError(); - if (error == UnixTable.NoEntry) - return new Entry(EntryKind.Missing, null, 0); if (error != UnixTable.NotADirectory && error != table.Loop) return Entry.Failed(error); // Not a folder of its own (a link gives ENOTDIR on Linux, ELOOP or ENOTDIR on macOS): a link has a target to read. @@ -31,9 +29,7 @@ public override Entry Open(SafeHandle folder, string name, bool forRemoval) if (With(folder, dirfd => ReadLinkAt(dirfd, name, ref target, 1)) >= 0) return new Entry(EntryKind.Link, null, 0); error = Marshal.GetLastPInvokeError(); - return error == UnixTable.Invalid ? new Entry(EntryKind.Other, null, 0) - : error == UnixTable.NoEntry ? new Entry(EntryKind.Missing, null, 0) - : Entry.Failed(error); + return error == UnixTable.Invalid ? new Entry(EntryKind.Other, null, 0) : Entry.Failed(error); } public override int Remove(SafeHandle folder, string name, Entry entry, bool classic) => @@ -41,7 +37,7 @@ public override int Remove(SafeHandle folder, string name, Entry entry, bool cla ? 0 : Marshal.GetLastPInvokeError(); - public override Failure Classify(int error) => table.Classify(error); + public override Failure Classify(int error) => UnixTable.Classify(error); // Runs a call with the file descriptor a handle holds, which stays open until the call returns. private static T With(SafeHandle handle, Func call) diff --git a/src/WriteableFileProviders.Physical/WriteablePhysicalFileProvider.cs b/src/WriteableFileProviders.Physical/WriteablePhysicalFileProvider.cs index b9807e5..4149505 100644 --- a/src/WriteableFileProviders.Physical/WriteablePhysicalFileProvider.cs +++ b/src/WriteableFileProviders.Physical/WriteablePhysicalFileProvider.cs @@ -100,10 +100,12 @@ namespace WriteableFileProviders.Physical; /// gone into, and the delete never reaches outside the folder it opened. The folder that holds it is opened by its path, /// links followed, unless is set: then every folder from the root /// down is opened the same way, and a link swapped in on the way is refused. The names come from listing each folder by -/// its path, so a rename or a link swapped in meanwhile can only make the delete miss names, and fail. Like .NET's -/// recursive delete, it goes on past an entry it cannot delete, skips one gone meanwhile and throws the first failure -/// at the end; a folder that still has entries after it (a file another process holds open, on a file system without -/// POSIX deletes) throws . On Windows a folder holding a junction is deleted, the junction as +/// its path, and each is only ever an entry of that folder (. and .., which some file systems list as +/// files, are skipped), so a rename or a link swapped in meanwhile can only make the delete miss names, and fail. Like +/// .NET's recursive delete, it goes on past an entry it cannot delete, skips one gone meanwhile and throws the first +/// failure at the end; a folder that still has entries after it (a file another process holds open, on a file system +/// without POSIX deletes) throws . Cancelled, it stops between entries, the folder partly +/// deleted. On Windows a folder holding a junction is deleted, the junction as /// itself. On macOS the folder that holds the deleted one, and with RefuseLinks every folder on the way, must be /// readable. This is how a folder is deleted on Linux on x64, Arm and Arm64, macOS and Windows; elsewhere it is deleted /// by its path, as .NET does, and a link swapped in while that runs can steer it elsewhere. A file is deleted by its @@ -473,7 +475,8 @@ protected override Task DeleteCoreAsync(ResolvedPath path, CancellationTok } // Through handles, so a link swapped in while the delete runs can never take it elsewhere. var walk = new FolderDelete(folders, Opening, ClassicDeletes); - return Task.FromResult(walk.Delete(path, _root, path.RelativeSegments, _refuseLinks, link => RefusedLink(path, link))); + return Task.FromResult(walk.Delete(path, _root, path.RelativeSegments, _refuseLinks, link => RefusedLink(path, link), + cancellationToken)); } /// Renames the file; the destination is taken atomically, so an existing one is never replaced. diff --git a/tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs b/tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs index 4dedf84..606c86f 100644 --- a/tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs +++ b/tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs @@ -2,6 +2,7 @@ using System.Runtime.InteropServices; using System.Runtime.Versioning; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Microsoft.Win32.SafeHandles; namespace WriteableFileProviders.Physical.Tests; @@ -11,7 +12,7 @@ namespace WriteableFileProviders.Physical.Tests; /// rules each system's answers are read by, and the delete by path where the system is not known. /// [TestClass] -public sealed class PhysicalFolderDeleteTests +public sealed partial class PhysicalFolderDeleteTests { private string _base = ""; private string _root = ""; @@ -208,6 +209,24 @@ public async Task AnEntryGoneBetweenTheListingAndItsOpen_IsSkipped() Assert.IsFalse(Path.Exists(Disk("dir"))); } + [TestMethod] + public async Task ASubfolderGoneBetweenItsOpenAndItsListing_IsSkipped() + { + Write("x", "dir", "sub", "x.txt"); + Write("y", "dir", "y.txt"); + var folders = new ScriptedFolders(FolderSystem.Current!) + { + AfterOpen = name => + { + if (name == "sub") + Directory.Delete(Disk("dir", "sub"), recursive: true); + }, + }; + using var provider = new WriteablePhysicalFileProvider(_root) { Folders = folders }; + Assert.IsTrue(await provider.DeleteAsync("dir")); + Assert.IsFalse(Path.Exists(Disk("dir"))); + } + [TestMethod] public async Task ATargetGoneBeforeItIsOpened_DeletesNothing() { @@ -220,6 +239,36 @@ public async Task ATargetGoneBeforeItIsOpened_DeletesNothing() Assert.IsFalse(await provider.DeleteAsync("dir")); } + [TestMethod] + public async Task ACancelledDelete_StopsBetweenEntries() + { + Write("x", "dir", "a.txt"); + Write("y", "dir", "b.txt"); + using var cancel = new CancellationTokenSource(); + using var provider = Provider(opening: path => + { + if (path.StartsWith(Disk("dir") + Path.DirectorySeparatorChar, StringComparison.Ordinal)) + cancel.Cancel(); + }); + await Assert.ThrowsAsync(() => provider.DeleteAsync("dir", cancel.Token)); + Assert.IsTrue(Directory.Exists(Disk("dir")), "Stopped before the folder itself"); + Assert.AreEqual(1, Directory.GetFiles(Disk("dir")).Length, "One entry deleted, then the delete stopped"); + } + + [TestMethod] + public async Task WithRefuseLinks_ACancelledDelete_StopsOnTheWay() + { + Write("x", "a", "b", "c", "x.txt"); + using var cancel = new CancellationTokenSource(); + using var provider = Provider(refuseLinks: true, opening: path => + { + if (path == Disk("a")) + cancel.Cancel(); + }); + await Assert.ThrowsAsync(() => provider.DeleteAsync("a/b/c", cancel.Token)); + Assert.IsTrue(File.Exists(Disk("a", "b", "c", "x.txt"))); + } + // ----- Links that were there ----- [TestMethod] @@ -342,6 +391,36 @@ public async Task OnLinuxAndMacOS_AFolderThatMayNotBeSearched_IsRefused() } } + [TestMethod] + [OSCondition(OperatingSystems.Windows)] + [SupportedOSPlatform("windows")] + [DataRow(false)] + [DataRow(true)] + public async Task OnWindows_FoldersOnTheWayHeldOpenWithoutDeleteSharing_DoNotStopTheDelete(bool refuseLinks) + { + // As a shell holds its current folder: the folder that holds the target, and the root on the way, are opened + // elsewhere with read and write shared, delete not. Only what the delete removes is opened to be deleted. + Write("x", "a", "b", "x.txt"); + using var parent = HoldFolder(Disk("a")); + using var root = HoldFolder(_root); + using var provider = Provider(refuseLinks); + Assert.IsTrue(await provider.DeleteAsync("a/b")); + Assert.IsFalse(Path.Exists(Disk("a", "b"))); + } + + // A folder held open with read and write shared, delete not. + [SupportedOSPlatform("windows")] + private static SafeFileHandle HoldFolder(string path) + { + const uint listDirectory = 0x1, synchronize = 0x100000, shareReadWrite = 0x3, openExisting = 3, backupSemantics = 0x02000000; + var handle = CreateFile(path, listDirectory | synchronize, shareReadWrite, 0, openExisting, backupSemantics, 0); + Assert.IsFalse(handle.IsInvalid, $"The folder is held ({Marshal.GetLastPInvokeError()})"); + return handle; + } + + [LibraryImport("kernel32.dll", EntryPoint = "CreateFileW", StringMarshalling = StringMarshalling.Utf16, SetLastError = true)] + private static partial SafeFileHandle CreateFile(string name, uint access, uint share, nint security, uint disposition, uint flags, nint template); + // ----- The other ways a folder is deleted ----- [TestMethod] @@ -442,12 +521,44 @@ public async Task AnEntryThatCannotBeOpened_FailsTheDelete_AndTheRestGoes() public async Task AFolderThatCannotBeListed_FailsTheDelete() { Write("x", "dir", "x.txt"); - var folders = new ScriptedFolders(FolderSystem.Current!) { ListsNothing = true }; + var folders = new ScriptedFolders(FolderSystem.Current!) { Listing = _ => throw new UnauthorizedAccessException("The folder may not be listed.") }; using var provider = new WriteablePhysicalFileProvider(_root) { Folders = folders }; - await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); + var e = await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); + Assert.AreEqual("The folder may not be listed.", e.Message); Assert.IsTrue(File.Exists(Disk("dir", "x.txt"))); } + [TestMethod] + public async Task AListingThatNamesTheFolderItselfOrItsParent_NeverLeavesTheFolder() + { + // Some file systems (FUSE, NFS) list '.' and '..' as files, which .NET then reports: opened in the folder, '..' + // would be its parent and '.' the folder again. + Write("x", "dir", "x.txt"); + Write("sibling", "sibling.txt"); + var folders = new ScriptedFolders(FolderSystem.Current!) + { + Listing = path => path == Disk("dir") ? [".", "..", "x.txt"] : [], + }; + using var provider = new WriteablePhysicalFileProvider(_root) { Folders = folders }; + Assert.IsTrue(await provider.DeleteAsync("dir")); + Assert.IsFalse(Path.Exists(Disk("dir"))); + Assert.AreEqual("sibling", File.ReadAllText(Disk("sibling.txt")), "The parent was not emptied"); + } + + [TestMethod] + public async Task AListedNameHoldingASeparator_IsRefused() + { + Write("x", "dir", "sub", "x.txt"); + var folders = new ScriptedFolders(FolderSystem.Current!) + { + Listing = path => path == Disk("dir") ? ["sub/x.txt"] : [], + }; + using var provider = new WriteablePhysicalFileProvider(_root) { Folders = folders }; + var e = await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); + StringAssert.Contains(e.Message, "'sub/x.txt'"); + Assert.IsTrue(File.Exists(Disk("dir", "sub", "x.txt"))); + } + [TestMethod] [DataRow(WindowsRules.PathNotFound, false)] [DataRow(WindowsRules.AccessDenied, true)] @@ -519,13 +630,11 @@ public void UnixTable_ComposesItsFlags() Assert.AreEqual(0x200000 | 0x10000 | 0x80000 | 0x20000, UnixTable.Linux.OpenEntry); Assert.AreEqual(0x200000 | 0x4000 | 0x80000 | 0x8000, UnixTable.LinuxArm.OpenEntry); Assert.AreEqual(0x100000 | 0x1000000 | 0x100, UnixTable.MacOS.OpenEntry); - Assert.AreEqual(Failure.Gone, UnixTable.Linux.Classify(UnixTable.NoEntry)); - Assert.AreEqual(Failure.Gone, UnixTable.Linux.Classify(UnixTable.NotADirectory)); - Assert.AreEqual(Failure.Refused, UnixTable.Linux.Classify(UnixTable.AccessDenied)); - Assert.AreEqual(Failure.Refused, UnixTable.MacOS.Classify(UnixTable.NotPermitted)); - Assert.AreEqual(Failure.NotEmpty, UnixTable.Linux.Classify(39)); - Assert.AreEqual(Failure.NotEmpty, UnixTable.MacOS.Classify(66)); - Assert.AreEqual(Failure.Other, UnixTable.MacOS.Classify(39), "39 is another error on macOS"); + Assert.AreEqual(Failure.Gone, UnixTable.Classify(UnixTable.NoEntry)); + Assert.AreEqual(Failure.Gone, UnixTable.Classify(UnixTable.NotADirectory)); + Assert.AreEqual(Failure.Refused, UnixTable.Classify(UnixTable.AccessDenied)); + Assert.AreEqual(Failure.Refused, UnixTable.Classify(UnixTable.NotPermitted)); + Assert.AreEqual(Failure.Other, UnixTable.Classify(39), "A folder not empty, on Linux"); } [TestMethod] @@ -542,7 +651,7 @@ public void WindowsRules_ReadWhatWindowsReports() Assert.AreEqual(Failure.Gone, WindowsRules.Classify(WindowsRules.FileNotFound)); Assert.AreEqual(Failure.Gone, WindowsRules.Classify(WindowsRules.PathNotFound)); Assert.AreEqual(Failure.Refused, WindowsRules.Classify(WindowsRules.AccessDenied)); - Assert.AreEqual(Failure.NotEmpty, WindowsRules.Classify(WindowsRules.DirectoryNotEmpty)); + Assert.AreEqual(Failure.Other, WindowsRules.Classify(WindowsRules.DirectoryNotEmpty)); Assert.AreEqual(Failure.Other, WindowsRules.Classify(32)); Assert.IsTrue(WindowsRules.FallsBack(WindowsRules.InvalidParameter)); Assert.IsFalse(WindowsRules.FallsBack(WindowsRules.AccessDenied)); @@ -567,7 +676,9 @@ private sealed class ScriptedFolders(FolderSystem system) : FolderSystem public int? OpenPathError { get; init; } - public bool ListsNothing { get; init; } + public Func>? Listing { get; init; } + + public Action? AfterOpen { get; init; } public List<(string Name, bool Classic)> Removed { get; } = []; @@ -581,8 +692,14 @@ private sealed class ScriptedFolders(FolderSystem system) : FolderSystem return system.OpenPath(path, out error); } - public override Entry Open(SafeHandle folder, string name, bool forRemoval) => - OpenErrors.TryGetValue(name, out var error) ? Entry.Failed(error) : system.Open(folder, name, forRemoval); + public override Entry Open(SafeHandle folder, string name, bool forRemoval) + { + if (OpenErrors.TryGetValue(name, out var error)) + return Entry.Failed(error); + var entry = system.Open(folder, name, forRemoval); + AfterOpen?.Invoke(name); + return entry; + } public override int Remove(SafeHandle folder, string name, Entry entry, bool classic) { @@ -594,6 +711,8 @@ public override int Remove(SafeHandle folder, string name, Entry entry, bool cla public override bool FallsBack(int error) => WindowsRules.FallsBack(error); - public override string ListingPath(string path) => ListsNothing ? Path.Combine(path, "nowhere") : system.ListingPath(path); + public override string ListingPath(string path) => system.ListingPath(path); + + public override IEnumerator List(string path) => Listing is { } listing ? listing(path).GetEnumerator() : system.List(path); } } diff --git a/tests/WriteableFileProviders.Physical.Tests/WriteableFileProviders.Physical.Tests.csproj b/tests/WriteableFileProviders.Physical.Tests/WriteableFileProviders.Physical.Tests.csproj index cd2aff5..e6a0b06 100644 --- a/tests/WriteableFileProviders.Physical.Tests/WriteableFileProviders.Physical.Tests.csproj +++ b/tests/WriteableFileProviders.Physical.Tests/WriteableFileProviders.Physical.Tests.csproj @@ -1,5 +1,10 @@ + + + true + + From 8ce8a2debccdab1c5c2e45f0b780ac116bba6603 Mon Sep 17 00:00:00 2001 From: Osvay Soma Date: Tue, 13 Oct 2026 12:38:01 +0200 Subject: [PATCH 4/5] Refuse an empty listed name, and look a refused name up as listed An empty name, opened relative to a folder's handle on Windows, is the folder itself: refused as a name with a separator is. A removal refused for a name another process deleted counts as gone, and that check now looks the name up in the extended form on Windows, which keeps a trailing dot or space, so 'x.' is never taken for 'x'. Also: the changelog's cancellation scoped to where the walk runs, wraps, a dead test override, and the plan's table noting ENOTEMPTY is no longer read. Refs #70 --- CHANGELOG.md | 6 +-- docs/plans/2026-10-13-2027-1-1.md | 5 ++- .../FolderDelete.cs | 10 +++-- src/WriteableFileProviders.Physical/README.md | 6 +-- .../WriteablePhysicalFileProvider.cs | 14 +++---- .../PhysicalFolderDeleteTests.cs | 42 +++++++++++++++---- 6 files changed, 57 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6d5dd7..a6e3f0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,9 +27,9 @@ version's section as its GitHub release notes. opened the same way and a link swapped in there is refused. On Windows a folder holding a junction is now deleted (before, `UnauthorizedAccessException`). A folder that still has entries after the delete (a file another process holds open on a file system without POSIX deletes) throws `IOException`, where it returned true. On macOS the folder - that holds the deleted one, and with `RefuseLinks` every folder on the way, must be readable. The delete can now be - cancelled: it stops between entries, the folder partly deleted. Elsewhere (Linux on other architectures, other - systems) a folder is deleted by its path, as before. + that holds the deleted one, and with `RefuseLinks` every folder on the way, must be readable. On those systems the + delete can now be cancelled: it stops between entries, the folder partly deleted. Elsewhere (Linux on other + architectures, other systems) a folder is deleted by its path, as before. ## 2027.1.0 (2026-10-13) diff --git a/docs/plans/2026-10-13-2027-1-1.md b/docs/plans/2026-10-13-2027-1-1.md index fb46975..eac175b 100644 --- a/docs/plans/2026-10-13-2027-1-1.md +++ b/docs/plans/2026-10-13-2027-1-1.md @@ -139,7 +139,7 @@ Decided while planning. The maintainer's approval of this plan approves them. | `O_CLOEXEC` | 0x80000 | 0x80000 | 0x1000000 | | `AT_REMOVEDIR` | 0x200 | 0x200 | 0x80 | | `ENOENT`, `EACCES`, `EPERM`, `ENOTDIR`, `EINVAL` | 2, 13, 1, 20, 22 | same | same | - | `ELOOP`, `ENOTEMPTY` | 40, 39 | 40, 39 | 62, 66 | + | `ELOOP`, `ENOTEMPTY` (unread since step 3: "anything else") | 40, 39 | 40, 39 | 62, 66 | What each error means to the walk (N5): `ENOENT`, gone (skipped, or false at the start); `EACCES` and `EPERM`, refused (`UnauthorizedAccessException`); `ENOTDIR` and `ELOOP` from an open, not a folder (then `readlinkat`); `EINVAL` from @@ -483,3 +483,6 @@ expectation (the winner is done before the loser retries); the locked restore of Minors: a subfolder gone between its open and its listing is skipped (F4); the walk is cancellable between entries (F5); `Failure.NotEmpty` and the table's `ENOTEMPTY` go, so a folder not empty is "anything else" (F6, a departure from D6's table); one representation of gone (F7). F8, dropping `ClassicDeletes`, was declined: the plan names it. + Round 2: F1 to F7 verified. G1, an empty listed name would reopen the folder itself on Windows, refused as a name with + a separator is; G2, the check for a name gone after a refused removal looks it up in the extended form on Windows, + which keeps a trailing dot or space, with a test; nits. diff --git a/src/WriteableFileProviders.Physical/FolderDelete.cs b/src/WriteableFileProviders.Physical/FolderDelete.cs index 24ff616..d4ee308 100644 --- a/src/WriteableFileProviders.Physical/FolderDelete.cs +++ b/src/WriteableFileProviders.Physical/FolderDelete.cs @@ -278,10 +278,11 @@ private void DeleteContents(SafeHandle folder, string fullPath, string relative, // would be its parent and '.' itself. if (name is "." or "..") continue; - if (name.AsSpan().IndexOfAny(_separators) >= 0) + // Empty, a name would open the folder again (Windows); with a separator, an entry further down. + if (name.Length == 0 || name.AsSpan().IndexOfAny(_separators) >= 0) { first ??= new IOException( - $"'{relative}' could not be emptied: its listing named '{name}', which is not the name of an entry."); + $"'{relative}' could not be emptied: its listing named '{name}', which is no entry's name."); continue; } var childPath = Path.Join(fullPath, name); @@ -342,12 +343,13 @@ private bool Next(string fullPath, [NotNullWhen(true)] ref IEnumerator? // Whether a removal failed because the entry is gone: as the system says, or, where it answers that the access is // refused (Windows, for a folder another process deleted while this one held it), because nothing has the name any - // more. Only the answer depends on it, never what is deleted. + // more, looked up in the form that keeps a name as listed (on Windows, the extended form, which keeps a trailing dot + // or space). Only the answer depends on it, never what is deleted. private bool IsGone(int error, string fullPath) => system.Classify(error) switch { Failure.Gone => true, - Failure.Refused => (int)new FileInfo(fullPath).Attributes == -1, + Failure.Refused => (int)new FileInfo(system.ListingPath(fullPath)).Attributes == -1, _ => false, }; diff --git a/src/WriteableFileProviders.Physical/README.md b/src/WriteableFileProviders.Physical/README.md index d502cff..055889a 100644 --- a/src/WriteableFileProviders.Physical/README.md +++ b/src/WriteableFileProviders.Physical/README.md @@ -73,9 +73,9 @@ None. The provider reads and writes with the permissions of the process. Without it, a recursive listing lists a linked folder but doesn't walk into it. `RefuseLinks` checks the path just before each operation, so a process that can create links inside the root can still swap one in between, except for a folder delete on Linux (x64, Arm, Arm64), macOS and Windows, which opens every folder from the root without - following a link. Hard links and mounts on - Linux and macOS aren't visible as links, and other reparse points (cloud placeholders, Windows container layers) - aren't links. The polling `Watch` can still report a change behind a linked folder. + following a link. Hard links and mounts on Linux and macOS aren't visible as links, and other reparse points (cloud + placeholders, Windows container layers) aren't links. The polling `Watch` can still report a change behind a linked + folder. - **A folder delete never leaves the folder** on Linux (x64, Arm, Arm64), macOS and Windows. It opens each entry in its folder without following a link and removes it there, so a link inside, even one swapped in while it runs, is removed as itself. It goes on past what it can't delete and throws the first failure; cancelled, it stops between diff --git a/src/WriteableFileProviders.Physical/WriteablePhysicalFileProvider.cs b/src/WriteableFileProviders.Physical/WriteablePhysicalFileProvider.cs index 4149505..5aa4406 100644 --- a/src/WriteableFileProviders.Physical/WriteablePhysicalFileProvider.cs +++ b/src/WriteableFileProviders.Physical/WriteablePhysicalFileProvider.cs @@ -105,11 +105,10 @@ namespace WriteableFileProviders.Physical; /// .NET's recursive delete, it goes on past an entry it cannot delete, skips one gone meanwhile and throws the first /// failure at the end; a folder that still has entries after it (a file another process holds open, on a file system /// without POSIX deletes) throws . Cancelled, it stops between entries, the folder partly -/// deleted. On Windows a folder holding a junction is deleted, the junction as -/// itself. On macOS the folder that holds the deleted one, and with RefuseLinks every folder on the way, must be -/// readable. This is how a folder is deleted on Linux on x64, Arm and Arm64, macOS and Windows; elsewhere it is deleted -/// by its path, as .NET does, and a link swapped in while that runs can steer it elsewhere. A file is deleted by its -/// path. +/// deleted. On Windows a folder holding a junction is deleted, the junction as itself. On macOS the folder that holds +/// the deleted one, and with RefuseLinks every folder on the way, must be readable. This is how a folder is deleted on +/// Linux on x64, Arm and Arm64, macOS and Windows; elsewhere it is deleted by its path, as .NET does, and a link swapped +/// in while that runs can steer it elsewhere. A file is deleted by its path. /// /// /// Access. Where the operating system refuses the process access to a file or a folder that a member opens, @@ -475,8 +474,9 @@ protected override Task DeleteCoreAsync(ResolvedPath path, CancellationTok } // Through handles, so a link swapped in while the delete runs can never take it elsewhere. var walk = new FolderDelete(folders, Opening, ClassicDeletes); - return Task.FromResult(walk.Delete(path, _root, path.RelativeSegments, _refuseLinks, link => RefusedLink(path, link), - cancellationToken)); + var deleted = walk.Delete(path, _root, path.RelativeSegments, _refuseLinks, link => RefusedLink(path, link), + cancellationToken); + return Task.FromResult(deleted); } /// Renames the file; the destination is taken atomically, so an existing one is never replaced. diff --git a/tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs b/tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs index 606c86f..b14e712 100644 --- a/tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs +++ b/tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs @@ -537,7 +537,7 @@ public async Task AListingThatNamesTheFolderItselfOrItsParent_NeverLeavesTheFold Write("sibling", "sibling.txt"); var folders = new ScriptedFolders(FolderSystem.Current!) { - Listing = path => path == Disk("dir") ? [".", "..", "x.txt"] : [], + Listing = path => path == Disk("dir") ? [".", "..", "x.txt"] : null, }; using var provider = new WriteablePhysicalFileProvider(_root) { Folders = folders }; Assert.IsTrue(await provider.DeleteAsync("dir")); @@ -546,19 +546,45 @@ public async Task AListingThatNamesTheFolderItselfOrItsParent_NeverLeavesTheFold } [TestMethod] - public async Task AListedNameHoldingASeparator_IsRefused() + [DataRow("sub/x.txt")] + [DataRow("")] + public async Task AListedNameThatIsNoEntrysName_IsRefused(string name) { + // A separator would reach an entry further down; an empty name would open the folder again (Windows). Write("x", "dir", "sub", "x.txt"); var folders = new ScriptedFolders(FolderSystem.Current!) { - Listing = path => path == Disk("dir") ? ["sub/x.txt"] : [], + Listing = path => path == Disk("dir") ? [name] : null, }; using var provider = new WriteablePhysicalFileProvider(_root) { Folders = folders }; var e = await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); - StringAssert.Contains(e.Message, "'sub/x.txt'"); + StringAssert.Contains(e.Message, $"named '{name}'"); Assert.IsTrue(File.Exists(Disk("dir", "sub", "x.txt"))); } + [TestMethod] + [OSCondition(OperatingSystems.Windows)] + public async Task OnWindows_ARefusalForANameWithATrailingDot_IsNotTakenForAnotherName() + { + // 'x.' and 'x' are two names in the extended form, the form the walk lists and removes by; a plain path would + // read 'x.' as 'x', which is not there, and take the refusal for a name gone. + Directory.CreateDirectory(Disk("dir")); + var trailing = WindowsRules.ExtendedPath(Disk("dir", "x.")); + File.WriteAllText(trailing, "kept"); + File.SetAttributes(trailing, FileAttributes.ReadOnly); + try + { + using var provider = Provider(); + var e = await Assert.ThrowsExactlyAsync(() => provider.DeleteAsync("dir")); + StringAssert.Contains(e.Message, "'dir/x.'"); + } + finally + { + File.SetAttributes(trailing, FileAttributes.Normal); + File.Delete(trailing); + } + } + [TestMethod] [DataRow(WindowsRules.PathNotFound, false)] [DataRow(WindowsRules.AccessDenied, true)] @@ -676,7 +702,8 @@ private sealed class ScriptedFolders(FolderSystem system) : FolderSystem public int? OpenPathError { get; init; } - public Func>? Listing { get; init; } + // The names to list in a folder, by its path; null for the folder's real listing. + public Func?>? Listing { get; init; } public Action? AfterOpen { get; init; } @@ -711,8 +738,7 @@ public override int Remove(SafeHandle folder, string name, Entry entry, bool cla public override bool FallsBack(int error) => WindowsRules.FallsBack(error); - public override string ListingPath(string path) => system.ListingPath(path); - - public override IEnumerator List(string path) => Listing is { } listing ? listing(path).GetEnumerator() : system.List(path); + public override IEnumerator List(string path) => + Listing?.Invoke(path) is { } names ? names.GetEnumerator() : system.List(path); } } From 372a2878099a824dba891bd1ee280407d248e359 Mon Sep 17 00:00:00 2001 From: Osvay Soma Date: Tue, 13 Oct 2026 12:44:11 +0200 Subject: [PATCH 5/5] Pass the scripted system's lookups through to the real one Refs #70 --- docs/plans/2026-10-13-2027-1-1.md | 2 +- .../PhysicalFolderDeleteTests.cs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/plans/2026-10-13-2027-1-1.md b/docs/plans/2026-10-13-2027-1-1.md index eac175b..e621f80 100644 --- a/docs/plans/2026-10-13-2027-1-1.md +++ b/docs/plans/2026-10-13-2027-1-1.md @@ -485,4 +485,4 @@ expectation (the winner is done before the loser retries); the locked restore of from D6's table); one representation of gone (F7). F8, dropping `ClassicDeletes`, was declined: the plan names it. Round 2: F1 to F7 verified. G1, an empty listed name would reopen the folder itself on Windows, refused as a name with a separator is; G2, the check for a name gone after a refused removal looks it up in the extended form on Windows, - which keeps a trailing dot or space, with a test; nits. + which keeps a trailing dot or space, with a test; nits. Round 3: G1 to G3 verified, clean; one test nit applied. diff --git a/tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs b/tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs index b14e712..ccaa1f4 100644 --- a/tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs +++ b/tests/WriteableFileProviders.Physical.Tests/PhysicalFolderDeleteTests.cs @@ -738,6 +738,8 @@ public override int Remove(SafeHandle folder, string name, Entry entry, bool cla public override bool FallsBack(int error) => WindowsRules.FallsBack(error); + public override string ListingPath(string path) => system.ListingPath(path); + public override IEnumerator List(string path) => Listing?.Invoke(path) is { } names ? names.GetEnumerator() : system.List(path); }