From e12d92d04928cdd4e5929ad6879f85e296bd332a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:32:04 +0000 Subject: [PATCH] fix: skip a file the hashing pass cannot read instead of aborting [patch] FileHasher.HashFiles caught only IOException, and UnauthorizedAccessException does not derive from it. Hashing runs under Parallel.ForEach, so a single permission-denied file -- an OS-protected file, or one owned by another user -- escaped the delegate and surfaced as an AggregateException out of HashFiles, uncaught by the calling verb or Program.Main. That killed the whole hashing phase, discarding the hashes every other thread had already produced. Catch UnauthorizedAccessException alongside IOException, matching Deduplicator.StillMatchesGroup, which already catches both for the equivalent re-hash before deletion. The shared report path moves into a helper so the two branches cannot drift. The test arranges the denial with a directory standing in for a file, which File.OpenRead rejects with UnauthorizedAccessException on every platform for every user. Denying permission on a real file proves nothing under a privileged process, which reads it regardless. Fixes #126 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NPXBwr1rm8hybVmmWhx7wd --- .gitignore | 18 +++++++++++ .../FileScannerAndHasherTests.cs | 32 +++++++++++++++++++ FileDeduplicator/FileHasher.cs | 21 +++++++++--- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index dc0470a..e043c9f 100644 --- a/.gitignore +++ b/.gitignore @@ -203,6 +203,11 @@ PublishScripts/ **/[Pp]ackages/* # except build/, which is used as an MSBuild target. !**/[Pp]ackages/build/ +# and except a Unity project's Packages/, which is source: Unity's package manifest and its +# resolved lock file are both meant to be committed, and a NuGet restore folder never contains +# a file by either name. +!**/[Pp]ackages/manifest.json +!**/[Pp]ackages/packages-lock.json # Uncomment if necessary however generally it will be regenerated when needed #!**/[Pp]ackages/repositories.config # NuGet v3's project.json files produces more ignorable files @@ -651,3 +656,16 @@ Temporary Items # ImGui.ini files imgui.ini + +# Game engine projects +# +# Godot: the import cache, and the mono/temp bin+obj a C# build writes. +.godot/ + +# Unity: .meta files are source, not the Visual Studio C++ build artifact that the `*.meta` rule +# further up targets. Unity generates one per asset and it carries the GUID that scenes, prefabs +# and serialized references point at, so ignoring them gives every clone fresh GUIDs and silently +# breaks those references - including for a plug-in whose .dll is itself a build output. This +# negation has to come after that rule to win, and is scoped to the asset tree so the Visual +# Studio artifact stays ignored everywhere else. +!**/[Aa]ssets/**/*.meta diff --git a/FileDeduplicator.Test/FileScannerAndHasherTests.cs b/FileDeduplicator.Test/FileScannerAndHasherTests.cs index 9f65b9a..0ffad50 100644 --- a/FileDeduplicator.Test/FileScannerAndHasherTests.cs +++ b/FileDeduplicator.Test/FileScannerAndHasherTests.cs @@ -164,4 +164,36 @@ public void ParallelHashingAgreesWithSingleFileHashing() Assert.AreEqual(FileHasher.ComputeHash(file), hashes[file]); } } + + /// + /// One file the process cannot read must not take the whole hashing pass down with it. Hashing + /// runs under Parallel.ForEach, so an exception escaping the delegate surfaces as an + /// and discards the results every other thread had already + /// produced. + /// + [TestMethod] + public void HashingSkipsAnUnreadableFileAndStillHashesTheRest() + { + // Arrange + using TempTree tree = new(); + AbsoluteFilePath readable = tree.Write("readable.txt", "one"); + AbsoluteFilePath alsoReadable = tree.Write("also-readable.txt", "two"); + + // A directory standing in for a file: File.OpenRead throws UnauthorizedAccessException for + // one on every platform and for every user. Denying permission on a real file would not -- + // a privileged process, root or an elevated CI runner, reads it anyway -- so that + // arrangement would pass whether or not the exception is handled. + string deniedPath = Path.Combine(tree.Root.WeakString, "denied"); + _ = Directory.CreateDirectory(deniedPath); + AbsoluteFilePath unreadable = deniedPath.As(); + + // Act + Dictionary hashes = FileHasher.HashFiles([readable, alsoReadable, unreadable]); + + // Assert + Assert.HasCount(2, hashes); + Assert.AreEqual(FileHasher.ComputeHash(readable), hashes[readable]); + Assert.AreEqual(FileHasher.ComputeHash(alsoReadable), hashes[alsoReadable]); + Assert.IsFalse(hashes.ContainsKey(unreadable)); + } } diff --git a/FileDeduplicator/FileHasher.cs b/FileDeduplicator/FileHasher.cs index 4a6c00c..bfb5ff0 100644 --- a/FileDeduplicator/FileHasher.cs +++ b/FileDeduplicator/FileHasher.cs @@ -32,16 +32,29 @@ internal static Dictionary HashFiles(IReadOnlyList(results); } + private static void ReportSkipped(AbsoluteFilePath filePath, Exception ex) + { + lock (ConsoleLock) + { + Console.WriteLine($" Error hashing {filePath.FileName}: {ex.Message}"); + } + } + internal static string ComputeHash(AbsoluteFilePath filePath) { using FileStream stream = File.OpenRead(filePath.WeakString);