From 582bbd7a78743f7fff45e7fd512573520ff5ec71 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 23 Apr 2026 06:55:34 +0000 Subject: [PATCH] fix: address review feedback on filename sanitizer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Revert GetFileName to return unsanitized name. AllDebridDebridClient. GetSymlinkPath matches torrent.Files[].Path (original debrid data) against GetFileName, so sanitizing it broke AllDebrid Symlink lookups for any torrent with bracketed filenames. Sanitization now happens inside GetDownloadPath at the point the filesystem path is assembled, while matching still uses the unsanitized name. - Make SanitizeFilename / SanitizePath accept String? and return String.Empty for null/empty input so the declared non-null return type is honoured. - SanitizePath splits on both '/' and '\\' regardless of host OS, so paths arriving from the debrid provider with foreign separators are still segmented correctly on Linux containers. - Rewrite the control-character tests to use explicit \u escape sequences (, , , €, Ÿ) so the test inputs are visible in diffs and survive editor/CI round-trips. Replace the single-row empty [Theory] with a [Fact] and add a null case. Add tests asserting SanitizeFilenameIfEnabled and SanitizePathIfEnabled return input unchanged when the toggle is disabled, and sanitize when enabled. --- .../Helpers/FilenameSanitizerTest.cs | 120 +++++++++++++----- .../Helpers/DownloadHelper.cs | 9 +- .../Helpers/FilenameSanitizer.cs | 24 ++-- 3 files changed, 104 insertions(+), 49 deletions(-) diff --git a/server/RdtClient.Service.Test/Helpers/FilenameSanitizerTest.cs b/server/RdtClient.Service.Test/Helpers/FilenameSanitizerTest.cs index c48e46b..b558c64 100644 --- a/server/RdtClient.Service.Test/Helpers/FilenameSanitizerTest.cs +++ b/server/RdtClient.Service.Test/Helpers/FilenameSanitizerTest.cs @@ -1,4 +1,5 @@ using RdtClient.Service.Helpers; +using RdtClient.Service.Services; namespace RdtClient.Service.Test.Helpers; @@ -43,28 +44,18 @@ public class FilenameSanitizerTest Assert.Equal(expected, result); } - [Fact] - public void SanitizeFilename_StripsControlCharacters() + [Theory] + // Uses explicit \u escapes so the control characters survive editors, + // diffs, and CI log copy-paste. Each case verifies a different range + // of Unicode control characters. + [InlineData("Movie\u0001Name\u001F.mkv", "MovieName.mkv")] // C0 range (0x00-0x1F) + [InlineData("Movie\tName.mkv", "MovieName.mkv")] // tab + [InlineData("Movie\u007FName.mkv", "MovieName.mkv")] // DEL (0x7F) + [InlineData("Movie\u0080Name\u009F.mkv", "MovieName.mkv")] // C1 range (0x80-0x9F) + public void SanitizeFilename_StripsControlCharacters(String input, String expected) { - var input = "MovieName.mkv"; var result = FilenameSanitizer.SanitizeFilename(input); - Assert.Equal("MovieName.mkv", result); - } - - [Fact] - public void SanitizeFilename_StripsDeleteCharacter() - { - var input = "MovieName.mkv"; - var result = FilenameSanitizer.SanitizeFilename(input); - Assert.Equal("MovieName.mkv", result); - } - - [Fact] - public void SanitizeFilename_StripsExtendedControlCharacters() - { - var input = "Movie€NameŸ.mkv"; - var result = FilenameSanitizer.SanitizeFilename(input); - Assert.Equal("MovieName.mkv", result); + Assert.Equal(expected, result); } [Fact] @@ -83,12 +74,18 @@ public class FilenameSanitizerTest Assert.Equal(input, result); } - [Theory] - [InlineData("", "")] - public void SanitizeFilename_HandlesEmpty(String input, String expected) + [Fact] + public void SanitizeFilename_ReturnsEmptyForEmpty() { - var result = FilenameSanitizer.SanitizeFilename(input); - Assert.Equal(expected, result); + var result = FilenameSanitizer.SanitizeFilename(""); + Assert.Equal("", result); + } + + [Fact] + public void SanitizeFilename_ReturnsEmptyForNull() + { + var result = FilenameSanitizer.SanitizeFilename(null); + Assert.Equal("", result); } [Fact] @@ -137,11 +134,74 @@ public class FilenameSanitizerTest Assert.Equal(path, result); } - [Theory] - [InlineData("")] - public void SanitizePath_HandlesEmpty(String input) + [Fact] + public void SanitizePath_HandlesMixedSeparators() { - var result = FilenameSanitizer.SanitizePath(input); - Assert.Equal(input, result); + var result = FilenameSanitizer.SanitizePath("data/downloads\\Some Show [2024]/Episode[eztv].mkv"); + var expected = String.Join(Path.DirectorySeparatorChar, "data", "downloads", "Some Show 2024", "Episodeeztv.mkv"); + + Assert.Equal(expected, result); + } + + [Fact] + public void SanitizePath_ReturnsEmptyForEmpty() + { + Assert.Equal("", FilenameSanitizer.SanitizePath("")); + } + + [Fact] + public void SanitizePath_ReturnsEmptyForNull() + { + Assert.Equal("", FilenameSanitizer.SanitizePath(null)); + } + + [Fact] + public void SanitizeFilenameIfEnabled_ReturnsUnchangedWhenDisabled() + { + var original = Settings.Get.DownloadClient.SanitizeFilenames; + try + { + Settings.Get.DownloadClient.SanitizeFilenames = false; + + const String input = "Movie [eztv].mkv"; + Assert.Equal(input, FilenameSanitizer.SanitizeFilenameIfEnabled(input)); + } + finally + { + Settings.Get.DownloadClient.SanitizeFilenames = original; + } + } + + [Fact] + public void SanitizeFilenameIfEnabled_SanitizesWhenEnabled() + { + var original = Settings.Get.DownloadClient.SanitizeFilenames; + try + { + Settings.Get.DownloadClient.SanitizeFilenames = true; + + Assert.Equal("Movie eztv.mkv", FilenameSanitizer.SanitizeFilenameIfEnabled("Movie [eztv].mkv")); + } + finally + { + Settings.Get.DownloadClient.SanitizeFilenames = original; + } + } + + [Fact] + public void SanitizePathIfEnabled_ReturnsUnchangedWhenDisabled() + { + var original = Settings.Get.DownloadClient.SanitizeFilenames; + try + { + Settings.Get.DownloadClient.SanitizeFilenames = false; + + var input = Path.Combine(Path.DirectorySeparatorChar + "data", "downloads", "Show [2024]", "Episode[eztv].mkv"); + Assert.Equal(input, FilenameSanitizer.SanitizePathIfEnabled(input)); + } + finally + { + Settings.Get.DownloadClient.SanitizeFilenames = original; + } } } diff --git a/server/RdtClient.Service/Helpers/DownloadHelper.cs b/server/RdtClient.Service/Helpers/DownloadHelper.cs index c88b1b2..982ad87 100644 --- a/server/RdtClient.Service/Helpers/DownloadHelper.cs +++ b/server/RdtClient.Service/Helpers/DownloadHelper.cs @@ -20,7 +20,7 @@ public static class DownloadHelper var torrentPath = Path.Combine(downloadPath, directory); - var originalFileName = GetOriginalFileName(download); + var originalFileName = GetFileName(download); if (originalFileName == null) { @@ -112,13 +112,6 @@ public static class DownloadHelper } public static String? GetFileName(Download download) - { - var cleaned = GetOriginalFileName(download); - - return cleaned == null ? null : FilenameSanitizer.SanitizeFilenameIfEnabled(cleaned); - } - - private static String? GetOriginalFileName(Download download) { if (String.IsNullOrWhiteSpace(download.Link)) { diff --git a/server/RdtClient.Service/Helpers/FilenameSanitizer.cs b/server/RdtClient.Service/Helpers/FilenameSanitizer.cs index d3a28ad..cde15d3 100644 --- a/server/RdtClient.Service/Helpers/FilenameSanitizer.cs +++ b/server/RdtClient.Service/Helpers/FilenameSanitizer.cs @@ -25,28 +25,28 @@ public static partial class FilenameSanitizer /// /// Sanitizes a filename if enabled in settings; otherwise returns it unchanged. /// - public static String SanitizeFilenameIfEnabled(String filename) + public static String SanitizeFilenameIfEnabled(String? filename) { - return IsEnabled ? SanitizeFilename(filename) : filename; + return IsEnabled ? SanitizeFilename(filename) : filename ?? String.Empty; } /// /// Sanitizes a full path if enabled in settings; otherwise returns it unchanged. /// - public static String SanitizePathIfEnabled(String filePath) + public static String SanitizePathIfEnabled(String? filePath) { - return IsEnabled ? SanitizePath(filePath) : filePath; + return IsEnabled ? SanitizePath(filePath) : filePath ?? String.Empty; } /// /// Sanitizes a filename by stripping problematic characters and normalizing whitespace. /// Does NOT touch directory separators — only a single filename segment. /// - public static String SanitizeFilename(String filename) + public static String SanitizeFilename(String? filename) { if (String.IsNullOrEmpty(filename)) { - return filename; + return String.Empty; } var sb = new StringBuilder(filename.Length); @@ -75,16 +75,18 @@ public static partial class FilenameSanitizer /// /// Sanitizes each segment of a full file path, preserving directory separators. + /// Splits on both '/' and '\' regardless of host OS so paths that arrive + /// from the debrid provider with foreign separators are still handled; + /// the result is re-joined with the host's native separator. /// - public static String SanitizePath(String filePath) + public static String SanitizePath(String? filePath) { if (String.IsNullOrEmpty(filePath)) { - return filePath; + return String.Empty; } - var separator = Path.DirectorySeparatorChar; - var segments = filePath.Split(separator); + var segments = filePath.Split('/', '\\'); for (var i = 0; i < segments.Length; i++) { @@ -94,6 +96,6 @@ public static partial class FilenameSanitizer } } - return String.Join(separator, segments); + return String.Join(Path.DirectorySeparatorChar, segments); } }