From d55d9b488150d86c595ce6bd6eb64853e97155f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 00:24:06 +0000 Subject: [PATCH] Skip paths the path type rejects instead of aborting the scan [patch] ktsu.Semantics.Paths rejects '<', '>', '|' and paths over 256 characters, all of which Unix file systems accept. One such file anywhere in the tree threw out of FileScanner.ScanForFiles and took every verb down with it. The scan now names and skips the file, as the hasher does for a file it cannot read, and a root that cannot be represented fails in one line with exit code 1. Fixes ktsu-dev/FileDeduplicator#140 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01EMKbHPuiquQwvUr7NBWPvL --- .../FileScannerAndHasherTests.cs | 65 +++++++++++++++++++ FileDeduplicator.Test/VerbOutputTests.cs | 59 +++++++++++++++++ FileDeduplicator/FileScanner.cs | 13 +++- FileDeduplicator/Verbs/BaseVerb.cs | 26 +++++++- 4 files changed, 161 insertions(+), 2 deletions(-) diff --git a/FileDeduplicator.Test/FileScannerAndHasherTests.cs b/FileDeduplicator.Test/FileScannerAndHasherTests.cs index e059b5e..44c8daa 100644 --- a/FileDeduplicator.Test/FileScannerAndHasherTests.cs +++ b/FileDeduplicator.Test/FileScannerAndHasherTests.cs @@ -323,6 +323,71 @@ public void HashingSkipsAnUnreadableFileAndStillHashesTheRest() Assert.IsFalse(hashes.ContainsKey(unreadable)); } + /// + /// A file name the path type rejects but the file system accepts must be skipped, not abort the + /// scan of the whole tree (ktsu-dev/FileDeduplicator#140). + /// + [TestMethod] + public void ScanSkipsAFileWhoseNameThePathTypeRejects() + { + // Arrange + using TempTree tree = new(); + AbsoluteFilePath a = tree.Write("a.txt", "x"); + AbsoluteFilePath b = tree.Write("b.txt", "x"); + WriteUnrepresentable(tree, "notes .txt"); + WriteUnrepresentable(tree, "left|right.txt"); + + // Act + IReadOnlyList files = FileScanner.ScanForFiles(tree.Root); + + // Assert + CollectionAssert.AreEquivalent(new[] { a, b }, files.ToArray()); + } + + /// + /// A path longer than the path type accepts must be skipped, not abort the scan of the whole + /// tree. Deep node_modules and build output trees routinely have such paths. + /// + [TestMethod] + public void ScanSkipsAFileWhosePathIsTooLongForThePathType() + { + // Arrange + using TempTree tree = new(); + AbsoluteFilePath a = tree.Write("a.txt", "x"); + AbsoluteFilePath b = tree.Write("b.txt", "x"); + string deep = string.Join('/', Enumerable.Repeat(new string('d', 60), 5)) + "/long.txt"; + WriteUnrepresentable(tree, deep); + + // Act + IReadOnlyList files = FileScanner.ScanForFiles(tree.Root); + + // Assert + CollectionAssert.AreEquivalent(new[] { a, b }, files.ToArray()); + } + + /// + /// Writes a file whose path rejects, which + /// cannot do because it hands back that type. Marks the test inconclusive where the file system + /// refuses the path too, as Windows does for < and |. + /// + /// The tree to write into. + /// Path relative to the tree's root, using forward slashes. + internal static void WriteUnrepresentable(TempTree tree, string relativePath) + { + string full = Path.Combine(tree.Root.WeakString, relativePath.Replace('/', Path.DirectorySeparatorChar)); + try + { + _ = Directory.CreateDirectory(Path.GetDirectoryName(full)!); + File.WriteAllText(full, "unrepresentable"); + } + catch (Exception ex) when (ex is IOException or ArgumentException or UnauthorizedAccessException) + { + Assert.Inconclusive($"This file system does not accept {full}, so there is nothing to stage: {ex.Message}"); + } + + Assert.ThrowsExactly(full.As, $"{full} is representable, so it does not stage the case under test."); + } + /// /// Creates a named pipe with mkfifo, or marks the test inconclusive where there is none. /// diff --git a/FileDeduplicator.Test/VerbOutputTests.cs b/FileDeduplicator.Test/VerbOutputTests.cs index c2f1d05..6ba4cd5 100644 --- a/FileDeduplicator.Test/VerbOutputTests.cs +++ b/FileDeduplicator.Test/VerbOutputTests.cs @@ -282,4 +282,63 @@ public void AnEmptyDirectoryStopsAfterDiscovery() Assert.Contains("Found 0 file(s).", output); Assert.DoesNotContain("Hashing files...", output); } + + /// + /// A file the path type cannot represent is named and skipped, and every verb still reports the + /// duplicates around it instead of crashing (ktsu-dev/FileDeduplicator#140). + /// + [TestMethod] + public void EveryVerbSkipsAnUnrepresentableFileAndReportsTheRest() + { + // Arrange + using TempTree tree = new(); + _ = tree.Write("a.txt", "alpha"); + AbsoluteFilePath copy = tree.Write("aa.txt", "alpha"); + FileScannerAndHasherTests.WriteUnrepresentable(tree, "notes .txt"); + string path = tree.Root.WeakString; + + // Act + (string Output, int ExitCode)[] runs = + [ + RunWithExitCode(new Scan { PathString = path }), + RunWithExitCode(new DryRun { PathString = path }), + RunWithExitCode(new Stats { PathString = path }), + RunWithExitCode(new Deduplicate { PathString = path }, "y"), + ]; + + // Assert + foreach ((string output, int exitCode) in runs) + { + Assert.AreEqual(0, exitCode, output); + Assert.Contains("notes .txt", output, "The skipped file was not named."); + Assert.Contains("Duplicate", output, StringComparison.OrdinalIgnoreCase); + } + + Assert.IsFalse(TempTree.Exists(copy), "Deduplicate did not remove the duplicate beside the skipped file."); + } + + /// + /// A root the path type cannot represent fails in one line with exit code 1, not a stack trace. + /// + [TestMethod] + public void ScanOfAnUnrepresentableRootReportsItInOneLine() + { + // Arrange + using TempTree tree = new(); + FileScannerAndHasherTests.WriteUnrepresentable(tree, "left|right/f.txt"); + string root = Path.Combine(tree.Root.WeakString, "left|right"); + + // Act + (string output, int exitCode) = RunWithExitCode(new Scan { PathString = root }); + + // Assert + Assert.AreEqual(1, exitCode, output); + Assert.Contains($"Cannot scan {root}", output); + } + + private static (string Output, int ExitCode) RunWithExitCode(BaseVerb verb, string stdin = "") + { + string output = ConsoleCapture.Run(verb, stdin, out int exitCode); + return (output, exitCode); + } } diff --git a/FileDeduplicator/FileScanner.cs b/FileDeduplicator/FileScanner.cs index b6aea0a..ab49978 100644 --- a/FileDeduplicator/FileScanner.cs +++ b/FileDeduplicator/FileScanner.cs @@ -85,7 +85,18 @@ internal static IReadOnlyList ScanForFiles(AbsoluteDirectoryPa continue; } - files.Add(file.As()); + // ktsu.Semantics.Paths rejects some names the file system accepts: '<', '>' and '|' are + // legal on Unix, and paths over 256 characters are routine under node_modules or deep + // build output. One such file must not abort the scan of the whole tree, so it is + // reported and skipped, the same way FileHasher handles a file it cannot read. + try + { + files.Add(file.As()); + } + catch (ArgumentException ex) + { + Console.WriteLine($" Skipped {file}: {ex.Message}"); + } } return files; diff --git a/FileDeduplicator/Verbs/BaseVerb.cs b/FileDeduplicator/Verbs/BaseVerb.cs index 4f63d8b..56baa30 100644 --- a/FileDeduplicator/Verbs/BaseVerb.cs +++ b/FileDeduplicator/Verbs/BaseVerb.cs @@ -59,7 +59,7 @@ public override int Run() try { ExitCode = 0; - if (!ValidateArgs() || (ScansADirectory && !FileScanner.IsScannableDirectory(Path))) + if (!ValidateArgs() || (ScansADirectory && (!IsRepresentablePath() || !FileScanner.IsScannableDirectory(Path)))) { return 1; } @@ -76,4 +76,28 @@ public override int Run() } internal abstract void Run(T options); + + /// + /// Checks that the root given by converts to an + /// , and says why in one line when it does not. + /// + /// + /// The conversion rejects characters and lengths the file system accepts, such as | in a + /// Unix directory name or a path over 256 characters, and the exception would otherwise escape + /// as a stack trace. + /// + /// when the root cannot be represented. + private bool IsRepresentablePath() + { + try + { + _ = Path; + return true; + } + catch (ArgumentException ex) + { + Console.WriteLine($"Cannot scan {PathString}: {ex.Message}"); + return false; + } + } }