refactor: swap joi-browser with superstruct (#1961)

Relay config is removed because libp2p already validates it. A bug was detected and fixed because the validation is stricter. This should improve the overall bundle size and safety handling the config.

BREAKING CHANGE: Constructor config validation is now a bit more strict - it does not allow `null` values or unknown properties.

Author: hugomrdias

package.json

@@ -125,8 +125,6 @@
     "is-stream": "^1.1.0",
     "iso-url": "~0.4.6",
     "joi": "^14.3.0",
-    "joi-browser": "^13.4.0",
-    "joi-multiaddr": "^4.0.0",
     "just-flatten-it": "^2.1.0",
     "just-safe-set": "^2.1.0",
     "libp2p": "~0.25.0-rc.5",
@@ -172,6 +170,7 @@
     "readable-stream": "^3.1.1",
     "receptacle": "^1.3.2",
     "stream-to-pull-stream": "^1.7.3",
+    "superstruct": "~0.6.0",
     "tar-stream": "^2.0.0",
     "temp": "~0.9.0",
     "update-notifier": "^2.5.0",

src/core/config.js

@@ -1,62 +1,86 @@
 'use strict'
 
-const Joi = require('joi').extend(require('joi-multiaddr'))
-
-const schema = Joi.object().keys({
-  repo: Joi.alternatives().try(
-    Joi.object(), // TODO: schema for IPFS repo
-    Joi.string()
-  ).allow(null),
-  repoOwner: Joi.boolean().default(true),
-  preload: Joi.object().keys({
-    enabled: Joi.boolean().default(true),
-    addresses: Joi.array().items(Joi.multiaddr().options({ convert: false })),
-    interval: Joi.number().integer().default(30 * 1000)
-  }).allow(null),
-  init: Joi.alternatives().try(
-    Joi.boolean(),
-    Joi.object().keys({ bits: Joi.number().integer() })
-  ).allow(null),
-  start: Joi.boolean(),
-  offline: Joi.boolean(),
-  pass: Joi.string().allow(''),
-  relay: Joi.object().keys({
-    enabled: Joi.boolean(),
-    hop: Joi.object().keys({
-      enabled: Joi.boolean(),
-      active: Joi.boolean()
-    }).allow(null)
-  }).allow(null),
-  EXPERIMENTAL: Joi.object().keys({
-    pubsub: Joi.boolean(),
-    ipnsPubsub: Joi.boolean(),
-    sharding: Joi.boolean(),
-    dht: Joi.boolean()
-  }).allow(null),
-  connectionManager: Joi.object().allow(null),
-  config: Joi.object().keys({
-    Addresses: Joi.object().keys({
-      Swarm: Joi.array().items(Joi.multiaddr().options({ convert: false })),
-      API: Joi.multiaddr().options({ convert: false }),
-      Gateway: Joi.multiaddr().options({ convert: false })
-    }).allow(null),
-    Discovery: Joi.object().keys({
-      MDNS: Joi.object().keys({
-        Enabled: Joi.boolean(),
-        Interval: Joi.number().integer()
-      }).allow(null),
-      webRTCStar: Joi.object().keys({
-        Enabled: Joi.boolean()
-      }).allow(null)
-    }).allow(null),
-    Bootstrap: Joi.array().items(Joi.multiaddr().IPFS().options({ convert: false }))
-  }).allow(null),
-  libp2p: Joi.alternatives().try(
-    Joi.func(),
-    Joi.object().keys({
-      modules: Joi.object().allow(null) // TODO: schemas for libp2p modules?
-    })
-  ).allow(null)
-}).options({ allowUnknown: true })
-
-module.exports.validate = (config) => Joi.attempt(config, schema)
+const Multiaddr = require('multiaddr')
+const mafmt = require('mafmt')
+const { struct, superstruct } = require('superstruct')
+
+const { optional, union } = struct
+const s = superstruct({
+  types: {
+    multiaddr: v => {
+      if (v === null) {
+        return `multiaddr invalid, value must be a string, Buffer, or another Multiaddr got ${v}`
+      }
+
+      try {
+        Multiaddr(v)
+      } catch (err) {
+        return `multiaddr invalid, ${err.message}`
+      }
+
+      return true
+    },
+    'multiaddr-ipfs': v => mafmt.IPFS.matches(v) ? true : `multiaddr IPFS invalid`
+  }
+})
+
+const configSchema = s({
+  repo: optional(s('object|string')),
+  repoOwner: 'boolean?',
+  preload: s({
+    enabled: 'boolean?',
+    addresses: optional(s(['multiaddr'])),
+    interval: 'number?'
+  }, { enabled: true, interval: 30 * 1000 }),
+  init: optional(union(['boolean', s({
+    bits: 'number?',
+    emptyRepo: 'boolean?',
+    privateKey: optional(s('object|string')), // object should be a custom type for PeerId using 'kind-of'
+    pass: 'string?'
+  })])),
+  start: 'boolean?',
+  offline: 'boolean?',
+  pass: 'string?',
+  silent: 'boolean?',
+  relay: 'object?', // relay validates in libp2p
+  EXPERIMENTAL: optional(s({
+    pubsub: 'boolean?',
+    ipnsPubsub: 'boolean?',
+    sharding: 'boolean?',
+    dht: 'boolean?'
+  })),
+  connectionManager: 'object?',
+  config: optional(s({
+    API: 'object?',
+    Addresses: optional(s({
+      Swarm: optional(s(['multiaddr'])),
+      API: 'multiaddr?',
+      Gateway: 'multiaddr'
+    })),
+    Discovery: optional(s({
+      MDNS: optional(s({
+        Enabled: 'boolean?',
+        Interval: 'number?'
+      })),
+      webRTCStar: optional(s({
+        Enabled: 'boolean?'
+      }))
+    })),
+    Bootstrap: optional(s(['multiaddr-ipfs']))
+  })),
+  libp2p: optional(union(['function', 'object'])) // libp2p validates this
+}, {
+  repoOwner: true
+})
+
+const validate = (opts) => {
+  const [err, options] = configSchema.validate(opts)
+
+  if (err) {
+    throw err
+  }
+
+  return options
+}
+
+module.exports = { validate }

test/core/bitswap.spec.js

@@ -113,10 +113,8 @@ describe('bitswap', function () {
 
     if (isNode) {
       config = Object.assign({}, config, {
-        config: {
-          Addresses: {
-            Swarm: ['/ip4/127.0.0.1/tcp/0']
-          }
+        Addresses: {
+          Swarm: ['/ip4/127.0.0.1/tcp/0']
         }
       })
     }

test/core/config.spec.js

@@ -19,16 +19,10 @@ describe('config', () => {
     expect(() => config.validate(cfg)).to.not.throw()
   })
 
-  it('should allow unknown key at root', () => {
-    const cfg = { [`${Date.now()}`]: 'test' }
-    expect(() => config.validate(cfg)).to.not.throw()
-  })
-
   it('should validate valid repo', () => {
     const cfgs = [
       { repo: { unknown: 'value' } },
       { repo: '/path/to-repo' },
-      { repo: null },
       { repo: undefined }
     ]
 
@@ -46,10 +40,8 @@ describe('config', () => {
   it('should validate valid init', () => {
     const cfgs = [
       { init: { bits: 138 } },
-      { init: { bits: 138, unknown: 'value' } },
       { init: true },
       { init: false },
-      { init: null },
       { init: undefined }
     ]
 
@@ -104,36 +96,10 @@ describe('config', () => {
     cfgs.forEach(cfg => expect(() => config.validate(cfg)).to.throw())
   })
 
-  it('should validate valid relay', () => {
-    const cfgs = [
-      { relay: { enabled: true, hop: { enabled: true } } },
-      { relay: { enabled: false, hop: { enabled: false } } },
-      { relay: { enabled: false, hop: null } },
-      { relay: { enabled: false } },
-      { relay: null },
-      { relay: undefined }
-    ]
-
-    cfgs.forEach(cfg => expect(() => config.validate(cfg)).to.not.throw())
-  })
-
-  it('should validate invalid relay', () => {
-    const cfgs = [
-      { relay: 138 },
-      { relay: { enabled: 138 } },
-      { relay: { enabled: true, hop: 138 } },
-      { relay: { enabled: true, hop: { enabled: 138 } } }
-    ]
-
-    cfgs.forEach(cfg => expect(() => config.validate(cfg)).to.throw())
-  })
-
   it('should validate valid EXPERIMENTAL', () => {
     const cfgs = [
       { EXPERIMENTAL: { pubsub: true, dht: true, sharding: true } },
       { EXPERIMENTAL: { pubsub: false, dht: false, sharding: false } },
-      { EXPERIMENTAL: { unknown: 'value' } },
-      { EXPERIMENTAL: null },
       { EXPERIMENTAL: undefined }
     ]
 
@@ -162,32 +128,22 @@ describe('config', () => {
       { config: { Addresses: { Gateway: '/ip4/127.0.0.1/tcp/9090' } } },
       { config: { Addresses: { Gateway: undefined } } },
 
-      { config: { Addresses: { unknown: 'value' } } },
-      { config: { Addresses: null } },
       { config: { Addresses: undefined } },
 
       { config: { Discovery: { MDNS: { Enabled: true } } } },
       { config: { Discovery: { MDNS: { Enabled: false } } } },
       { config: { Discovery: { MDNS: { Interval: 138 } } } },
-      { config: { Discovery: { MDNS: { unknown: 'value' } } } },
-      { config: { Discovery: { MDNS: null } } },
       { config: { Discovery: { MDNS: undefined } } },
 
       { config: { Discovery: { webRTCStar: { Enabled: true } } } },
       { config: { Discovery: { webRTCStar: { Enabled: false } } } },
-      { config: { Discovery: { webRTCStar: { unknown: 'value' } } } },
-      { config: { Discovery: { webRTCStar: null } } },
       { config: { Discovery: { webRTCStar: undefined } } },
 
-      { config: { Discovery: { unknown: 'value' } } },
-      { config: { Discovery: null } },
       { config: { Discovery: undefined } },
 
       { config: { Bootstrap: ['/ip4/104.236.176.52/tcp/4001/ipfs/QmSoLnSGccFuZQJzRadHn95W2CrSFmZuTdDWP8HXaHca9z'] } },
       { config: { Bootstrap: [] } },
 
-      { config: { unknown: 'value' } },
-      { config: null },
       { config: undefined }
     ]
 
@@ -222,12 +178,7 @@ describe('config', () => {
   it('should validate valid libp2p', () => {
     const cfgs = [
       { libp2p: { modules: {} } },
-      { libp2p: { modules: { unknown: 'value' } } },
-      { libp2p: { modules: null } },
-      { libp2p: { modules: undefined } },
-      { libp2p: { unknown: 'value' } },
       { libp2p: () => {} },
-      { libp2p: null },
       { libp2p: undefined }
     ]
 
@@ -236,7 +187,7 @@ describe('config', () => {
 
   it('should validate invalid libp2p', () => {
     const cfgs = [
-      { libp2p: { modules: 138 } },
+      { libp2p: 'error' },
       { libp2p: 138 }
     ]