alanmcgovern / monotorrent

The official repository for MonoTorrent, a bittorrent library for .NET
https://github.com/alanmcgovern/monotorrent
MIT License
1.14k stars 395 forks source link

Empty file in torrent causes infinite loop #560

Closed borigas closed 1 year ago

borigas commented 1 year ago

Creating a v2 torrent from a folder with an empty file causes an infinite loop in MerkleHash.TryHash() because src.Length == 0. This test repros the problem in master and release-v3.0.0-beta-0034.

[TestFixture]
public class EmptyFileInTorrentTests
{
    [Test]
    public void EmptyFileInTorrent ()
    {
        var dir = new DirectoryInfo ("Data");
        if (!dir.Exists)
            dir.Create ();

        var emptyFile = new FileInfo (Path.Combine (dir.FullName, "Empty.file"));
        File.WriteAllText (emptyFile.FullName, "");

        var nonEmptyFile = new FileInfo (Path.Combine (dir.FullName, "NonEmpty.file"));
        File.WriteAllText (nonEmptyFile.FullName, "aoeuaoeu");

        emptyFile.Refresh ();
        Assert.AreEqual (0, emptyFile.Length);

        ITorrentFileSource fileSource = new TorrentFileSource (dir.FullName);
        TorrentCreator torrentCreator = new TorrentCreator (TorrentType.V1V2Hybrid);
        var creationTask = torrentCreator.CreateAsync (fileSource);

        bool completed = creationTask.Wait (10000);
        Assert.IsTrue (completed);

        var torrent = creationTask.Result;
        Assert.IsNotNull (torrent);
    }
}

I spent some time debugging, but I don't know enough about how empty files are supposed to be hashed to provide a fix. If there's more I can do to help diagnose, please let me know. Thanks for your help!

alanmcgovern commented 1 year ago

Hah, whoops! I thought i had empty files covered, but obviously not for the V2 case!

