From c73bd2f68a57651e2b7a67b522f79556040963ae Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Wed, 19 Sep 2018 12:26:20 +0100 Subject: [PATCH] fix: allow null/undefined options (#1581) Options should be optional! Our API should be flexible enough to allow passing null or undefined in place of an options object. This PR fixes cases where we assume an options object has been passed. fixes: #1574 --- src/core/components/block.js | 4 ++ src/core/components/dag.js | 9 ++-- src/core/components/dht.js | 4 ++ src/core/components/dns.js | 2 + src/core/components/files.js | 6 +-- src/core/components/key.js | 1 + src/core/components/object.js | 6 +++ src/core/components/stats.js | 2 + src/core/components/swarm.js | 2 + src/core/components/version.js | 2 +- test/core/block.spec.js | 21 ++++++++ test/core/files.spec.js | 11 +++- test/core/object.spec.js | 93 +++++++++++++++++++++++++++++++++- test/core/stats.spec.js | 9 ++++ test/core/swarm.spec.js | 48 ++++++++++++++++++ 15 files changed, 211 insertions(+), 9 deletions(-) create mode 100644 test/core/swarm.spec.js diff --git a/src/core/components/block.js b/src/core/components/block.js index f350363a01..1e39880993 100644 --- a/src/core/components/block.js +++ b/src/core/components/block.js @@ -39,6 +39,8 @@ module.exports = function block (self) { options = {} } + options = options || {} + if (Array.isArray(block)) { return callback(new Error('Array is not supported')) } @@ -93,6 +95,8 @@ module.exports = function block (self) { options = {} } + options = options || {} + try { cid = cleanCid(cid) } catch (err) { diff --git a/src/core/components/dag.js b/src/core/components/dag.js index 317d6acb8a..5521591834 100644 --- a/src/core/components/dag.js +++ b/src/core/components/dag.js @@ -14,12 +14,15 @@ module.exports = function dag (self) { if (typeof options === 'function') { callback = options options = {} - } else if (options && options.cid && (options.format || options.hashAlg)) { + } + + options = options || {} + + if (options.cid && (options.format || options.hashAlg)) { return callback(new Error('Can\'t put dag node. Please provide either `cid` OR `format` and `hashAlg` options.')) - } else if (options && ((options.format && !options.hashAlg) || (!options.format && options.hashAlg))) { + } else if (((options.format && !options.hashAlg) || (!options.format && options.hashAlg))) { return callback(new Error('Can\'t put dag node. Please provide `format` AND `hashAlg` options.')) } - options = options || {} const optionDefaults = { format: 'dag-cbor', diff --git a/src/core/components/dht.js b/src/core/components/dht.js index 848b2e0621..9cfc91b7b4 100644 --- a/src/core/components/dht.js +++ b/src/core/components/dht.js @@ -28,6 +28,8 @@ module.exports = (self) => { options = {} } + options = options || {} + self._libp2pNode.dht.get(key, options.timeout, callback) }), @@ -134,6 +136,8 @@ module.exports = (self) => { options = {} } + options = options || {} + // ensure blocks are actually local every(keys, (key, cb) => { self._repo.blocks.has(key, cb) diff --git a/src/core/components/dns.js b/src/core/components/dns.js index 4895f01667..55d81f1c04 100644 --- a/src/core/components/dns.js +++ b/src/core/components/dns.js @@ -15,6 +15,8 @@ module.exports = () => { opts = {} } + opts = opts || {} + dns(domain, opts, callback) }) } diff --git a/src/core/components/files.js b/src/core/components/files.js index d575d893fa..47cf2d5abb 100644 --- a/src/core/components/files.js +++ b/src/core/components/files.js @@ -256,14 +256,14 @@ module.exports = function files (self) { return { add: (() => { - const add = promisify((data, options = {}, callback) => { + const add = promisify((data, options, callback) => { if (typeof options === 'function') { callback = options options = {} - } else if (!callback || typeof callback !== 'function') { - callback = noop } + options = options || {} + const ok = Buffer.isBuffer(data) || isStream.readable(data) || Array.isArray(data) || diff --git a/src/core/components/key.js b/src/core/components/key.js index b5bc5cd434..97bb2973a3 100644 --- a/src/core/components/key.js +++ b/src/core/components/key.js @@ -7,6 +7,7 @@ const promisify = require('promisify-es6') module.exports = function key (self) { return { gen: promisify((name, opts, callback) => { + opts = opts || {} self._keychain.createKey(name, opts.type, opts.size, callback) }), diff --git a/src/core/components/object.js b/src/core/components/object.js index d83deae62a..c0a3a82551 100644 --- a/src/core/components/object.js +++ b/src/core/components/object.js @@ -70,6 +70,8 @@ module.exports = function object (self) { options = {} } + options = options || {} + waterfall([ (cb) => { self.object.get(multihash, options, cb) @@ -151,6 +153,8 @@ module.exports = function object (self) { options = {} } + options = options || {} + const encoding = options.enc let node @@ -217,6 +221,8 @@ module.exports = function object (self) { options = {} } + options = options || {} + let mh, cid try { diff --git a/src/core/components/stats.js b/src/core/components/stats.js index 010a01431d..f67c2c8cd1 100644 --- a/src/core/components/stats.js +++ b/src/core/components/stats.js @@ -81,6 +81,8 @@ module.exports = function stats (self) { opts = {} } + opts = opts || {} + bandwidthStats(self, opts) .then((stats) => callback(null, stats)) .catch((err) => callback(err)) diff --git a/src/core/components/swarm.js b/src/core/components/swarm.js index c6c4d3073c..916f70cb49 100644 --- a/src/core/components/swarm.js +++ b/src/core/components/swarm.js @@ -13,6 +13,8 @@ module.exports = function swarm (self) { opts = {} } + opts = opts || {} + if (!self.isOnline()) { return callback(new Error(OFFLINE_ERROR)) } diff --git a/src/core/components/version.js b/src/core/components/version.js index 1afbc7a7fb..c9b663dd6d 100644 --- a/src/core/components/version.js +++ b/src/core/components/version.js @@ -13,7 +13,7 @@ module.exports = function version (self) { self.repo.version((err, repoVersion) => { if (err) { - callback(err) + return callback(err) } callback(null, { diff --git a/test/core/block.spec.js b/test/core/block.spec.js index fc47d3bf39..a02a381496 100644 --- a/test/core/block.spec.js +++ b/test/core/block.spec.js @@ -6,6 +6,7 @@ const chai = require('chai') const dirtyChai = require('dirty-chai') const expect = chai.expect chai.use(dirtyChai) +const hat = require('hat') const IPFSFactory = require('ipfsd-ctl') const IPFS = require('../../src/core') @@ -47,6 +48,15 @@ describe('block', () => { }) }) + describe('put', () => { + it('should not error when passed null options', (done) => { + ipfs.block.put(Buffer.from(hat()), null, (err) => { + expect(err).to.not.exist() + done() + }) + }) + }) + describe('rm', () => { it('should callback with error for invalid CID input', (done) => { ipfs.block.rm('INVALID CID', (err) => { @@ -65,5 +75,16 @@ describe('block', () => { done() }) }) + + it('should not error when passed null options', (done) => { + ipfs.block.put(Buffer.from(hat()), (err, block) => { + expect(err).to.not.exist() + + ipfs.block.stat(block.cid, null, (err) => { + expect(err).to.not.exist() + done() + }) + }) + }) }) }) diff --git a/test/core/files.spec.js b/test/core/files.spec.js index e34ecbed2c..d90bddd805 100644 --- a/test/core/files.spec.js +++ b/test/core/files.spec.js @@ -6,7 +6,7 @@ const chai = require('chai') const dirtyChai = require('dirty-chai') const expect = chai.expect chai.use(dirtyChai) - +const hat = require('hat') const pull = require('pull-stream') const IPFSFactory = require('ipfsd-ctl') const IPFS = require('../../src/core') @@ -75,4 +75,13 @@ describe('files', () => { ) }) }) + + describe('add', () => { + it('should not error when passed null options', (done) => { + ipfs.files.add(Buffer.from(hat()), null, (err) => { + expect(err).to.not.exist() + done() + }) + }) + }) }) diff --git a/test/core/object.spec.js b/test/core/object.spec.js index d126bb94b6..d71c258190 100644 --- a/test/core/object.spec.js +++ b/test/core/object.spec.js @@ -6,8 +6,9 @@ const chai = require('chai') const dirtyChai = require('dirty-chai') const expect = chai.expect chai.use(dirtyChai) - +const hat = require('hat') const IPFSFactory = require('ipfsd-ctl') +const auto = require('async/auto') const IPFS = require('../../src/core') describe('object', () => { @@ -45,6 +46,17 @@ describe('object', () => { done() }) }) + + it('should not error when passed null options', (done) => { + ipfs.object.put(Buffer.from(hat()), (err, dagNode) => { + expect(err).to.not.exist() + + ipfs.object.get(dagNode.multihash, null, (err) => { + expect(err).to.not.exist() + done() + }) + }) + }) }) describe('put', () => { @@ -55,5 +67,84 @@ describe('object', () => { done() }) }) + + it('should not error when passed null options', (done) => { + ipfs.object.put(Buffer.from(hat()), null, (err) => { + expect(err).to.not.exist() + done() + }) + }) + }) + + describe('patch.addLink', () => { + it('should not error when passed null options', (done) => { + auto({ + a: (cb) => ipfs.object.put(Buffer.from(hat()), cb), + b: (cb) => ipfs.object.put(Buffer.from(hat()), cb) + }, (err, nodes) => { + expect(err).to.not.exist() + + const link = { + name: nodes.b.name, + multihash: nodes.b.multihash, + size: nodes.b.size + } + + ipfs.object.patch.addLink(nodes.a.multihash, link, null, (err) => { + expect(err).to.not.exist() + done() + }) + }) + }) + }) + + describe('patch.rmLink', () => { + it('should not error when passed null options', (done) => { + auto({ + nodeA: (cb) => ipfs.object.put(Buffer.from(hat()), cb), + nodeB: (cb) => ipfs.object.put(Buffer.from(hat()), cb), + nodeAWithLink: ['nodeA', 'nodeB', (res, cb) => { + ipfs.object.patch.addLink(res.nodeA.multihash, { + name: res.nodeB.name, + multihash: res.nodeB.multihash, + size: res.nodeB.size + }, cb) + }] + }, (err, res) => { + expect(err).to.not.exist() + + const link = res.nodeAWithLink.links[0] + ipfs.object.patch.rmLink(res.nodeAWithLink.multihash, link, null, (err) => { + expect(err).to.not.exist() + done() + }) + }) + }) + }) + + describe('patch.appendData', () => { + it('should not error when passed null options', (done) => { + ipfs.object.put(Buffer.from(hat()), null, (err, dagNode) => { + expect(err).to.not.exist() + + ipfs.object.patch.appendData(dagNode.multihash, Buffer.from(hat()), null, (err) => { + expect(err).to.not.exist() + done() + }) + }) + }) + }) + + describe('patch.setData', () => { + it('should not error when passed null options', (done) => { + ipfs.object.put(Buffer.from(hat()), null, (err, dagNode) => { + expect(err).to.not.exist() + + ipfs.object.patch.setData(dagNode.multihash, Buffer.from(hat()), null, (err) => { + expect(err).to.not.exist() + done() + }) + }) + }) }) }) diff --git a/test/core/stats.spec.js b/test/core/stats.spec.js index 15e872efaf..d4a0a8dd7c 100644 --- a/test/core/stats.spec.js +++ b/test/core/stats.spec.js @@ -50,4 +50,13 @@ describe('stats', () => { ) }) }) + + describe('bw', () => { + it('should not error when passed null options', (done) => { + ipfs.stats.bw(null, (err) => { + expect(err).to.not.exist() + done() + }) + }) + }) }) diff --git a/test/core/swarm.spec.js b/test/core/swarm.spec.js new file mode 100644 index 0000000000..abf92fa3cc --- /dev/null +++ b/test/core/swarm.spec.js @@ -0,0 +1,48 @@ +/* eslint max-nested-callbacks: ["error", 8] */ +/* eslint-env mocha */ +'use strict' + +const chai = require('chai') +const dirtyChai = require('dirty-chai') +const expect = chai.expect +chai.use(dirtyChai) + +const IPFSFactory = require('ipfsd-ctl') +const IPFS = require('../../src/core') + +describe('swarm', () => { + let ipfsd, ipfs + + before(function (done) { + this.timeout(20 * 1000) + + const factory = IPFSFactory.create({ type: 'proc' }) + + factory.spawn({ + exec: IPFS, + initOptions: { bits: 512 } + }, (err, _ipfsd) => { + expect(err).to.not.exist() + ipfsd = _ipfsd + ipfs = _ipfsd.api + done() + }) + }) + + after((done) => { + if (ipfsd) { + ipfsd.stop(done) + } else { + done() + } + }) + + describe('peers', () => { + it('should not error when passed null options', (done) => { + ipfs.swarm.peers(null, (err) => { + expect(err).to.not.exist() + done() + }) + }) + }) +})