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

fixed buggy metadata vs metadata.info #4

Merged
merged 3 commits into from
May 11, 2014

Conversation

transitive-bullshit
Copy link
Member

ut_metadata.metadata has to contain only the info dictionary for properly sharing it with peers. this fixes my previous attempt to fix metadata and also fixes the ut_metadata constructor which takes in the full metadata.

@transitive-bullshit
Copy link
Member Author

Note: in addition to fixing this bug, the latest commit confirms support for torrents with multi-block metadata referenced in this issue.

info = bncode.encode(bncode.decode(metadata).info)
} catch (err) {
// TODO: throw or disregard invalid metadata?
//throw new Error('ut_metadata constructed with invalid metadata')
Copy link
Member

Choose a reason for hiding this comment

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

I think throwing an error is better than silently proceeding. If the user passes in a buffer, they think they have valid metadata. Better they get an exception and fix their code if this is not the case.

Copy link
Member

Choose a reason for hiding this comment

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

I'll fix this when I merge.

@feross
Copy link
Member

feross commented May 11, 2014

Awesome, thank you for making this more consistent. And especially thanks for the test that verifies that multi-block metadata is working. I knew this was working but the test will make sure it stays that way :)

@feross feross merged commit 5a3c0ab into webtorrent:master May 11, 2014
@feross
Copy link
Member

feross commented May 11, 2014

Published as 2.0.0.

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

Successfully merging this pull request may close these issues.

2 participants