In BitTorrent V1 empty files can exist anywhere in the torrent dictionary. MonoTorrent handles these by treating them as if they all existed at the start of the file (i.e. it sorts empty files so they're all at the start of TorrentManager.Files and Torrent.Files lists). This makes some of maths/calculations easier later on.

The same should be true for BitTorrent V2, with the additional support for completely skipping empty files when generating the V2 infohash. It should be more or less a 1 liner somewhere... let's see...

alanmcgovern commented 1 year ago

This should be fixed in master! Let me know if you have any issues, and thanks for the bug report!

borigas commented 1 year ago

Thanks for fixing the create with an empty file. That seems to be working as intended. I can create a torrent with an empty file now.

I'm having problems seeding and downloading the torrents now. I think it's still related to the empty files, but the test matrix is a bit confusing.

Here's some integration tests that hopefully demonstrate what I'm seeing. For the various scenarios mentioned, they create files and attempt to seed and download. Let me know how I can help here.

image

[TestFixture]
public class EmptyFileInTorrentTests
{
    public EmptyFileInTorrentTests ()
    {
        _tracker = GetTracker (_trackerPort);
    }

    const int _trackerPort = 40000;
    const int _seederPort = 40001;
    const int _leecherPort = 40002;
    private readonly TrackerServer _tracker;

    [Test]
    public async Task DownloadFileInTorrent_V1 () => await CreateAndDownloadTorrent (TorrentType.V1Only, false);

    [Test]
    public async Task DownloadFileInTorrent_V2 () => await CreateAndDownloadTorrent (TorrentType.V2Only, false);

    [Test]
    public async Task DownloadFileInTorrent_V2_OnlyOneNonEmptyFile () => await CreateAndDownloadTorrent (TorrentType.V2Only, false, nonEmptyFileCount: 1);

    [Test]
    public async Task DownloadFileInTorrent_V1V2 () => await CreateAndDownloadTorrent (TorrentType.V1V2Hybrid, false);

    [Test]
    public async Task DownloadEmptyFileInTorrent_V1 () => await CreateAndDownloadTorrent (TorrentType.V1Only, true);

    [Test]
    public async Task DownloadEmptyFileInTorrent_V2 () => await CreateAndDownloadTorrent (TorrentType.V2Only, true);

    [Test]
    public async Task DownloadEmptyFileInTorrent_V1V2 () => await CreateAndDownloadTorrent (TorrentType.V1V2Hybrid, true);

    public async Task CreateAndDownloadTorrent (TorrentType torrentType, bool createEmptyFile, int nonEmptyFileCount = 2)
    {

        var dir = new DirectoryInfo (nameof (CreateAndDownloadTorrent));
        if (dir.Exists)
            dir.Delete (true);

        dir.Create ();
        var seederDir = dir.CreateSubdirectory ("Seeder");
        var leecherDir = dir.CreateSubdirectory ("Leecher");

        var emptyFile = new FileInfo (Path.Combine (seederDir.FullName, "Empty.file"));
        if (createEmptyFile)
            File.WriteAllText (emptyFile.FullName, "");

        var nonEmptyFiles = new List<FileInfo> ();
        for (int i = 0; i < nonEmptyFileCount; i++) {
            var nonEmptyFile = new FileInfo (Path.Combine (seederDir.FullName, $"NonEmpty{i}.file"));
            File.WriteAllText (nonEmptyFile.FullName, $"aoeu{i}");
            nonEmptyFiles.Add (nonEmptyFile);
        }

        var fileSource = new TorrentFileSource (seederDir.FullName);
        fileSource.TorrentName = nameof (CreateAndDownloadTorrent);
        TorrentCreator torrentCreator = new TorrentCreator (torrentType);
        torrentCreator.Announce = $"http://localhost:{_trackerPort}/announce";
        var encodedTorrent = await torrentCreator.CreateAsync (fileSource);
        var torrent = Torrent.Load (encodedTorrent);

        using ClientEngine seederEngine = GetEngine (_seederPort);
        using ClientEngine leecherEngine = GetEngine (_leecherPort);

        var seederManager = await StartTorrent (seederEngine, torrent, seederDir.FullName);
        var leecherManager = await StartTorrent (leecherEngine, torrent, leecherDir.FullName);

        Assert.AreEqual (TorrentState.Seeding, seederManager.State);

        Stopwatch sw = Stopwatch.StartNew ();
        while (!leecherManager.Complete && sw.Elapsed.TotalSeconds < 5) {
            await Task.Delay (100);
        }

        Assert.IsTrue (leecherManager.Complete, "Torrent should complete");

        foreach (var file in nonEmptyFiles) {
            var leecherNonEmptyFile = new FileInfo (Path.Combine (leecherDir.FullName, file.Name));
            Assert.IsTrue (leecherNonEmptyFile.Exists, $"Non empty file {file.Name} should exist");
        }

        var leecherEmptyFile = new FileInfo (Path.Combine (leecherDir.FullName, emptyFile.Name));
        Assert.AreEqual (createEmptyFile, leecherEmptyFile.Exists, "Empty file should exist when created");
    }

    private TrackerServer GetTracker (int port)
    {
        var tracker = new TrackerServer ();
        tracker.AllowUnregisteredTorrents = true;
        var listenAddress = $"http://*:{port}/";

        var listener = TrackerListenerFactory.CreateHttp (listenAddress);
        tracker.RegisterListener (listener);
        listener.Start ();
        return tracker;
    }

    private ClientEngine GetEngine (int port)
    {
        // Give an example of how settings can be modified for the engine.
        var settingBuilder = new EngineSettingsBuilder {
            // Use a fixed port to accept incoming connections from other peers for testing purposes. Production usages should use a random port, 0, if possible.
            ListenEndPoint = new IPEndPoint (IPAddress.Any, port),
            ReportedAddress = new IPEndPoint (IPAddress.Parse ("127.0.0.1"), port),
            AutoSaveLoadFastResume = false,
        };
        var engine = new ClientEngine (settingBuilder.ToSettings ());
        return engine;
    }

    private async Task<TorrentManager> StartTorrent (ClientEngine clientEngine, Torrent torrent, string saveDirectory)
    {
        TorrentSettingsBuilder torrentSettingsBuilder = new TorrentSettingsBuilder () {
            CreateContainingDirectory = false,
        };
        TorrentManager manager = await clientEngine.AddAsync (torrent, saveDirectory, torrentSettingsBuilder.ToSettings ());
        await manager.HashCheckAsync (true);
        return manager;
    }
}
borigas commented 1 year ago

I should add that I put this here because I think it's related to the empty file issue that started this thread. If you'd rather a separate issue, please let me know. Happy to move it.

alanmcgovern commented 1 year ago

https://github.com/alanmcgovern/monotorrent/pull/565

working

Amazingly, there were three distinct failures and each fix caused one or two tests to go green. Phew!

The first 3 patches are the important ones, the final one is just a cleanup to avoid adding an unneeded empty dictionary to the torrent metadata. Thanks for writing up those tests, it was pretty easy so spelunk through the code to figure out where each failure was occurring!

By the way, if you feel comfortable enough submitting PRs with failing tests, I'm more than happy to add fixes to the PRs (or review any fixes you add)!

I want to do a pass on that PR, splitting up some of the changes and adding another test or two. The issue around skipping empty files is likely to surface in more places (urgh :( ) so i'm tempted to create an internal method to distinguish between empty/non-empty files.

borigas commented 1 year ago

Thanks for the idea. I'm happy to submit a PR with integration tests if you're willing to make them pass. I'm not familiar enough with the details of the library to be likely to fix something without introducing new bugs. It looks like most of the existing tests are more focused unit tests, so I'll create a new project unless you have a better idea where to put them.

borigas commented 1 year ago

Thanks for the fix! Things are looking good!

Are there plans for an updated beta nuget package? If not, I can build my own, but hoping maybe it's as simple as 1 CI click for you. Thanks again!