Merge pull request #686 from alanmcgovern/torrent-creation-hybrid-tor…

…rent

Improve the test which validates empty files in hybrid torrents (issue #685)

src/MonoTorrent.Client/MonoTorrent/Torrent.cs

@@ -428,7 +428,7 @@ void ProcessInfo (BEncodedDictionary dictionary, ref PieceHashesV1? hashesV1, re
 
                     case ("files"):
                         // This is the list of files using the v1 torrent format.
-                        v1Files = LoadTorrentFilesV1 ((BEncodedList) keypair.Value, PieceLength);
+                        v1Files = LoadTorrentFilesV1 ((BEncodedList) keypair.Value, PieceLength, hasV1Data && hasV2Data);
                         break;
 
                     case "file tree":
@@ -674,7 +674,7 @@ static TorrentFileAttributes AttrStringToAttributesEnum (string attr)
             return result;
         }
 
-        static IList<ITorrentFile> LoadTorrentFilesV1 (BEncodedList list, int pieceLength)
+        static IList<ITorrentFile> LoadTorrentFilesV1 (BEncodedList list, int pieceLength, bool isHybridTorrent)
         {
             var sb = new StringBuilder (32);
 
@@ -736,7 +736,16 @@ static IList<ITorrentFile> LoadTorrentFilesV1 (BEncodedList list, int pieceLengt
                     // FIXME: Log invalid paths somewhere?
                     continue;
 
+                // If this is *not* a padding file, ensure it is sorted alphabetically higher than the last non-padding file
+                // when loading a hybrid torrent.
+                // 
+                // By BEP52 spec, hybrid torrents Hybrid torrents have padding files inserted between each file, and so must
+                // have a fixed hash order to guarantee that the set up finrequired to have strictly alphabetical file ordering so
+                // the v1 hashes are guaranteed to match  after padding files are inserted.
                 PathValidator.Validate (tup.path);
+                var lastNonPaddingFile = files.FindLast (t => !t.attributes.HasFlag (TorrentFileAttributes.Padding) && t.length > 0);
+                if (isHybridTorrent && !tup.attributes.HasFlag (TorrentFileAttributes.Padding) && lastNonPaddingFile != null && StringComparer.Ordinal.Compare (tup.path, lastNonPaddingFile.path) < 0)
+                    throw new TorrentException ("The list of files must be in strict alphabetical order in a hybrid torrent");
                 files.Add (tup);
             }
 
@@ -804,9 +813,13 @@ static IList<ITorrentFile> LoadTorrentFilesV2 (BEncodedDictionary fileTree, int
 
             TorrentFile.Sort (files);
 
-            // padding of last torrent must be 0.
-            var last = files.Last ();
-            files[files.Count - 1] = new TorrentFile (last.Path, last.Length, last.StartPieceIndex, last.EndPieceIndex, last.OffsetInTorrent, last.PiecesRoot, TorrentFileAttributes.None, 0);
+            // padding of last non-empty file must be 0.
+            // There may not be any non-empty files, though that'd be a weird torrent :P
+            var lastIndex = files.FindLastIndex (f => f.Length > 0);
+            if (lastIndex != -1) {
+                var last = files[lastIndex];
+                files[lastIndex] = new TorrentFile (last.Path, last.Length, last.StartPieceIndex, last.EndPieceIndex, last.OffsetInTorrent, last.PiecesRoot, TorrentFileAttributes.None, 0);
+            }
             return Array.AsReadOnly (files.ToArray ());
         }
     }

src/MonoTorrent.Client/MonoTorrent/TorrentCreator.cs

@@ -65,12 +65,12 @@ class TorrentInfo : ITorrentInfo
             public int PieceLength { get;}
             public long Size { get; }
 
-            public TorrentInfo (InfoHashes infoHashes, IList<ITorrentFile> files, int pieceLength, long size)
+            public TorrentInfo (InfoHashes infoHashes, IList<ITorrentFile> files, int pieceLength)
             {
                 InfoHashes = infoHashes;
                 Files = files;
                 PieceLength = pieceLength;
-                Size = size;
+                Size = files.Sum (t => t.Length + t.Padding);
             }
         }
 
