Merge pull request #492 from alanmcgovern/invalid-chars-in-paths-mono…

…torrent-2.0

[core] Replace invalid characters in the generated path

src/MonoTorrent.Tests/Client/ClientEngineTests.cs

@@ -167,6 +167,38 @@ public void CacheDirectory_IsFile_UpdateSettings ()
             Assert.ThrowsAsync<ArgumentException> (() => engine.UpdateSettingsAsync (new EngineSettingsBuilder { CacheDirectory = cachePath }.ToSettings ()));
         }
 
+        [Test]
+        public async Task ContainingDirectory_InvalidCharacters ()
+        {
+            // You can't manually add peers to private torrents
+            using var rig = TestRig.CreateMultiFile (new TestWriter ());
+            await rig.Engine.RemoveAsync (rig.Engine.Torrents[0]);
+
+            var editor = new TorrentEditor (rig.TorrentDict);
+            editor.CanEditSecureMetadata = true;
+            editor.Name = $"{Path.GetInvalidPathChars()[0]}test{Path.GetInvalidPathChars ()[0]}";
+
+            var manager = await rig.Engine.AddAsync (editor.ToTorrent (), "path", new TorrentSettings ());
+            Assert.IsFalse(manager.ContainingDirectory.Contains (manager.Torrent.Name));
+            Assert.IsTrue (manager.ContainingDirectory.StartsWith (manager.SavePath));
+            Assert.AreEqual (Path.GetFullPath (manager.ContainingDirectory), manager.ContainingDirectory);
+            Assert.AreEqual (Path.GetFullPath (manager.SavePath), manager.SavePath);
+        }
+
+        [Test]
+        public async Task ContainingDirectory_PathBusting ()
+        {
+            // You can't manually add peers to private torrents
+            using var rig = TestRig.CreateMultiFile (new TestWriter ());
+            await rig.Engine.RemoveAsync (rig.Engine.Torrents[0]);
+
+            var editor = new TorrentEditor (rig.TorrentDict);
+            editor.CanEditSecureMetadata = true;
+            editor.Name = $"..{Path.DirectorySeparatorChar}..{Path.DirectorySeparatorChar}test{Path.GetInvalidPathChars ()[0]}";
+
+            Assert.ThrowsAsync<ArgumentException> (() => rig.Engine.AddAsync (editor.ToTorrent (), "path", new TorrentSettings ()));
+        }
+
         [Test]
         public void DownloadMetadata_Cancelled ()
         {

src/MonoTorrent.Tests/Client/TorrentFileInfoTests.cs

@@ -0,0 +1,69 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+using NUnit.Framework;
+
+namespace MonoTorrent.Client
+{
+    [TestFixture]
+    public class TorrentFileInfoTests
+    {
+        static string[] ValidPaths => new[] {
+            "Foo.cs",
+            $"dir1{Path.DirectorySeparatorChar}Foo.cs",
+            $"dir1{Path.DirectorySeparatorChar}dir2{Path.DirectorySeparatorChar}Foo.cs"
+        };
+        static string[] InvalidFilenames => new[] {
+            $"{Path.GetInvalidFileNameChars ()[0]}Foo.cs",
+            $"Fo{Path.GetInvalidFileNameChars ()[1]}o.cs",
+            $"dir1{Path.DirectorySeparatorChar}dir2{Path.DirectorySeparatorChar}Fo{Path.GetInvalidFileNameChars ()[2]}o.cs"
+        };
+
+        static string[] InvalidPaths => new[] {
+            $"dir1{Path.GetInvalidPathChars()[0]}asd{Path.DirectorySeparatorChar}Foo.cs",
+            $"dir1{Path.GetInvalidPathChars()[1]}{Path.DirectorySeparatorChar}dir{Path.GetInvalidPathChars()[2]}2{Path.DirectorySeparatorChar}Foo.cs",
+        };
+
+        static string[] InvalidPathAndFilenames => new[] {
+            $"dir{Path.GetInvalidPathChars()[0]}1{Path.DirectorySeparatorChar}dir2{Path.DirectorySeparatorChar}Fo{Path.GetInvalidFileNameChars()[0]}o.cs"
+        };
+
+        [Test]
+        public void PathIsValid ([ValueSource(nameof(ValidPaths))] string path)
+        {
+            Assert.AreEqual (path, TorrentFileInfo.PathAndFileNameEscape (path));
+            Assert.DoesNotThrow (() => Path.Combine (path, "test"));
+        }
+
+        [Test]
+        public void PathContainsInvalidChar ([ValueSource(nameof(InvalidPaths))] string path)
+        {
+            var escaped = TorrentFileInfo.PathAndFileNameEscape (path);
+            Assert.AreNotEqual (path, escaped);
+            Assert.IsTrue (Path.GetInvalidFileNameChars ().All (t => !Path.GetFileName (escaped).Contains (t)));
+            Assert.IsTrue (Path.GetInvalidPathChars ().All (t => !Path.GetDirectoryName (escaped).Contains (t)));
+        }
+
+        [Test]
+        public void PathAndFilenameContainInvalidChars ([ValueSource (nameof (InvalidPathAndFilenames))] string path)
+        {
+            var escaped = TorrentFileInfo.PathAndFileNameEscape (path);
+            Assert.AreNotEqual (path, escaped);
+            Assert.IsTrue (Path.GetInvalidFileNameChars ().All (t => !Path.GetFileName (escaped).Contains (t)));
+            Assert.IsTrue (Path.GetInvalidPathChars ().All (t => !Path.GetDirectoryName (escaped).Contains (t)));
+        }
+
+        [Test]
+        public void FilenameContainsInvalidChar ([ValueSource (nameof (InvalidFilenames))] string path)
+        {
+            var escaped = TorrentFileInfo.PathAndFileNameEscape (path);
+            Assert.AreNotEqual (path, escaped);
+            Assert.IsTrue (Path.GetInvalidFileNameChars ().All (t => !Path.GetFileName (escaped).Contains (t)));
+            Assert.IsTrue (Path.GetInvalidPathChars ().All (t => !Path.GetDirectoryName (escaped).Contains (t)));
+        }
+    }
+}

src/MonoTorrent/MonoTorrent.Client/Managers/TorrentFileInfo.cs

@@ -81,5 +81,25 @@ internal static TorrentFileInfo[] Create (int pieceLength, params (string torren
                 return new TorrentFileInfo (t, info.fullPath);
             }).ToArray ();
         }
