fix: address review feedback on filename sanitizer
- 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.
This commit is contained in:
parent
1098bac18f
commit
582bbd7a78
3 changed files with 104 additions and 49 deletions
|
|
@ -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 = "MovieName.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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
{
|
||||
|
|
|
|||
|
|
@ -25,28 +25,28 @@ public static partial class FilenameSanitizer
|
|||
/// <summary>
|
||||
/// Sanitizes a filename if enabled in settings; otherwise returns it unchanged.
|
||||
/// </summary>
|
||||
public static String SanitizeFilenameIfEnabled(String filename)
|
||||
public static String SanitizeFilenameIfEnabled(String? filename)
|
||||
{
|
||||
return IsEnabled ? SanitizeFilename(filename) : filename;
|
||||
return IsEnabled ? SanitizeFilename(filename) : filename ?? String.Empty;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Sanitizes a full path if enabled in settings; otherwise returns it unchanged.
|
||||
/// </summary>
|
||||
public static String SanitizePathIfEnabled(String filePath)
|
||||
public static String SanitizePathIfEnabled(String? filePath)
|
||||
{
|
||||
return IsEnabled ? SanitizePath(filePath) : filePath;
|
||||
return IsEnabled ? SanitizePath(filePath) : filePath ?? String.Empty;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Sanitizes a filename by stripping problematic characters and normalizing whitespace.
|
||||
/// Does NOT touch directory separators — only a single filename segment.
|
||||
/// </summary>
|
||||
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
|
|||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue