From 8fb5825bfc2384f4088a67991e3f2f8cdd808182 Mon Sep 17 00:00:00 2001 From: Hugo Dias Date: Thu, 28 Mar 2019 19:57:06 +0000 Subject: [PATCH] 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. --- package.json | 3 +- src/core/config.js | 144 ++++++++++++++++++++++---------------- test/core/bitswap.spec.js | 6 +- test/core/config.spec.js | 51 +------------- 4 files changed, 88 insertions(+), 116 deletions(-) diff --git a/package.json b/package.json index 5d1c7a45c9..602dbc096f 100644 --- a/package.json +++ b/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", diff --git a/src/core/config.js b/src/core/config.js index c3b9e892a4..7848e69f0e 100644 --- a/src/core/config.js +++ b/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 } diff --git a/test/core/bitswap.spec.js b/test/core/bitswap.spec.js index 9540aba53e..b3261d4d88 100644 --- a/test/core/bitswap.spec.js +++ b/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'] } }) } diff --git a/test/core/config.spec.js b/test/core/config.spec.js index 3250c5b9d4..4ce3ffab05 100644 --- a/test/core/config.spec.js +++ b/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 } ]