fix: handle peer-info validation errors (#887)

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 <oli@tableflip.io>

Author: olizilla

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.86.0",
     "ipfsd-ctl": "~0.40.0",
+    "nock": "^10.0.2",
     "pull-stream": "^3.6.9",
     "stream-equal": "^1.1.1"
   },

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
 }

test/node.js

@@ -0,0 +1,2 @@
+'use strict'
+require('./node/swarm')

test/node/swarm.js

@@ -0,0 +1,110 @@
+/* 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) => {
+      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()
+    })
+  })
+})