From dd603be7889556046ce5e5753ae0e79ab74e1ba6 Mon Sep 17 00:00:00 2001
From: victorbjelkholm <victorbjelkholm@gmail.com>
Date: Sun, 18 Feb 2018 18:39:29 +0100
Subject: [PATCH] Better validation of Multiaddrs in bootstrap command

mafmt handles both instances of multiaddr and multiaddrs in strings.

isMultiaddr only handles instances of multiaddr.

Demonstration:

```
> const ma = require('multiaddr')

> const mafmt = require('mafmt').IPFS.matches

> mafmt('/ip4/0.0.0.0/tcp/5001/ipfs/Qm')
true

> ma.isMultiaddr('/ip4/0.0.0.0/tcp/5001/ipfs/Qm')
false

> m = ma('/ip4/0.0.0.0/tcp/5001/ipfs/Qm')
<Multiaddr 0400000000061389a503020562 - /ip4/0.0.0.0/tcp/5001/ipfs/Qm>

> mafmt(m)
true

> ma.isMultiaddr(m)
true
```
---
 src/core/components/bootstrap.js | 29 +++++++++++++----------------
 test/core/bootstrap.spec.js      |  9 +++++++++
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/src/core/components/bootstrap.js b/src/core/components/bootstrap.js
index 559f8e05b3..3d816ac946 100644
--- a/src/core/components/bootstrap.js
+++ b/src/core/components/bootstrap.js
@@ -1,24 +1,21 @@
 'use strict'
 
 const defaultNodes = require('../runtime/config-nodejs.json').Bootstrap
-const Multiaddr = require('multiaddr')
+const isMultiaddr = require('mafmt').IPFS.matches
 const promisify = require('promisify-es6')
 
-function isValid (ma) {
-  if (typeof ma === 'string') {
-    try {
-      ma = new Multiaddr(ma)
-      return Boolean(ma)
-    } catch (err) {
-      return false
-    }
-  } else if (ma) {
-    return Multiaddr.isMultiaddr(ma)
-  } else {
+function isValidMultiaddr (ma) {
+  try {
+    return isMultiaddr(ma)
+  } catch (err) {
     return false
   }
 }
 
+function invalidMultiaddrError (ma) {
+  return new Error(`${ma} is not a valid Multiaddr`)
+}
+
 module.exports = function bootstrap (self) {
   return {
     list: promisify((callback) => {
@@ -35,8 +32,8 @@ module.exports = function bootstrap (self) {
         args = { default: false }
       }
 
-      if (multiaddr && !isValid(multiaddr)) {
-        return setImmediate(() => callback(new Error('Not valid multiaddr')))
+      if (multiaddr && !isValidMultiaddr(multiaddr)) {
+        return setImmediate(() => callback(invalidMultiaddrError(multiaddr)))
       }
 
       self._repo.config.get((err, config) => {
@@ -64,8 +61,8 @@ module.exports = function bootstrap (self) {
         callback = args
         args = {all: false}
       }
-      if (multiaddr && !isValid(multiaddr)) {
-        return setImmediate(() => callback(new Error('Not valid multiaddr')))
+      if (multiaddr && !isValidMultiaddr(multiaddr)) {
+        return setImmediate(() => callback(invalidMultiaddrError(multiaddr)))
       }
 
       self._repo.config.get((err, config) => {
diff --git a/test/core/bootstrap.spec.js b/test/core/bootstrap.spec.js
index 698af528b4..dd780ce8fd 100644
--- a/test/core/bootstrap.spec.js
+++ b/test/core/bootstrap.spec.js
@@ -115,4 +115,13 @@ describe('bootstrap', () => {
       })
     })
   })
+
+  it('fails if passing in a invalid multiaddr', (done) => {
+    node.bootstrap.add('/funky/invalid/multiaddr', (err, res) => {
+      expect(err).to.match(/not a valid Multiaddr/)
+      expect(err).to.match(/funky/)
+      expect(res).to.not.exist()
+      done()
+    })
+  })
 })