Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebTorrent.remove method removes correct torrent and, incorrectly, the last torrent #2767

Open
LostBeard opened this issue Mar 27, 2024 · 1 comment

Comments

@LostBeard
Copy link

LostBeard commented Mar 27, 2024

What version of this package are you using?
2.2.0

What operating system, Node.js, and npm version?
Browser, Chrome 64bit, Windows 10

What happened?
(client is WebTorrent instance)
Calling client.remove(torrent.infoHash) with a valid torrent will remove that torrent but also remove the last torrent in client.torrents also.

The reason is:
client.remove() calls client_remove() where the below line is used without checking if the this.torrents.indexOf(torrent) returns -1, which would be fine if client._remove() was not called again by torrent._destroy() when it (_remove) calls torrent.destroy().

this.torrents.splice(this.torrents.indexOf(torrent), 1)

The code that causes allows the issue:

index.js (WebTorrent)

webtorrent/index.js

Lines 402 to 419 in ff83504

async remove (torrentId, opts, cb) {
if (typeof opts === 'function') return this.remove(torrentId, null, opts)
this._debug('remove')
const torrent = await this.get(torrentId)
if (!torrent) throw new Error(`No torrent with id ${torrentId}`)
this._remove(torrent, opts, cb)
}
_remove (torrent, opts, cb) {
if (!torrent) return
if (typeof opts === 'function') return this._remove(torrent, null, opts)
this.torrents.splice(this.torrents.indexOf(torrent), 1)
torrent.destroy(opts, cb)
if (this.dht) {
this.dht._tables.remove(torrent.infoHash)
}
}

torrent.js

webtorrent/lib/torrent.js

Lines 748 to 760 in ff83504

destroy (opts, cb) {
if (typeof opts === 'function') return this.destroy(null, opts)
this._destroy(null, opts, cb)
}
_destroy (err, opts, cb) {
if (typeof opts === 'function') return this._destroy(err, null, opts)
if (this.destroyed) return
this.destroyed = true
this._debug('destroy')
this.client._remove(this)

What did you expect to happen?
I expected only 1 torrent to be removed.

Are you willing to submit a pull request to fix this bug?
Yes. I have a pull request ready.

LostBeard added a commit to LostBeard/webtorrent that referenced this issue Mar 27, 2024
@LostBeard
Copy link
Author

LostBeard commented Mar 29, 2024

To reproduce the bug

The below code can be pasted into a website console to test for the bug. The example uses the jsdelivr site to load the webtorrent.min.js in this GitHub repo's dist folder.

The magnet links are Big Buck Bunny and Sintel from the free magnet links page.

Steps to reproduce:

var WebTorrentModule = await import('https://cdn.jsdelivr.net/gh/webtorrent/webtorrent/dist/webtorrent.min.js');
var WebTorrent = WebTorrentModule.default;
console.log('WebTorrent.VERSION', WebTorrent.VERSION);
// prints "WebTorrent.VERSION 2.2.0"
var client = new WebTorrent();
// Sintel and Big Buck Bunny magnets (trimmed to just hashes for brevity)
var magnets = [ 'magnet:?xt=urn:btih:08ada5a7a6183aae1e09d831df6748d566095a10', 'magnet:?xt=urn:btih:dd8255ecdc7ca55fb0bbf81323d87062db1f6d1c' ];
client.add(magnets[0]);
client.add(magnets[1]);
console.log('torrents.length', client.torrents.length);
// prints "torrents.length 2"
// calling the below line will cause both the specified torrent and then the last torrent in client.torrents to be removed
await client.remove(magnets[0]);
console.log('torrents.length', client.torrents.length);
// prints "torrents.length 0" due to bug
// should print "torrents.length 1"

(Edited to simplify example code)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant