Merge pull request #16 from libp2p/fix/multiaddr

fix: ensure all addresses are multiaddr

Author: daviddias

package.json

@@ -40,11 +40,11 @@
     "aegir": "^8.0.0",
     "buffer-loader": "0.0.1",
     "chai": "^3.5.0",
-    "multiaddr": "^2.0.2",
-    "pre-commit": "^1.1.2"
+    "pre-commit": "^1.1.3"
   },
   "dependencies": {
-    "babel-runtime": "^6.6.1",
+    "babel-runtime": "^6.11.6",
+    "multiaddr": "^2.0.3",
     "peer-id": "^0.7.0"
   },
   "contributors": [
@@ -54,4 +54,4 @@
     "Stephen Whitmore <stephen.whitmore@gmail.com>",
     "dignifiedquire <dignifiedquire@gmail.com>"
   ]
-}
+}

src/index.js

@@ -1,9 +1,18 @@
 'use strict'
 
 const Id = require('peer-id')
+const multiaddr = require('multiaddr')
 
 exports = module.exports = Peer
 
+function ensureMultiaddr (addr) {
+  if (multiaddr.isMultiaddr(addr)) {
+    return addr
+  }
+
+  return multiaddr(addr)
+}
+
 // Peer represents a peer on the IPFS network
 function Peer (peerId) {
   if (!(this instanceof Peer)) {
@@ -21,37 +30,43 @@ function Peer (peerId) {
 
   this.multiaddr = {}
 
-  this.multiaddr.add = (multiaddr) => {
+  this.multiaddr.add = (addr) => {
+    addr = ensureMultiaddr(addr)
+
     var exists = false
     this.multiaddrs.some((m, i) => {
-      if (m.toString() === multiaddr.toString()) {
+      if (m.equals(addr)) {
         exists = true
         return true
       }
     })
     if (!exists) {
-      this.multiaddrs.push(multiaddr)
+      this.multiaddrs.push(addr)
     }
   }
 
   // to prevent multiaddr explosion
-  this.multiaddr.addSafe = (multiaddr) => {
+  this.multiaddr.addSafe = (addr) => {
+    addr = ensureMultiaddr(addr)
+
     var check = false
     observedMultiaddrs.some((m, i) => {
-      if (m.toString() === multiaddr.toString()) {
-        this.multiaddr.add(multiaddr)
+      if (m.equals(addr)) {
+        this.multiaddr.add(addr)
         observedMultiaddrs.splice(i, 1)
         check = true
       }
     })
     if (!check) {
-      observedMultiaddrs.push(multiaddr)
+      observedMultiaddrs.push(addr)
     }
   }
 
-  this.multiaddr.rm = (multiaddr) => {
+  this.multiaddr.rm = (addr) => {
+    addr = ensureMultiaddr(addr)
+
     this.multiaddrs.some((m, i) => {
-      if (m.toString() === multiaddr.toString()) {
+      if (m.equals(addr)) {
         this.multiaddrs.splice(i, 1)
         return true
       }

test/peer-info.spec.js

@@ -9,61 +9,63 @@ const PeerInfo = require('../src')
 describe('peer-info', function () {
   this.timeout(20000)
 
-  it('create with Id', (done) => {
+  it('create with Id', () => {
     const id = PeerId.create()
     const pi = new PeerInfo(id)
     expect(pi).to.exist
     expect(pi.id).to.exist
     expect(pi.id).to.deep.equal(id)
-    done()
   })
 
-  it('create without passing an Id', (done) => {
+  it('create without passing an Id', () => {
     const pi = new PeerInfo()
     expect(pi).to.exist
     expect(pi.id).to.exist
-    done()
   })
 
-  it('create without "new"', (done) => {
+  it('create without "new"', () => {
     const pi = PeerInfo()
     expect(pi).to.exist
     expect(pi.id).to.exist
-    done()
   })
 
-  it('add multiaddr', (done) => {
+  it('add multiaddr', () => {
     const pi = new PeerInfo()
     expect(pi).to.exist
     const mh = Multiaddr('/ip4/127.0.0.1/tcp/5001')
     pi.multiaddr.add(mh)
     expect(pi.multiaddrs.length).to.equal(1)
-    done()
   })
 
-  it('add repeated multiaddr', (done) => {
+  it('add multiaddr that are buffers', () => {
+    const pi = new PeerInfo()
+    expect(pi).to.exist
+    const mh = Multiaddr('/ip4/127.0.0.1/tcp/5001')
+    pi.multiaddr.add(mh.buffer)
+    expect(pi.multiaddrs[0] instanceof Multiaddr).to.equal(true)
+  })
+
+  it('add repeated multiaddr', () => {
     const pi = new PeerInfo()
     expect(pi).to.exist
     const mh = Multiaddr('/ip4/127.0.0.1/tcp/5001')
     pi.multiaddr.add(mh)
     expect(pi.multiaddrs.length).to.equal(1)
     pi.multiaddr.add(mh)
     expect(pi.multiaddrs.length).to.equal(1)
-    done()
   })
 
-  it('rm multiaddr', (done) => {
+  it('rm multiaddr', () => {
     const pi = new PeerInfo()
     expect(pi).to.exist
     const mh = Multiaddr('/ip4/127.0.0.1/tcp/5001')
     pi.multiaddr.add(mh)
     expect(pi.multiaddrs.length).to.equal(1)
     pi.multiaddr.rm(mh)
     expect(pi.multiaddrs.length).to.equal(0)
-    done()
   })
 
-  it('addSafe - avoid multiaddr explosion', (done) => {
+  it('addSafe - avoid multiaddr explosion', () => {
     const pi = new PeerInfo()
     expect(pi).to.exist
     const mh = Multiaddr('/ip4/127.0.0.1/tcp/5001')
@@ -76,10 +78,18 @@ describe('peer-info', function () {
     pi.multiaddr.addSafe(mh2)
     pi.multiaddr.addSafe(mh3)
     expect(pi.multiaddrs.length).to.equal(1)
-    done()
   })
 
-  it('replace multiaddr', (done) => {
+  it('addSafe - multiaddr that are buffers', () => {
+    const pi = new PeerInfo()
+    expect(pi).to.exist
+    const mh = Multiaddr('/ip4/127.0.0.1/tcp/5001')
+    pi.multiaddr.addSafe(mh.buffer)
+    pi.multiaddr.addSafe(mh.buffer)
+    expect(pi.multiaddrs[0] instanceof Multiaddr).to.equal(true)
+  })
+
+  it('replace multiaddr', () => {
     const pi = new PeerInfo()
     expect(pi).to.exist
     const mh1 = Multiaddr('/ip4/127.0.0.1/tcp/5001')
@@ -102,11 +112,9 @@ describe('peer-info', function () {
     pi.multiaddr.replace(old, fresh)
 
     expect(pi.multiaddrs.length).to.equal(4)
-
-    done()
   })
 
-  it('replace multiaddr (no arrays)', (done) => {
+  it('replace multiaddr (no arrays)', () => {
     const pi = new PeerInfo()
     expect(pi).to.exist
     const mh1 = Multiaddr('/ip4/127.0.0.1/tcp/5001')
@@ -128,7 +136,5 @@ describe('peer-info', function () {
     pi.multiaddr.replace(old, fresh)
 
     expect(pi.multiaddrs.length).to.equal(4)
-
-    done()
   })
 })