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

Author: Alan Shaw

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) {

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',

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)

src/core/components/dns.js

@@ -15,6 +15,8 @@ module.exports = () => {
       opts = {}
     }
 
+    opts = opts || {}
+
     dns(domain, opts, callback)
   })
 }

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) ||

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)
     }),
 

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 {

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))

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))
       }

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, {

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()
+        })
+      })
+    })
   })
 })

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()
+      })
+    })
+  })
 })

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()
+        })
+      })
+    })
   })
 })

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()
+      })
+    })
+  })
 })

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()
+      })
+    })
+  })
+})