Skip to content

Conversation

@cleberpereiradasilva
Copy link

Fix bug in perr.js file, when this._extensionData.m is undefined an exception occurs in line 183.

: this._extensionData && this._extensionData.m[type];
: this._extensionData.m !== undefined
? this._extensionData && this._extensionData.m[type]
: undefined
Copy link
Owner

Choose a reason for hiding this comment

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

Hey, thanks for the PR! This project isn't really getting any love so nice to have contributions :).

This change could fail if this._extensionData is undefined - something more like this._extensionData && this._extensionData.m && this._extensionData.m[type] might work. Though if m can be undefined it will also be an issue in Peer.prototype.supportsExtension. It could be worth add a new function Peer.prototype.getExtension which returns they extension value for a key and encapsulates the undefined checking.

Choose a reason for hiding this comment

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

Oh, ok, Im new in contributions. Im learning and liked for this repo. I love .torrents!

Choose a reason for hiding this comment

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

What do when this._extensionData.m is undefined?

Copy link
Owner

Choose a reason for hiding this comment

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

Just set code to undefined. It'd be best to break this out so it's more readable. So the code could be -

var code = null;
if (Message.EXTENDED_HANDSHAKE === type) {
  code = 0;
} else if (this._extensionData && this._extensionData.m) {
  code = this._extensionData.m[type];
}

@cleberpereiradasilva cleberpereiradasilva changed the title Closes #1 Try fix bug his._extensionData.m undefined in peer.js line 183 Mar 3, 2019
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