From 2e7448c9bcc17b722ea26a209fb5b2ab86a0554c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C5=93ur?= <coeur@gmx.fr>
Date: Fri, 3 Nov 2023 06:25:42 +0100
Subject: [PATCH] fix: appendSanitizedComponent is too aggressive on non-WIN32
 (and not enough aggressive on WIN32) (#6187)

---
 libtransmission/torrent-files.cc            | 51 +++++++++++++++++----
 tests/libtransmission/torrent-files-test.cc | 41 +++++++++--------
 2 files changed, 62 insertions(+), 30 deletions(-)

diff --git a/libtransmission/torrent-files.cc b/libtransmission/torrent-files.cc
index 5da760ce4..711698c48 100644
--- a/libtransmission/torrent-files.cc
+++ b/libtransmission/torrent-files.cc
@@ -307,18 +307,25 @@ void tr_torrent_files::remove(std::string_view parent_in, std::string_view tmpdi
 namespace
 {
 
+// `isUnixReservedFile` and `isWin32ReservedFile` kept as `maybe_unused`
+// for potential support of different filesystems on the same OS
+[[nodiscard, maybe_unused]] bool isUnixReservedFile(std::string_view in) noexcept
+{
+    static auto constexpr ReservedNames = std::array<std::string_view, 2>{
+        "."sv,
+        ".."sv,
+    };
+    return (std::find(std::begin(ReservedNames), std::end(ReservedNames), in) != std::end(ReservedNames));
+}
+
 // https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file
 // Do not use the following reserved names for the name of a file:
 // CON, PRN, AUX, NUL, COM1, COM2, COM3, COM4, COM5, COM6, COM7, COM8,
 // COM9, LPT1, LPT2, LPT3, LPT4, LPT5, LPT6, LPT7, LPT8, and LPT9.
 // Also avoid these names followed immediately by an extension;
 // for example, NUL.txt is not recommended.
-[[nodiscard]] bool isReservedFile([[maybe_unused]] std::string_view in) noexcept
+[[nodiscard, maybe_unused]] bool isWin32ReservedFile(std::string_view in) noexcept
 {
-#ifndef _WIN32
-    // Of course, on Unix-like platforms none of this applies.
-    return false;
-#else
     if (std::empty(in))
     {
         return false;
@@ -355,18 +362,30 @@ namespace
         std::begin(ReservedPrefixes),
         std::end(ReservedPrefixes),
         [in_upper_sv](auto const& prefix) { return tr_strv_starts_with(in_upper_sv, prefix); });
+}
+
+[[nodiscard]] bool isReservedFile(std::string_view in) noexcept
+{
+#ifdef _WIN32
+    return isWin32ReservedFile(in);
+#else
+    return isUnixReservedFile(in);
 #endif
 }
 
+// `isUnixReservedChar` and `isWin32ReservedChar` kept as `maybe_unused`
+// for potential support of different filesystems on the same OS
+[[nodiscard, maybe_unused]] auto constexpr isUnixReservedChar(unsigned char ch) noexcept
+{
+    return ch == '/';
+}
+
 // https://docs.microsoft.com/en-us/windows/desktop/FileIO/naming-a-file
 // Use any character in the current code page for a name, including Unicode
 // characters and characters in the extended character set (128–255),
 // except for the following:
-[[nodiscard]] auto constexpr isReservedChar(char ch) noexcept
+[[nodiscard, maybe_unused]] auto constexpr isWin32ReservedChar(unsigned char ch) noexcept
 {
-#if !defined(_WIN32)
-    return ch == '/';
-#else
     switch (ch)
     {
     case '"':
@@ -380,13 +399,23 @@ namespace
     case '|':
         return true;
     default:
-        return false;
+        return ch <= 31;
     }
+}
+
+[[nodiscard]] auto constexpr isReservedChar(unsigned char ch) noexcept
+{
+#ifdef _WIN32
+    return isWin32ReservedChar(ch);
+#else
+    return isUnixReservedChar(ch);
 #endif
 }
 
+// https://en.wikipedia.org/wiki/Filename#Comparison_of_filename_limitations
 void appendSanitizedComponent(std::string_view in, tr_pathbuf& out)
 {
+#ifdef _WIN32
     // remove leading and trailing spaces
     in = tr_strv_strip(in);
 
@@ -395,7 +424,9 @@ void appendSanitizedComponent(std::string_view in, tr_pathbuf& out)
     {
         in.remove_suffix(1);
     }
+#endif
 
+    // replace reserved filenames with an underscore
     if (isReservedFile(in))
     {
         out.append('_');
diff --git a/tests/libtransmission/torrent-files-test.cc b/tests/libtransmission/torrent-files-test.cc
index 0f8073004..95e449724 100644
--- a/tests/libtransmission/torrent-files-test.cc
+++ b/tests/libtransmission/torrent-files-test.cc
@@ -143,30 +143,31 @@ TEST_F(TorrentFilesTest, hasAnyLocalData)
 
 TEST_F(TorrentFilesTest, isSubpathPortable)
 {
-    static auto constexpr Tests = std::array<std::pair<std::string_view, bool>, 15>{ {
+    static auto constexpr NotWin32 = TR_IF_WIN32(false, true);
+
+    static auto constexpr Tests = std::array<std::pair<std::string_view, bool>, 18>{ {
+        // never portable
+        { ".", false },
+        { "..", false },
+
         // don't end with periods
-        { "foo.", false },
-        { "foo..", false },
+        { "foo.", NotWin32 },
+        { "foo..", NotWin32 },
 
         // don't begin or end with whitespace
-        { " foo ", false },
-        { " foo", false },
-        { "foo ", false },
+        { " foo ", NotWin32 },
+        { " foo", NotWin32 },
+        { "foo ", NotWin32 },
 
-    // reserved names and characters (platform-dependent)
-#ifdef _WIN32
-        { "COM1", false },
-        { "COM1.txt", false },
-        { "Com1", false },
-        { "com1", false },
-        { "hell:o.txt", false },
-#else
-        { "COM1", true },
-        { "COM1.txt", true },
-        { "Com1", true },
-        { "com1", true },
-        { "hell:o.txt", true },
-#endif
+        // reserved names
+        { "COM1", NotWin32 },
+        { "COM1.txt", NotWin32 },
+        { "Com1", NotWin32 },
+        { "com1", NotWin32 },
+
+        // reserved characters
+        { "hell:o.txt", NotWin32 },
+        { "hell\to.txt", NotWin32 },
 
         // everything else
         { ".foo", true },