From 05e5c37230faebe3748e62b66b4ececa5ec0a78c Mon Sep 17 00:00:00 2001 From: William Wolf Date: Mon, 16 Feb 2015 16:36:04 -0800 Subject: [PATCH] The 'fRelay' param on Version requests is optional. I'v seen `RangeError: index out of range` crashes from nodes that don't send it, when `bitcore-p2p` tries to `readUInt8()` the last bit. This also adds `relay` property to `Peer` objects that is respected when sending `Version` messages. --- lib/messages.js | 11 +++++++++-- lib/peer.js | 5 +++-- test/data/messages.json | 3 +++ test/messages.js | 15 +++++++++++++++ test/peer.js | 23 ++++++++++++++++++++++- 5 files changed, 52 insertions(+), 5 deletions(-) diff --git a/lib/messages.js b/lib/messages.js index 05d6d1e..2fcf9c0 100644 --- a/lib/messages.js +++ b/lib/messages.js @@ -186,12 +186,13 @@ module.exports.Message = Message; * @param{string} subversion - version of the client * @param{Buffer} nonce - a random 8 bytes buffer */ -function Version(subversion, nonce) { +function Version(subversion, nonce, relay) { var packageInfo = require('../package.json'); this.command = 'version'; this.version = PROTOCOL_VERSION; this.subversion = subversion || '/bitcore:' + packageInfo.version + '/'; this.nonce = nonce || CONNECTION_NONCE; + this.relay = relay === false ? false : true; } util.inherits(Version, Message); @@ -257,7 +258,12 @@ Version.prototype.fromBuffer = function(payload) { * @desc Whether the remote peer should announce relayed transactions or not, see BIP 0037 * @type {boolean} */ - this.relay = !!parser.readUInt8(); + // This field is optional, so should not always be read + if(parser.finished()) { + this.relay = true; + } else { + this.relay = !!parser.readUInt8(); + } this._checkFinished(parser); return this; @@ -274,6 +280,7 @@ Version.prototype.getPayload = function() { put.varint(this.subversion.length); put.put(new Buffer(this.subversion, 'ascii')); put.word32le(this.start_height); + put.word8(this.relay); return put.buffer(); }; diff --git a/lib/peer.js b/lib/peer.js index 908f7b4..435fbbb 100644 --- a/lib/peer.js +++ b/lib/peer.js @@ -33,7 +33,7 @@ var MAX_RECEIVE_BUFFER = 10000000; * @returns {Peer} A new instance of Peer. * @constructor */ -function Peer(host, port, network) { +function Peer(host, port, network, relay) { if (!(this instanceof Peer)) { return new Peer(host, port, network); } @@ -48,6 +48,7 @@ function Peer(host, port, network) { this.status = Peer.STATUS.DISCONNECTED; this.network = network || Networks.defaultNetwork; this.port = port || this.network.port; + this.relay = relay === false ? false : true; this.dataBuffer = new Buffers(); @@ -161,7 +162,7 @@ Peer.prototype.sendMessage = function(message) { * Internal function that sends VERSION message to the remote peer. */ Peer.prototype._sendVersion = function() { - var message = new Messages.Version(); + var message = new Messages.Version(null, null, this.relay); this.sendMessage(message); }; diff --git a/test/data/messages.json b/test/data/messages.json index 21728f0..8bbb310 100644 --- a/test/data/messages.json +++ b/test/data/messages.json @@ -3,6 +3,9 @@ "message": "f9beb4d976657273696f6e000000000065000000fc970f17721101000100000000000000ba62885400000000010000000000000000000000000000000000ffffba8886dceab0010000000000000000000000000000000000ffff05095522208de7e1c1ef80a1cea70f2f5361746f7368693a302e392e312fa317050001", "payload": "721101000100000000000000ba62885400000000010000000000000000000000000000000000ffffba8886dceab0010000000000000000000000000000000000ffff05095522208de7e1c1ef80a1cea70f2f5361746f7368693a302e392e312fa317050001" }, + "VERSION_NO_FRELAY": { + "payload": "701101000100000000000000d78be25400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000008f7e557f1b892912102f626974636f72653a302e31302e302f00000000" + }, "ALERT": { "message": "", "payload": "73010000003766404f00000000b305434f00000000f2030000f1030000001027000048ee00000064000000004653656520626974636f696e2e6f72672f666562323020696620796f7520686176652074726f75626c6520636f6e6e656374696e67206166746572203230204665627275617279004730450221008389df45f0703f39ec8c1cc42c13810ffcae14995bb648340219e353b63b53eb022009ec65e1c1aaeec1fd334c6b684bde2b3f573060d5b70c3a46723326e4e8a4f1" diff --git a/test/messages.js b/test/messages.js index 204bfaf..ee260c3 100644 --- a/test/messages.js +++ b/test/messages.js @@ -113,7 +113,22 @@ describe('Messages', function() { clazz.forBlock(BufferUtils.reverse(new Buffer(hash, 'hex'))).should.deep.equal(b); clazz.forTransaction(BufferUtils.reverse(new Buffer(hash, 'hex'))).should.deep.equal(t); }); + }); + it('Version#fromBuffer works w/o fRelay arg', function() { + var messageHex = Data.VERSION_NO_FRELAY.payload; + var message = new Messages.Version() + .fromBuffer(new Buffer(messageHex, 'hex')); + }); + + it('Version#relay setting works', function() { + [true,false].forEach(function(relay) { + var message = new Messages.Version(null,null,relay); + message.relay.should.equal(relay); + var messageBuf = message.getPayload(); + var newMessage = new Messages.Version().fromBuffer(messageBuf) + newMessage.relay.should.equal(relay); + }); }); }); diff --git a/test/peer.js b/test/peer.js index d02a3d6..30a2aae 100644 --- a/test/peer.js +++ b/test/peer.js @@ -20,7 +20,7 @@ describe('Peer', function() { describe('Integration test', function() { it('parses this stream of data from a connection', function(callback) { - var peer = new p2p.Peer(''); + var peer = new Peer(''); var stub = sinon.stub(); var dataCallback; var connectCallback; @@ -120,4 +120,25 @@ describe('Peer', function() { peer.should.equal(peer2); }); + + it('Peer.relay setting set properly', function() { + var peer = new Peer('localhost'); + peer.relay.should.equal(true); + var peer2 = new Peer('localhost', null, null, false); + peer2.relay.should.equal(false); + var peer3 = new Peer('localhost', null, null, true); + peer3.relay.should.equal(true); + }); + + it('Peer.relay setting respected', function() { + [true,false].forEach(function(relay) { + var peer = new Peer('localhost', null, null, relay); + var peerSendMessageStub = sinon.stub(Peer.prototype, 'sendMessage', function(message) { + message.relay.should.equal(relay); + }); + peer._sendVersion(); + peerSendMessageStub.restore(); + }); + }); + });