From e83c243b8d0d25f710ffa6b905290c8491b66a00 Mon Sep 17 00:00:00 2001 From: Oli Evans Date: Tue, 6 Nov 2018 17:05:48 +0000 Subject: [PATCH 1/4] fix: handle peer-info validation errors BREAKING CHANGE. Previously swarm.peers would throw an uncaught error if any peer in the reponse could have its peerId or multiaddr validated. This PR catches errors that occur while validating the peer info. The returned array will contain an entry for every peer in the ipfs response. peer-info objects that couldn't be validated, now have an `error` property and a `rawPeerInfo` property. This at least means the count of peers in the response will be accurate, and there the info is available to the caller. This means that callers now have to deal with peer-info objects that may not have a `peer or `addr` property. Adds `nock` tests to exercice the code under different error conditions. Doing so uncovered a bug in our legacy go-ipfs <= 0.4.4 peer info parsing, which is also fixed. The code was trying to decapusalate the peerId from the multiaddr, but doing so trims the peerId rather than returning it. fixes #885 License: MIT Signed-off-by: Oli Evans --- package.json | 3 +- src/swarm/peers.js | 93 ++++++++++++++++++++----------------- test/swarm.spec.js | 111 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 164 insertions(+), 43 deletions(-) create mode 100644 test/swarm.spec.js diff --git a/package.json b/package.json index b28f9a6b0..eb0b0a4c6 100644 --- a/package.json +++ b/package.json @@ -48,7 +48,7 @@ "libp2p-crypto": "~0.14.0", "lodash": "^4.17.11", "lru-cache": "^4.1.3", - "multiaddr": "^5.0.0", + "multiaddr": "^5.0.2", "multibase": "~0.5.0", "multihashes": "~0.4.14", "ndjson": "^1.5.0", @@ -87,6 +87,7 @@ "gulp": "^3.9.1", "interface-ipfs-core": "~0.84.3", "ipfsd-ctl": "~0.40.0", + "nock": "^10.0.2", "pull-stream": "^3.6.9", "stream-equal": "^1.1.1" }, diff --git a/src/swarm/peers.js b/src/swarm/peers.js index 9ccbb1d2a..37d11cd7e 100644 --- a/src/swarm/peers.js +++ b/src/swarm/peers.js @@ -10,56 +10,65 @@ module.exports = (send) => { callback = opts opts = {} } - const verbose = opts.v || opts.verbose - send({ path: 'swarm/peers', qs: opts - }, (err, result) => { + }, (err, response) => { if (err) { return callback(err) } + const peerInfo = parsePeersResponse(verbose, response) + callback(null, peerInfo) + }) + }) +} - // go-ipfs <= 0.4.4 - if (result.Strings) { - return callback(null, result.Strings.map((p) => { - const res = {} - - if (verbose) { - const parts = p.split(' ') - res.addr = multiaddr(parts[0]) - res.latency = parts[1] - } else { - res.addr = multiaddr(p) - } - - res.peer = PeerId.createFromB58String( - res.addr.decapsulate('ipfs') - ) - - return res - })) - } - - // go-ipfs >= 0.4.5 - callback(null, (result.Peers || []).map((p) => { - const res = { - addr: multiaddr(p.Addr), - peer: PeerId.createFromB58String(p.Peer), - muxer: p.Muxer - } - - if (p.Latency) { - res.latency = p.Latency - } +function parsePeersResponse (verbose, response) { + // go-ipfs <= 0.4.4 + if (Array.isArray(response.Strings)) { + return response.Strings.map(parseLegacyPeer.bind(null, verbose)) + } + // go-ipfs >= 0.4.5 + if (Array.isArray(response.Peers)) { + return response.Peers.map(parsePeer.bind(null, verbose)) + } + return [] +} - if (p.Streams) { - res.streams = p.Streams - } +function parseLegacyPeer (verbose, peer) { + const res = {} + try { + if (verbose) { + const parts = peer.split(' ') + res.addr = multiaddr(parts[0]) + res.latency = parts[1] + } else { + res.addr = multiaddr(peer) + } + res.peer = PeerId.createFromB58String(res.addr.getPeerId()) + } catch (error) { + res.error = error + res.rawPeerInfo = peer + } + return res +} - return res - })) - }) - }) +function parsePeer (verbose, peer) { + const res = {} + try { + res.addr = multiaddr(peer.Addr) + res.peer = PeerId.createFromB58String(peer.Peer) + res.muxer = peer.Muxer + } catch (error) { + res.error = error + res.rawPeerInfo = peer + } + if (peer.Latency) { + res.latency = peer.Latency + } + if (peer.Streams) { + res.streams = peer.Streams + } + return res } diff --git a/test/swarm.spec.js b/test/swarm.spec.js new file mode 100644 index 000000000..f7a549418 --- /dev/null +++ b/test/swarm.spec.js @@ -0,0 +1,111 @@ +/* eslint-env mocha */ +'use strict' + +const nock = require('nock') +const chai = require('chai') +const dirtyChai = require('dirty-chai') +const expect = chai.expect +chai.use(dirtyChai) + +const IPFSApi = require('../src') + +describe('.swarm.peers', function () { + this.timeout(50 * 1000) // slow CI + + const ipfs = IPFSApi('/ip4/127.0.0.1/tcp/5001') + const apiUrl = 'http://127.0.0.1:5001' + + it('handles a peer response', (done) => { + const response = { 'Peers': [{ 'Addr': '/ip4/104.131.131.82/tcp/4001', 'Peer': 'QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ', 'Latency': '', 'Muxer': '', 'Streams': null }] } + + const scope = nock(apiUrl) + .post('/api/v0/swarm/peers') + .query(true) + .reply(200, response) + + ipfs.swarm.peers((err, res) => { + expect(err).to.not.exist() + expect(res).to.be.a('array') + expect(res.length).to.equal(1) + expect(res[0].error).to.not.exist() + expect(res[0].addr.toString()).to.equal(response.Peers[0].Addr) + expect(res[0].peer.toB58String()).to.equal(response.Peers[0].Peer) + expect(scope.isDone()).to.equal(true) + done() + }) + }) + + it('handles a go-ipfs <= 0.4.4 peer response', (done) => { + const response = { 'Strings': ['/ip4/73.109.217.59/tcp/49311/ipfs/QmWjxEGC7BthJrCf7QTModrcsRweHbupdPTY4oGMVoDZXm'] } + + const scope = nock(apiUrl) + .post('/api/v0/swarm/peers') + .query(true) + .reply(200, response) + + ipfs.swarm.peers((err, res) => { + console.log(res[0].rawPeerInfo) + expect(err).to.not.exist() + expect(res).to.be.a('array') + expect(res.length).to.equal(1) + expect(res[0].error).to.not.exist() + expect(res[0].addr.toString()).to.equal('/ip4/73.109.217.59/tcp/49311/ipfs/QmWjxEGC7BthJrCf7QTModrcsRweHbupdPTY4oGMVoDZXm') + expect(res[0].peer.toB58String()).to.equal('QmWjxEGC7BthJrCf7QTModrcsRweHbupdPTY4oGMVoDZXm') + expect(scope.isDone()).to.equal(true) + done() + }) + }) + + it('handles an ip6 quic peer', (done) => { + const response = { 'Peers': [{ 'Addr': '/ip6/2001:8a0:7ac5:4201:3ac9:86ff:fe31:7095/udp/4001/quic', 'Peer': 'QmcgpsyWgH8Y8ajJz1Cu72KnS5uo2Aa2LpzU7kinSupNKC', 'Latency': '', 'Muxer': '', 'Streams': null }] } + + const scope = nock(apiUrl) + .post('/api/v0/swarm/peers') + .query(true) + .reply(200, response) + + ipfs.swarm.peers((err, res) => { + expect(err).to.not.exist() + expect(res).to.be.a('array') + expect(res.length).to.equal(1) + expect(res[0].error).to.not.exist() + expect(res[0].addr.toString()).to.equal(response.Peers[0].Addr) + expect(res[0].peer.toB58String()).to.equal(response.Peers[0].Peer) + expect(scope.isDone()).to.equal(true) + done() + }) + }) + + it('handles unvalidatable peer addr', (done) => { + const response = { 'Peers': [{ 'Addr': '/ip4/104.131.131.82/future-tech', 'Peer': 'QmcgpsyWgH8Y8ajJz1Cu72KnS5uo2Aa2LpzU7kinSupNKC', 'Latency': '', 'Muxer': '', 'Streams': null }] } + + const scope = nock(apiUrl) + .post('/api/v0/swarm/peers') + .query(true) + .reply(200, response) + + ipfs.swarm.peers((err, res) => { + expect(err).to.not.exist() + expect(res).to.be.a('array') + expect(res.length).to.equal(1) + expect(res[0].error).to.exist() + expect(res[0].rawPeerInfo).to.deep.equal(response.Peers[0]) + expect(scope.isDone()).to.equal(true) + done() + }) + }) + + it('handles an error response', (done) => { + const scope = nock(apiUrl) + .post('/api/v0/swarm/peers') + .query(true) + .replyWithError('something awful happened') + + ipfs.swarm.peers((err, res) => { + expect(err.message).to.equal('something awful happened') + expect(res).to.not.exist() + expect(scope.isDone()).to.equal(true) + done() + }) + }) +}) From 8c86ba3da884148cad8a7f714c591ac34ca9c3c4 Mon Sep 17 00:00:00 2001 From: Oli Evans Date: Wed, 7 Nov 2018 14:04:46 +0000 Subject: [PATCH 2/4] fix: ensure nock tests only run in node env License: MIT Signed-off-by: Oli Evans --- test/node.js | 1 + test/{swarm.spec.js => node/swarm.js} | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 test/node.js rename test/{swarm.spec.js => node/swarm.js} (99%) diff --git a/test/node.js b/test/node.js new file mode 100644 index 000000000..c85526c1c --- /dev/null +++ b/test/node.js @@ -0,0 +1 @@ +require('./node/swarm') diff --git a/test/swarm.spec.js b/test/node/swarm.js similarity index 99% rename from test/swarm.spec.js rename to test/node/swarm.js index f7a549418..bae8995f3 100644 --- a/test/swarm.spec.js +++ b/test/node/swarm.js @@ -7,7 +7,7 @@ const dirtyChai = require('dirty-chai') const expect = chai.expect chai.use(dirtyChai) -const IPFSApi = require('../src') +const IPFSApi = require('../../src') describe('.swarm.peers', function () { this.timeout(50 * 1000) // slow CI From c964aa438c7cfd403ddbb94267dce8c0c5f03024 Mon Sep 17 00:00:00 2001 From: Oli Evans Date: Wed, 7 Nov 2018 17:18:58 +0000 Subject: [PATCH 3/4] chore: fix lint errors in swarm.peers test License: MIT Signed-off-by: Oli Evans --- test/node/swarm.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/node/swarm.js b/test/node/swarm.js index bae8995f3..5ac36ca6b 100644 --- a/test/node/swarm.js +++ b/test/node/swarm.js @@ -16,7 +16,7 @@ describe('.swarm.peers', function () { const apiUrl = 'http://127.0.0.1:5001' it('handles a peer response', (done) => { - const response = { 'Peers': [{ 'Addr': '/ip4/104.131.131.82/tcp/4001', 'Peer': 'QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ', 'Latency': '', 'Muxer': '', 'Streams': null }] } + const response = { Peers: [{ Addr: '/ip4/104.131.131.82/tcp/4001', Peer: 'QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ', Latency: '', Muxer: '', Streams: null }] } const scope = nock(apiUrl) .post('/api/v0/swarm/peers') @@ -36,7 +36,7 @@ describe('.swarm.peers', function () { }) it('handles a go-ipfs <= 0.4.4 peer response', (done) => { - const response = { 'Strings': ['/ip4/73.109.217.59/tcp/49311/ipfs/QmWjxEGC7BthJrCf7QTModrcsRweHbupdPTY4oGMVoDZXm'] } + const response = { Strings: ['/ip4/73.109.217.59/tcp/49311/ipfs/QmWjxEGC7BthJrCf7QTModrcsRweHbupdPTY4oGMVoDZXm'] } const scope = nock(apiUrl) .post('/api/v0/swarm/peers') @@ -44,7 +44,6 @@ describe('.swarm.peers', function () { .reply(200, response) ipfs.swarm.peers((err, res) => { - console.log(res[0].rawPeerInfo) expect(err).to.not.exist() expect(res).to.be.a('array') expect(res.length).to.equal(1) @@ -57,7 +56,7 @@ describe('.swarm.peers', function () { }) it('handles an ip6 quic peer', (done) => { - const response = { 'Peers': [{ 'Addr': '/ip6/2001:8a0:7ac5:4201:3ac9:86ff:fe31:7095/udp/4001/quic', 'Peer': 'QmcgpsyWgH8Y8ajJz1Cu72KnS5uo2Aa2LpzU7kinSupNKC', 'Latency': '', 'Muxer': '', 'Streams': null }] } + const response = { Peers: [{ Addr: '/ip6/2001:8a0:7ac5:4201:3ac9:86ff:fe31:7095/udp/4001/quic', Peer: 'QmcgpsyWgH8Y8ajJz1Cu72KnS5uo2Aa2LpzU7kinSupNKC', Latency: '', Muxer: '', Streams: null }] } const scope = nock(apiUrl) .post('/api/v0/swarm/peers') @@ -77,7 +76,7 @@ describe('.swarm.peers', function () { }) it('handles unvalidatable peer addr', (done) => { - const response = { 'Peers': [{ 'Addr': '/ip4/104.131.131.82/future-tech', 'Peer': 'QmcgpsyWgH8Y8ajJz1Cu72KnS5uo2Aa2LpzU7kinSupNKC', 'Latency': '', 'Muxer': '', 'Streams': null }] } + const response = { Peers: [{ Addr: '/ip4/104.131.131.82/future-tech', Peer: 'QmcgpsyWgH8Y8ajJz1Cu72KnS5uo2Aa2LpzU7kinSupNKC', Latency: '', Muxer: '', Streams: null }] } const scope = nock(apiUrl) .post('/api/v0/swarm/peers') From b8817e7a138558efc895a9293a630de22c4f7edf Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 26 Nov 2018 11:15:42 +0000 Subject: [PATCH 4/4] chore: appease linter License: MIT Signed-off-by: Alan Shaw --- test/node.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/node.js b/test/node.js index c85526c1c..1082ea315 100644 --- a/test/node.js +++ b/test/node.js @@ -1 +1,2 @@ +'use strict' require('./node/swarm')