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

If torrent is not private, leave private flag unset #1411

Merged
merged 1 commit into from
May 25, 2018
Merged

Conversation

feross
Copy link
Member

@feross feross commented May 25, 2018

If torrent is not private, leave private flag unset. This ensures that the torrent info hash will match the result generated by other tools, including webtorrent-cli.

Fixes: webtorrent/webtorrent-hybrid#79


More info:

WebTorrent Desktop uses the result of the "Private" checkbox at torrent creation time to set the torrent's private flag to true or false. Unfortunately, there's an issue with the way this private flag spec was designed. There are actually 3 possible values for the private flag:

  • private: true
  • private: false
  • not specified (defaults to false)

The problem is that private flag is part of the info section in the .torrent file, which means that each of these three values for private will produce a different info hash.

So, what's happening is that webtorrent-cli and webtorrent-hybrid are not setting private at all, while WebTorrent Desktop is setting it to false.

We should either set it to true or leave it not specified to match what webtorrent-cli, webtorrent-hybrid, and other mainstream torrent clients do.

If torrent is not private, leave private flag unset. This ensures that the torrent info hash will match the result generated by other tools, including webtorrent-cli.
Copy link
Member

@DiegoRBaquero DiegoRBaquero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to leave it unset rather than changing the others to setting it to false. It's unneeded info in the torrent tbh.

Copy link
Contributor

@codealchemist codealchemist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense and the change is looking good.
Thanks!

@DiegoRBaquero DiegoRBaquero merged commit ad63ec8 into master May 25, 2018
@DiegoRBaquero DiegoRBaquero deleted the is-private branch May 25, 2018 17:15
@feross
Copy link
Member Author

feross commented May 27, 2018

Thanks for the review!

@lock lock bot locked as resolved and limited conversation to collaborators Aug 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants