From 00f2bb1d3ff1d9b95f5f0cbbae1d4f3ecf9d2daa Mon Sep 17 00:00:00 2001 From: "alan.mcgovern" Date: Sun, 25 Aug 2024 14:36:04 +0100 Subject: [PATCH 1/2] Improve the test which validates empty files in hybrid torrents The test didn't verify the underlying bencoded dictionary contained no 'pieces root', but it did verify that no pieces root was loaded for zero length files. This fixes one potential source of incompatibility when working with zero length files. --- src/MonoTorrent.Client/MonoTorrent/TorrentCreator.cs | 8 +++++--- .../MonoTorrent/TorrentCreatorTests.cs | 7 +++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/MonoTorrent.Client/MonoTorrent/TorrentCreator.cs b/src/MonoTorrent.Client/MonoTorrent/TorrentCreator.cs index 7433ce3e1..f68b2ea85 100644 --- a/src/MonoTorrent.Client/MonoTorrent/TorrentCreator.cs +++ b/src/MonoTorrent.Client/MonoTorrent/TorrentCreator.cs @@ -340,9 +340,10 @@ void AppendFileTree (ITorrentManagerFile key, ReadOnlyMemory 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 +474,9 @@ void AddCommonStuff (BEncodedDictionary torrent) var merkleLayers = new Dictionary> (); 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); } diff --git a/src/Tests/Tests.MonoTorrent.Client/MonoTorrent/TorrentCreatorTests.cs b/src/Tests/Tests.MonoTorrent.Client/MonoTorrent/TorrentCreatorTests.cs index 181d7df4d..5590639e1 100644 --- a/src/Tests/Tests.MonoTorrent.Client/MonoTorrent/TorrentCreatorTests.cs +++ b/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")); From 252a08647f1e72e384231061cf98b6bfef2261dc Mon Sep 17 00:00:00 2001 From: Alan McGovern Date: Tue, 27 Aug 2024 07:57:39 +0100 Subject: [PATCH 2/2] Ensure BitTorrent v1 files are in the same order as v2 files They were out of sync when empty files were in the torrent, however due to interesting quirks, this didn't cause the infohash or any piece hashes to be incorrect as only empty files were out-of-order. Fix this by resorting the files one more time before putting them in the torrent metadata. Also - this is only detectable by directly validating the content of the BEncodedDictionary for some reason. A validation was missing while parsing the underlying BENcodedDictionary, it was written to operate on the parsed ITorrentFile objects instead, and at that point it was all correct. --- src/MonoTorrent.Client/MonoTorrent/Torrent.cs | 23 ++++-- .../MonoTorrent/TorrentCreator.cs | 77 +++++++++++-------- .../MonoTorrent/TorrentCreatorBep47Tests.cs | 47 ++++++++++- .../MonoTorrent/TorrentCreatorTests.cs | 38 ++++++++- 4 files changed, 147 insertions(+), 38 deletions(-) diff --git a/src/MonoTorrent.Client/MonoTorrent/Torrent.cs b/src/MonoTorrent.Client/MonoTorrent/Torrent.cs index f0ba11f1a..aee787fa8 100644 --- a/src/MonoTorrent.Client/MonoTorrent/Torrent.cs +++ b/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 LoadTorrentFilesV1 (BEncodedList list, int pieceLength) + static IList LoadTorrentFilesV1 (BEncodedList list, int pieceLength, bool isHybridTorrent) { var sb = new StringBuilder (32); @@ -736,7 +736,16 @@ static IList 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 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 ()); } } diff --git a/src/MonoTorrent.Client/MonoTorrent/TorrentCreator.cs b/src/MonoTorrent.Client/MonoTorrent/TorrentCreator.cs index f68b2ea85..b6a38cc3e 100644 --- a/src/MonoTorrent.Client/MonoTorrent/TorrentCreator.cs +++ b/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 files, int pieceLength, long size) + public TorrentInfo (InfoHashes infoHashes, IList 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 files, TorrentInfo torrentInfo) + public TorrentManagerInfo (TorrentInfo torrentInfo) { - Files = files; + Files = Array.AsReadOnly (torrentInfo.Files.Cast ().ToArray ()); TorrentInfo = torrentInfo; } } @@ -240,9 +240,6 @@ public async Task 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 CreateAsync (string name, ITorrentFileSource f internal async Task 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 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 CreateAsync (string name, ITorrentFileSo void AppendFileTree (ITorrentManagerFile key, ReadOnlyMemory 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 (); @@ -604,16 +615,20 @@ static BEncodedValue ToPaddingFileInfoDict (ITorrentManagerFile file, Dictionary return fileDict; } - static void Validate (List maps) + static void EnsureNoDuplicateFiles (List 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 (); + 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]); + } + } } } } diff --git a/src/Tests/Tests.MonoTorrent.Client/MonoTorrent/TorrentCreatorBep47Tests.cs b/src/Tests/Tests.MonoTorrent.Client/MonoTorrent/TorrentCreatorBep47Tests.cs index 9bd4324c5..ae597838b 100644 --- a/src/Tests/Tests.MonoTorrent.Client/MonoTorrent/TorrentCreatorBep47Tests.cs +++ b/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) { diff --git a/src/Tests/Tests.MonoTorrent.Client/MonoTorrent/TorrentCreatorTests.cs b/src/Tests/Tests.MonoTorrent.Client/MonoTorrent/TorrentCreatorTests.cs index 5590639e1..aa99c2555 100644 --- a/src/Tests/Tests.MonoTorrent.Client/MonoTorrent/TorrentCreatorTests.cs +++ b/src/Tests/Tests.MonoTorrent.Client/MonoTorrent/TorrentCreatorTests.cs @@ -287,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) @@ -338,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);