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; + } + } }