+
+        internal static string PathEscape (string path)
+        {
+            foreach (var illegal in System.IO.Path.GetInvalidPathChars ())
+                path = path.Replace ($"{illegal}", Convert.ToString (illegal, 16));
+            return path;
+        }
+
+        internal static string PathAndFileNameEscape (string path)
+        {
+            var probableFilenameIndex = path.LastIndexOf (System.IO.Path.DirectorySeparatorChar);
+            var dir = probableFilenameIndex == -1 ? "" : path.Substring (0, probableFilenameIndex);
+            var filename = probableFilenameIndex == -1 ? path : path.Substring (probableFilenameIndex + 1);
+
+            dir = PathEscape (dir);
+
+            foreach (var illegal in System.IO.Path.GetInvalidFileNameChars ())
+                filename = filename.Replace ($"{illegal}", $"_{Convert.ToString (illegal, 16)}_");
+            return System.IO.Path.Combine (dir, filename);
+        }
     }
 }

src/MonoTorrent/MonoTorrent.Client/Managers/TorrentManager.cs

@@ -312,7 +312,21 @@ public double PartialProgress {
         public double Progress => Bitfield.PercentComplete;
 
         /// <summary>
-        /// The directory to download the files to
+        /// The top level directory where files can be located. If the torrent contains one file, this is the directory where
+        /// that file is stored. If the torrent contains two or more files, this value is generated by concatenating <see cref="SavePath"/>
+        /// and <see cref="Torrent.Name"/> after replacing all invalid characters with equivalents which are safe to use in file paths.
+        /// <see cref="ContainingDirectory"/> will be <see langword="null"/> until the torrent metadata has been downloaded and <see cref="HasMetadata"/> returns
+        /// <see langword="true"/>
+        public string ContainingDirectory {
+            get; private set;
+        }
+
+        /// <summary>
+        /// If this is a single file torrent, the file will be saved directly inside this directory and <see cref="ContainingDirectory"/> will
+        /// be the same as <see cref="SavePath"/>. If this is a multi-file torrent and <see cref="TorrentSettings.CreateContainingDirectory"/>
+        /// is set to <see langword="true"/>, all files will be stored in a sub-directory of <see cref="SavePath"/>. The subdirectory name will
+        /// be based on <see cref="Torrent.Name"/>, except invalid characters will be replaced. In this scenario all files will be found within
+        /// the directory specified by <see cref="ContainingDirectory"/>.
         /// </summary>
         public string SavePath { get; private set; }
 
@@ -579,7 +593,7 @@ public async Task MoveFilesAsync (string newRoot, bool overWriteExisting)
 
             try {
                 await Engine.DiskManager.MoveFilesAsync (Files, newRoot, overWriteExisting);
-                SavePath = newRoot;
+                ContainingDirectory = SavePath = newRoot;
             } catch (Exception ex) {
                 TrySetError (Reason.WriteFailure, ex);
             }
@@ -610,14 +624,20 @@ internal void SetMetadata (Torrent torrent)
             UnhashedPieces = new MutableBitField (Torrent.Pieces.Count).SetAll (true);
 
             // Now we know the torrent name, use it as the base directory name when it's a multi-file torrent
-            var savePath = SavePath;
-            if (Torrent.Files.Count > 1 && Settings.CreateContainingDirectory)
-                savePath = Path.Combine (savePath, Torrent.Name);
+            if (Torrent.Files.Count == 1 || !Settings.CreateContainingDirectory)
+                ContainingDirectory = SavePath;
+            else {
+                PathValidator.Validate (Torrent.Name);
+                ContainingDirectory = Path.GetFullPath (Path.Combine (SavePath, TorrentFileInfo.PathEscape (Torrent.Name)));
+            }
+
+            if (!ContainingDirectory.StartsWith (SavePath))
+                throw new InvalidOperationException ($"The containing directory path '{ContainingDirectory}' must be a subdirectory of '{SavePath}'.");
 
             // All files marked as 'Normal' priority by default so 'PartialProgressSelector'
             // should be set to 'true' for each piece as all files are being downloaded.
             Files = Torrent.Files.Select (file =>
-                new TorrentFileInfo (file, Path.Combine (savePath, file.Path))
+                new TorrentFileInfo (file, Path.Combine (ContainingDirectory, TorrentFileInfo.PathAndFileNameEscape (file.Path)))
             ).Cast<ITorrentFileInfo> ().ToList ().AsReadOnly ();
 
             PieceManager.Initialise ();

src/MonoTorrent/MonoTorrent/EditableTorrent.cs

@@ -43,6 +43,7 @@ public abstract class EditableTorrent
         static readonly BEncodedString CreatedByKey = "created by";
         static readonly BEncodedString EncodingKey = "encoding";
         static readonly BEncodedString InfoKey = "info";
+        static readonly BEncodedString NameKey = "name";
         private protected static readonly BEncodedString PieceLengthKey = "piece length";
         static readonly BEncodedString PrivateKey = "private";
         static readonly BEncodedString PublisherKey = "publisher";
@@ -85,6 +86,11 @@ protected BEncodedDictionary Metadata {
             get; private set;
         }
 
+        public string Name {
+            get => GetString (InfoDict, NameKey);
+            set => SetString (InfoDict, NameKey, value);
+        }
+
         public long PieceLength {
             get => GetLong (InfoDict, PieceLengthKey);
             set => SetLong (InfoDict, PieceLengthKey, value);