@@ -82,9 +82,9 @@ class TorrentManagerInfo : ITorrentManagerInfo
             public TorrentInfo TorrentInfo { get; set; }
             ITorrentInfo? ITorrentManagerInfo.TorrentInfo => TorrentInfo;
 
-            public TorrentManagerInfo (IList<ITorrentManagerFile> files, TorrentInfo torrentInfo)
+            public TorrentManagerInfo (TorrentInfo torrentInfo)
             {
-                Files = files;
+                Files = Array.AsReadOnly (torrentInfo.Files.Cast<ITorrentManagerFile> ().ToArray ());
                 TorrentInfo = torrentInfo;
             }
         }
@@ -240,9 +240,6 @@ public async Task<BEncodedDictionary> CreateAsync (ITorrentFileSource fileSource
             if (mappings.Count == 0)
                 throw new ArgumentException ("The file source must contain one or more files", nameof (fileSource));
 
-            mappings.Sort ((left, right) => StringComparer.Ordinal.Compare (left.Destination, right.Destination));
-            Validate (mappings);
-
             return await CreateAsync (fileSource.TorrentName, fileSource, token);
         }
 
@@ -251,37 +248,46 @@ internal Task<BEncodedDictionary> CreateAsync (string name, ITorrentFileSource f
 
         internal async Task<BEncodedDictionary> CreateAsync (string name, ITorrentFileSource fileSource, CancellationToken token)
         {
-            var source = fileSource.Files.ToArray ();
+            var source = fileSource.Files.ToList ();
+            foreach (var file in source)
+                if (file.Source.Contains (Path.AltDirectorySeparatorChar) || file.Destination.Contains (Path.AltDirectorySeparatorChar))
+                    throw new InvalidOperationException ("DERP");
+
+            EnsureNoDuplicateFiles (source);
+
             if (source.All (t => t.Length == 0))
                 throw new InvalidOperationException ("All files which were selected to be included this torrent have a length of zero. At least one file must have a non-zero length.");
 
             if (!InfoDict.ContainsKey (PieceLengthKey))
                 PieceLength = RecommendedPieceSize (source.Sum (t => t.Length));
 
-            var rawFiles = source.Select (file => {
-                var length = file.Length;
-                var padding = (int) ((UsePadding  && length % PieceLength > 0) ? PieceLength - (length % PieceLength) : 0);
-                var info = (file.Destination, length, padding, file.Source);
-                return info;
-            }).ToArray ();
 
             // Hybrid and V2 torrents *must* hash files in the same order as they end up being stored in the bencoded dictionary,
-            // which means they must be alphabetical.
+            // which means they must be alphabetical. Do this before creating the TorrentFileInfo objects so the start/end piece indices
+            // are calculated correctly, which is needed so the files are hashed in the correct order for V1 metadata if this is a
+            // hybrid torrent
             if (Type.HasV2 ())
-                rawFiles = rawFiles.OrderBy (t => t.Destination, StringComparer.Ordinal).ToArray ();
+                source = source.OrderBy (t => t.Destination, StringComparer.Ordinal).ToList ();
+
+            // The last non-empty file should have no padding bytes. There may be additional
+            // empty files after this one depending on how the files are sorted, but they have
+            // no impact on padding.
+            var lastNonEmptyFileIndex = source.FindLastIndex (t => t.Length > 0);
 
-            // The last non-empty file never has padding bytes
-            var last = rawFiles.Where (t => t.length != 0).Last ();
-            var index = Array.IndexOf (rawFiles, last);
-            rawFiles[index].padding = 0;
+            // TorrentFileInfo.Create will sort the files so the empty ones are first.
+            // Resort them before putting them in the BEncodedDictionary metadata for the torrent
+            var files = TorrentFileInfo.Create (PieceLength, source.Select ((file, index) => {
+                var length = file.Length;
+                var padding =  (int) ((UsePadding && index < lastNonEmptyFileIndex && length % PieceLength > 0) ? PieceLength - (length % PieceLength) : 0);
+                var info = (file.Destination, length, padding, file.Source);
+                return info;
+            }).ToArray ());
 
-            var files = TorrentFileInfo.Create (PieceLength, rawFiles);
-            var manager = new TorrentManagerInfo (files,
+            var manager = new TorrentManagerInfo (
                 new TorrentInfo (
                     new InfoHashes (Type.HasV1 () ? InfoHash.FromMemory (new byte[20]) : null, Type.HasV2 () ? InfoHash.FromMemory (new byte[32]) : null),
                     files,
-                    PieceLength,
-                    files.Sum (t => t.Length + t.Padding)
+                    PieceLength
                 )
             );
 
@@ -317,8 +323,13 @@ internal async Task<BEncodedDictionary> CreateAsync (string name, ITorrentFileSo
                 info["file tree"] = fileTree;
             }
 
+            // re-sort these by destination path if we have BitTorrent v2 metadata. The files were sorted this way originally
+            // but empty ones were popped to the front when creating ITorrentManagerFile objects.
+            if (Type.HasV2 ())
+                files = files.OrderBy (t => t.Path, StringComparer.Ordinal).ToArray ();
+
             if (Type.HasV1 ()) {
-                if (manager.Files.Count == 1 && files[0].Path == name)
+                if (manager.Files.Count == 1 && source[0].Destination == name)
                     CreateSingleFileTorrent (torrent, merkleLayers, fileSHA1Hashes, fileMD5Hashes, files);
                 else
                     CreateMultiFileTorrent (torrent, merkleLayers, fileSHA1Hashes, fileMD5Hashes, files);
@@ -329,7 +340,7 @@ internal async Task<BEncodedDictionary> CreateAsync (string name, ITorrentFileSo
 
         void AppendFileTree (ITorrentManagerFile key, ReadOnlyMemory<byte> value, BEncodedDictionary fileTree)
         {
-            var parts = key.Path.Split ('/');
+            var parts = key.Path.Split (Path.DirectorySeparatorChar);
             foreach (var part in parts) {
                 if (!fileTree.TryGetValue (part, out BEncodedValue? inner)) {
                     fileTree[part] = inner = new BEncodedDictionary ();
@@ -340,9 +351,10 @@ void AppendFileTree (ITorrentManagerFile key, ReadOnlyMemory<byte> value, BEncod
                 value = MerkleTreeHasher.Hash (value.Span, BitOps.CeilLog2 (PieceLength / Constants.BlockSize));
 
             var fileData = new BEncodedDictionary {
-                {"length", (BEncodedNumber) key.Length },
-                { "pieces root", BEncodedString.FromMemory (value) }
+                {"length", (BEncodedNumber) key.Length }
             };
+            if (!value.IsEmpty)
+                fileData["pieces root"] = BEncodedString.FromMemory (value);
 
             fileTree.Add ("", fileData);
         }
@@ -473,8 +485,9 @@ void AddCommonStuff (BEncodedDictionary torrent)
 
             var merkleLayers = new Dictionary<ITorrentManagerFile, ReadOnlyMemory<byte>> ();
             if (merkleHashes.Length > 0) {
+                // NOTE: Empty files have no merkle root as they have no data. We still include them in this dictionary so the files are embedded in the torrent.
                 foreach (var file in manager.Files)
-                    merkleLayers.Add (file, merkleHashes.Slice (file.StartPieceIndex * 32, (file.EndPieceIndex - file.StartPieceIndex + 1) * 32));
+                    merkleLayers.Add (file, file.Length == 0 ? default : merkleHashes.Slice (file.StartPieceIndex * 32, file.PieceCount * 32));
             }
             return (sha1Hashes, merkleLayers, fileSHA1Hashes, fileMD5Hashes);
         }
@@ -602,16 +615,20 @@ static BEncodedValue ToPaddingFileInfoDict (ITorrentManagerFile file, Dictionary
             return fileDict;
         }
 
-        static void Validate (List<FileMapping> maps)
+        static void EnsureNoDuplicateFiles (List<FileMapping> maps)
         {
             foreach (FileMapping map in maps)
                 PathValidator.Validate (map.Destination);
 
-            // Ensure all the destination files are unique too. The files should already be sorted.
-            for (int i = 1; i < maps.Count; i++)
-                if (maps[i - 1].Destination == maps[i].Destination)
+            var knownFiles = new Dictionary<string, FileMapping> ();
+            for (int i = 0; i < maps.Count; i++) {
+                if (knownFiles.TryGetValue (maps[i].Destination, out var prior)) {
                     throw new ArgumentException (
-                        $"Files '{maps[i - 1].Source}' and '{maps[i].Source}' both map to the same destination '{maps[i].Destination}'");
+                        $"Files '{maps[i].Source}' and '{prior.Source}' both map to the same destination '{maps[i].Destination}'");
+                } else {
+                    knownFiles.Add (maps[i].Destination, maps[i]);
+                }
+            }
         }
     }
 }

src/Tests/Tests.MonoTorrent.Client/MonoTorrent/TorrentCreatorBep47Tests.cs

@@ -143,11 +143,56 @@ public async Task FileLengthSameAsPieceLength ([Values (TorrentType.V1Only, Torr
                 }
             };
 
-            var torrent = Torrent.Load (await CreateTestBenc (type, files));
+            var rawDict = await CreateTestBenc (type, files);
+            var torrent = Torrent.Load (rawDict);
             Assert.AreEqual (0, torrent.Files[0].Padding);
             Assert.AreEqual (0, torrent.Files[1].Padding);
         }
 
+        [Test]
+        public async Task HybridTorrentWithEmptyFiles ()
+        {
+            // Hybrid torrents must be strictly alphabetically ordered so v1 and v2 metadata ends up
+            // matching. These are in the wrong order.
+            var inputFiles = new Source {
+                TorrentName = "asfg",
+                Files = new[] {
+                    new FileMapping (Path.Combine("a", "File1"), Path.Combine("a", "File1"), 2),
+                    new FileMapping (Path.Combine("a", "File2"), Path.Combine("a", "File2"), 0),
+                    new FileMapping (Path.Combine("a", "File0"), Path.Combine("a", "File0"), 1),
+                }
+            };
+
+            var rawDict = await CreateTestBenc (TorrentType.V1V2Hybrid, inputFiles);
+
+            // Load the torrent for good measure
+            Assert.DoesNotThrow (() => Torrent.Load (rawDict));
+
+            // Validate order in the v1 data. Duplicate the underlying list first as we'll remove padding from it later.
+            var filesList = (BEncodedList) ((BEncodedDictionary) rawDict["info"])["files"];
+
+            // We should have 1 padding file - the last file is empty, so the second last file has
+            // no padding either. Only the first one does.
+            Assert.AreEqual (4, filesList.Count);
+
+            var padding = (BEncodedDictionary) filesList[1];
+            var path = (BEncodedList) padding["path"];
+            Assert.AreEqual (".pad", ((BEncodedString) path[0]).Text);
+            Assert.AreEqual (PieceLength - 1, ((BEncodedNumber) padding["length"]).Number);
+
+            // Remove the padding, then check the order of the actual files!
+            filesList.RemoveAt (1);
+
+            for (int i = 0; i < filesList.Count; i++) {
+                var dict = (BEncodedDictionary) filesList[i];
+                var parts = (BEncodedList) dict["path"];
+                Assert.AreEqual (2, parts.Count);
+                Assert.AreEqual ("a", parts[0].ToString ());
+                Assert.AreEqual ("File" + i, parts[1].ToString ());
+            }
+
+        }
+
         [Test]
         public async Task Padding ([Values(TorrentType.V1OnlyWithPaddingFiles, TorrentType.V1V2Hybrid)] TorrentType type)
         {

src/Tests/Tests.MonoTorrent.Client/MonoTorrent/TorrentCreatorTests.cs

@@ -242,6 +242,13 @@ public async Task CreateV2Torrent_WithExtraEmptyFile ()
             files.Add (new FileMapping ("empty_source", "empty_dest", 0));
 
             var torrentDict = await creator.CreateAsync (new CustomFileSource (files));
+            var fileTree = (BEncodedDictionary) ((BEncodedDictionary) torrentDict["info"])["file tree"];
+
+            // Get the metadata for this file specifically. It should only have a length of zero, nothing else.
+            var emptyFile = (BEncodedDictionary) ((BEncodedDictionary) fileTree["empty_dest"])[""];
+            Assert.IsFalse (emptyFile.ContainsKey ("pieces root"));
+            Assert.AreEqual (new BEncodedNumber(0), emptyFile["length"]);
+
             var actual = Torrent.Load (torrentDict);
             var expected = Torrent.Load (Path.Combine (Path.GetDirectoryName (typeof (TorrentCreatorTests).Assembly.Location), $"test_torrent_64.torrent"));
 
@@ -280,6 +287,42 @@ public async Task CreateV2Torrent_SortFilesCorrectly ()
             Assert.AreEqual ("C", torrent.Files[2].Path);
         }
 
+        [Test]
+        public async Task CreateHybridTorrent_SortFilesCorrectly ()
+        {
+            var destFiles = new[] {
+                "A.txt",
+                "B.txt",
+                Path.Combine ("D", "a", "A.txt"),
+                Path.Combine("a", "z", "Z.txt"),
+            };
+
+            var dir = Path.Combine (Path.Combine ("foo", "bar", "baz"));
+            var fileSource = new CustomFileSource (destFiles.Select (t =>
+                new FileMapping (Path.Combine (dir, t), t, 4)
+            ).ToList ());
+
+            TorrentCreator torrentCreator = new TorrentCreator (TorrentType.V1V2Hybrid, TestFactories);
+            var torrent = await torrentCreator.CreateAsync (fileSource);
+
+            var fileTree = (BEncodedDictionary) ((BEncodedDictionary) torrent["info"])["file tree"];
+
+            // Ensure the directory tree was converted into a dictionary tree.
+            Assert.IsTrue (fileTree.ContainsKey ("A.txt"));
+            Assert.IsTrue (fileTree.ContainsKey ("D"));
+            Assert.IsTrue (fileTree.ContainsKey ("a"));
+
+            // Get the metadata for this file specifically. It should only have a length of zero, nothing else.
+            var dFile = (BEncodedDictionary) ((BEncodedDictionary) fileTree["D"]);
+            Assert.IsTrue (dFile.ContainsKey ("a"));
+            Assert.IsTrue (((BEncodedDictionary) dFile["a"]).ContainsKey ("A.txt"));
+
+            var aFile = (BEncodedDictionary) ((BEncodedDictionary) fileTree["a"]);
+            Assert.IsTrue (aFile.ContainsKey ("z"));
+            Assert.IsTrue (((BEncodedDictionary) aFile["z"]).ContainsKey ("Z.txt"));
+
+
+        }
 
         [Test]
         public void CannotCreateTorrentWithAllEmptyFiles ([Values (TorrentType.V1Only, TorrentType.V1V2Hybrid, TorrentType.V2Only)] TorrentType torrentType)
@@ -331,7 +374,7 @@ public void IllegalDestinationPath ()
                 var source = new Source {
                     TorrentName = "asd",
                     Files = new[] {
-                        new FileMapping("a", "../../dest1", 123)
+                        new FileMapping("a", Path.Combine ("..", "..", "dest1"), 123)
                     }
                 };
                 new TorrentCreator (TorrentType.V1Only, Factories.Default.WithPieceWriterCreator (files => new DiskWriter (files))).Create (source);