-
-
Notifications
You must be signed in to change notification settings - Fork 86
Feat: BitTorrent v2 Support [BEP 52] #193
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
base: master
Are you sure you want to change the base?
Conversation
- Support both SHA1 (v1) and SHA256 (v2) infohashes in validation - Add v2 hex string (64 chars) and buffer (32 bytes) detection - Update magnet parsing to accept both btih and btmh URNs - Add v2 torrent file parsing with file tree structure - Generate both SHA1 and SHA256 hashes for v2 torrents - Update validation logic to require either v1 or v2 hash 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
index.js
Outdated
// if info hash (hex/base-32 string) | ||
// if info hash v1 (hex/base-32 string) | ||
return magnet(`magnet:?xt=urn:btih:${torrentId}`) | ||
} else if (typeof torrentId === 'string' && /^[a-f0-9]{64}$/i.test(torrentId)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hate this double if thing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this better?
index.js
Outdated
// if info hash (buffer) | ||
// if info hash v1 (buffer) | ||
return magnet(`magnet:?xt=urn:btih:${arr2hex(torrentId)}`) | ||
} else if (ArrayBuffer.isView(torrentId) && torrentId.length === 32) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. Redundant. This better?
index.js
Outdated
* Parse a torrent identifier (magnet uri, .torrent file, info hash) | ||
* @param {string|ArrayBufferView|Object} torrentId | ||
* @param {Object} options | ||
* @param {string} options.hashMode - 'v1', 'v2', or 'both' (default: 'v1') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this a thing? can't it simply detect if it's a V1 V2 or both? is this really required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I think we would need to - wouldn't know in advance what version a torrent is at the time options are passed.
It's a three way switch, most reliably detected ime by checking for (length || files) for v1, file tree for v2, and both for hybrid. (Checking the meta version can be iffy and doesnt disambiguate hybrid from v2-only) :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ThaUnknown I considered this. I suppose if the user only supplies an infoHash
without infoHashV2
, a btih
without a btmh
, or a torrent file with length or files without a file tree, then assume v1. Same for vice-versa.
Applied this change in f34b051
index.js
Outdated
return magnet(`magnet:?xt=urn:btih:${arr2hex(torrentId)}`) | ||
} else if (ArrayBuffer.isView(torrentId) && torrentId.length === 32) { | ||
// if info hash v2 (buffer) | ||
return magnet(`magnet:?xt=urn:btmh:1220${arr2hex(torrentId)}`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not exactly sure how the torrentId field is used downstream, but I see ability to specify a torrent by SHA1 hash, SHA256 hash, but I'm not sure how we would pass both in the case of a hybrid torrent here. Would we just treat both swarms as separate? It is legal to have both btih
and btmh
in a magnet (and how hybrid magnets usually work)
|
||
if (isV2) { | ||
// BitTorrent v2 specific validation | ||
ensure(torrent.info['file tree'], 'info[\'file tree\']') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably also check for presence of piece layers here too :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great call. 5577ce6
index.js
Outdated
ensure(torrent.info['file tree'], 'info[\'file tree\']') | ||
} else { | ||
ensure(typeof torrent.info.length === 'number', 'info.length') | ||
// BitTorrent v1 validation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am reading on phone so not sure if I'm getting the control flow right, but is this v1 validation leg exclusive with v2? We would want to do both in the case of hybrid torrents.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops. Right. v2 Torrents would have had their v1 validation skipped. Fixed: 301c07b
index.js
Outdated
} | ||
} | ||
processFileTree(torrent.info['file tree']) | ||
torrent.info.files = files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Flattening the tree for easier processing is the right call, but it seems like we may be implicitly making a hybrid torrent here? Adding files
without pieces
would look like an (invalid) torrent file to some consumers. Thinking about what we might need to make piece validation easier downstream, and some method for flattening the file tree to get the path, length, and piece root would be good, but I think we may not want to assign it to the v1 files key
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right! We'd accidentally be making invalid hybrid torrents when someone feeds in a v2-only one. Fixed: f147872
Thanks for getting to this so quick, my week filled up faster than in anticipated. We will probably need to follow this up with some convenience methods for getting piece roots and piece layers at file offsets when working on the downstream webtorrent impl, but it looks like most of the parts we need are in place here, modulo open comments |
This ensures that pure v2 torrents don't appear to be invalid hybrids to downstream consumers, while still providing the flattened file structure for easier processing
@ThaUnknown @sneakers-the-rat Incorporated your feedback and made a bunch of tweaks. Can rebase/squash before merging, if you'd like. |
@ThaUnknown @sneakers-the-rat Awaiting feedback |
What is the purpose of this pull request? (put an "X" next to item)
[ ] Documentation update
[ ] Bug fix
[ X ] New feature
[ ] Other, please explain:
What changes did you make? (Give an overview)
Added parsing of
infoHash
and/orinfoHashv2
, as well as generation of v1, v2, or Hybrid torrents.Which issue (if any) does this pull request address?
Issue #88