Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Commit

Permalink
refactor: swap joi-browser with superstruct (#1961)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hugomrdias authored and Alan Shaw committed Mar 28, 2019
1 parent 1c07779 commit 8fb5825
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 116 deletions.
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
144 changes: 84 additions & 60 deletions src/core/config.js
Original file line number Diff line number Diff line change
@@ -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 }
6 changes: 2 additions & 4 deletions test/core/bitswap.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']
}
})
}
Expand Down
51 changes: 1 addition & 50 deletions test/core/config.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
]

Expand All @@ -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 }
]

Expand Down Expand Up @@ -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 }
]

Expand Down Expand Up @@ -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 }
]

Expand Down Expand Up @@ -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 }
]

Expand All @@ -236,7 +187,7 @@ describe('config', () => {

it('should validate invalid libp2p', () => {
const cfgs = [
{ libp2p: { modules: 138 } },
{ libp2p: 'error' },
{ libp2p: 138 }
]

Expand Down

0 comments on commit 8fb5825

Please sign in to comment.