From cd682c21dd0d2e5711437784da1bfd2c2154e7fe Mon Sep 17 00:00:00 2001 From: Oli Evans Date: Fri, 25 Jan 2019 13:30:17 +0000 Subject: [PATCH 1/8] fix: throw on invalid multiaddr to constructor - dont assume that invalid multiaddrs are valid hosts - if multiaddrOrHost starts with a /, assume its a multiaddr - throw if that multiaddr is invalid. Previously, we'd try and use invalid multiaddrs as the host, which would lead to requests like http:///dnsaddr/foobar.com:5001/api License: MIT Signed-off-by: Oli Evans --- src/index.js | 14 +++++++------- test/constructor.spec.js | 8 ++++++++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/index.js b/src/index.js index 5643084d0..0e38c4823 100644 --- a/src/index.js +++ b/src/index.js @@ -8,13 +8,13 @@ const sendRequest = require('./utils/send-request') function ipfsClient (hostOrMultiaddr, port, opts) { const config = getConfig() - - try { - const maddr = multiaddr(hostOrMultiaddr).nodeAddress() - config.host = maddr.address - config.port = maddr.port - } catch (e) { - if (typeof hostOrMultiaddr === 'string') { + if (typeof hostOrMultiaddr === 'string') { + if (hostOrMultiaddr[0] === '/') { + // throws if multiaddr is malformed or can't be converted to a nodeAddress + const maddr = multiaddr(hostOrMultiaddr).nodeAddress() + config.host = maddr.address + config.port = maddr.port + } else { config.host = hostOrMultiaddr config.port = port && typeof port !== 'object' ? port : config.port } diff --git a/test/constructor.spec.js b/test/constructor.spec.js index e2ec239f0..a4777bddb 100644 --- a/test/constructor.spec.js +++ b/test/constructor.spec.js @@ -74,5 +74,13 @@ describe('ipfs-http-client constructor tests', () => { clientWorks(ipfsClient(splitted[2], splitted[4], { protocol: 'http' }), done) }) + + it('error in invalid mutliaddr', () => { + const splitted = apiAddr.split('/') + + expect(() => ipfsClient('/dns4')).to.throw('invalid address') + expect(() => ipfsClient('/hello')).to.throw('no protocol with name') + expect(() => ipfsClient('/dns4/ipfs.io')).to.throw('multiaddr must have a valid format') + }) }) }) From 574a54f38ded8bcfb2fdb046322260bf106a131a Mon Sep 17 00:00:00 2001 From: Oli Evans Date: Fri, 25 Jan 2019 14:59:42 +0000 Subject: [PATCH 2/8] chore: fix linting. remove unused variable. License: MIT Signed-off-by: Oli Evans --- test/constructor.spec.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/constructor.spec.js b/test/constructor.spec.js index a4777bddb..d875ac880 100644 --- a/test/constructor.spec.js +++ b/test/constructor.spec.js @@ -76,8 +76,6 @@ describe('ipfs-http-client constructor tests', () => { }) it('error in invalid mutliaddr', () => { - const splitted = apiAddr.split('/') - expect(() => ipfsClient('/dns4')).to.throw('invalid address') expect(() => ipfsClient('/hello')).to.throw('no protocol with name') expect(() => ipfsClient('/dns4/ipfs.io')).to.throw('multiaddr must have a valid format') From 75d76a749b2d1a0d5c12c0cbc2c1c11901229772 Mon Sep 17 00:00:00 2001 From: Oli Evans Date: Mon, 28 Jan 2019 09:52:37 +0000 Subject: [PATCH 3/8] fix: refactor constructor arg parsing - explicitly handle all the cases we expect to be passed - convert each param into an object so we can merge them - presedence is right to left as with Object.assign License: MIT Signed-off-by: Oli Evans --- src/index.js | 50 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/src/index.js b/src/index.js index 0e38c4823..a7032fa40 100644 --- a/src/index.js +++ b/src/index.js @@ -7,35 +7,36 @@ const getConfig = require('./utils/default-config') const sendRequest = require('./utils/send-request') function ipfsClient (hostOrMultiaddr, port, opts) { - const config = getConfig() - if (typeof hostOrMultiaddr === 'string') { + // convert all three params to objects that we can merge. + let hostAndPort = {} + + if (hostOrMultiaddr.host) { + hostAndPort = hostOrMultiaddr + + } else if (multiaddr.isMultiaddr(hostOrMultiaddr)) { + hostAndPort = toHostAndPort(hostOrMultiaddr) + + } else if (typeof hostOrMultiaddr === 'string') { if (hostOrMultiaddr[0] === '/') { // throws if multiaddr is malformed or can't be converted to a nodeAddress - const maddr = multiaddr(hostOrMultiaddr).nodeAddress() - config.host = maddr.address - config.port = maddr.port + hostAndPort = toHostAndPort(multiaddr(hostOrMultiaddr)) } else { - config.host = hostOrMultiaddr - config.port = port && typeof port !== 'object' ? port : config.port + // hostAndPort is domain or ip address as a string + hostAndPort = hostOrMultiaddr } - } - let lastIndex = arguments.length - while (!opts && lastIndex-- > 0) { - opts = arguments[lastIndex] - if (opts) break + // autoconfigure host and port in browser + } else if (!hostOrMultiaddr && typeof self !== 'undefined') { + const split = self.location.host.split(':') + hostAndPort.host = split[0] + hostAndPort.port = split[1] } - Object.assign(config, opts) - - // autoconfigure in browser - if (!config.host && - typeof self !== 'undefined') { - const split = self.location.host.split(':') - config.host = split[0] - config.port = split[1] + if (port && typeof port !== 'object') { + port = { port: port } } + const config = Object.assign(getConfig(), hostAndPort, port, opts) const requestAPI = sendRequest(config) const cmds = loadCommands(requestAPI, config) cmds.send = requestAPI @@ -44,4 +45,13 @@ function ipfsClient (hostOrMultiaddr, port, opts) { return cmds } +// throws if multiaddr can't be converted to a nodeAddress +function toHostAndPort (multiaddr) { + const nodeAddr = multiaddr.nodeAddress() + return { + host: nodeAddr.address, + port: nodeAddr.port + } +} + module.exports = ipfsClient From 0ebb098cfd65b5953f0ae47a53cd5b1b92e6244a Mon Sep 17 00:00:00 2001 From: Oli Evans Date: Mon, 28 Jan 2019 11:01:36 +0000 Subject: [PATCH 4/8] fix: improve constructor param handling tests - ensure explicitly passed parameters are used; dont pass in the default values and assume the are being set correctly TODO: needs https://github.com/ipfs/js-ipfs-http-client/pull/935 to expose the protocol and api-path properties License: MIT Signed-off-by: Oli Evans --- src/index.js | 22 ++++--- test/constructor.spec.js | 136 +++++++++++++++++++++++++-------------- 2 files changed, 101 insertions(+), 57 deletions(-) diff --git a/src/index.js b/src/index.js index a7032fa40..e7ea6f01b 100644 --- a/src/index.js +++ b/src/index.js @@ -10,26 +10,28 @@ function ipfsClient (hostOrMultiaddr, port, opts) { // convert all three params to objects that we can merge. let hostAndPort = {} - if (hostOrMultiaddr.host) { - hostAndPort = hostOrMultiaddr + if (!hostOrMultiaddr) { + // autoconfigure host and port in browser + if (typeof self !== 'undefined') { + const split = self.location.host.split(':') + hostAndPort.host = split[0] + hostAndPort.port = split[1] + } } else if (multiaddr.isMultiaddr(hostOrMultiaddr)) { hostAndPort = toHostAndPort(hostOrMultiaddr) + } else if (typeof hostOrMultiaddr === 'object') { + hostAndPort = hostOrMultiaddr + } else if (typeof hostOrMultiaddr === 'string') { if (hostOrMultiaddr[0] === '/') { // throws if multiaddr is malformed or can't be converted to a nodeAddress hostAndPort = toHostAndPort(multiaddr(hostOrMultiaddr)) } else { - // hostAndPort is domain or ip address as a string - hostAndPort = hostOrMultiaddr + // hostOrMultiaddr is domain or ip address as a string + hostAndPort.host = hostOrMultiaddr } - - // autoconfigure host and port in browser - } else if (!hostOrMultiaddr && typeof self !== 'undefined') { - const split = self.location.host.split(':') - hostAndPort.host = split[0] - hostAndPort.port = split[1] } if (port && typeof port !== 'object') { diff --git a/test/constructor.spec.js b/test/constructor.spec.js index d875ac880..e04ef4d71 100644 --- a/test/constructor.spec.js +++ b/test/constructor.spec.js @@ -1,6 +1,7 @@ /* eslint-env mocha */ 'use strict' +const multiaddr = require('multiaddr') const chai = require('chai') const dirtyChai = require('dirty-chai') const expect = chai.expect @@ -9,18 +10,73 @@ chai.use(dirtyChai) const f = require('./utils/factory') const ipfsClient = require('../src/index.js') -function clientWorks (client, done) { - client.id((err, id) => { - expect(err).to.not.exist() +describe('ipfs-http-client constructor tests', () => { + describe('parameter permuations', () => { + it('none', () => { + const host = 'localhost' + const port = '5001' + const protocol = 'http' + const ipfs = ipfsClient() + expectConfig(ipfs, { host, port, protocol }) + }) - expect(id).to.have.a.property('id') - expect(id).to.have.a.property('publicKey') - done() + it('opts', () => { + const host = 'wizard.world' + const port = '999' + const protocol = 'https' + const ipfs = ipfsClient({ host, port, protocol }) + expectConfig(ipfs, { host, port }) + }) + + it('mutliaddr dns4 string, opts', () => { + const host = 'foo.com' + const port = '1001' + const protocol = 'https' + const addr = `/dns4/${host}/tcp/${port}` + const ipfs = ipfsClient(addr, { protocol }) + expectConfig(ipfs, { host, port, protocol }) + }) + + it('mutliaddr ipv4 string', () => { + const host = '101.101.101.101' + const port = '1001' + const addr = `/ip4/${host}/tcp/${port}` + const ipfs = ipfsClient(addr) + expectConfig(ipfs, { host, port }) + }) + + it('mutliaddr instance', () => { + const host = 'ace.place' + const port = '1001' + const addr = multiaddr(`/dns4/${host}/tcp/${port}`) + const ipfs = ipfsClient(addr) + expectConfig(ipfs, { host, port }) + }) + + it('host and port strings', () => { + const host = '1.1.1.1' + const port = '9999' + const ipfs = ipfsClient(host, port) + expectConfig(ipfs, { host, port }) + }) + + // TOOD: need to add `api-path` to `getEndpointConfig()` so we can test that setting a non-default value works + it.skip('host, port and api path', () => { + const host = '10.100.100.255' + const port = '9999' + const apiPath = '/future/api/v1' + const ipfs = ipfsClient(host, port, {'api-path': apiPath}) + expectConfig(ipfs, { host, port, apiPath }) + }) + + it('throws on invalid mutliaddr', () => { + expect(() => ipfsClient('/dns4')).to.throw('invalid address') + expect(() => ipfsClient('/hello')).to.throw('no protocol with name') + expect(() => ipfsClient('/dns4/ipfs.io')).to.throw('multiaddr must have a valid format') + }) }) -} -describe('ipfs-http-client constructor tests', () => { - describe('parameter permuations', () => { + describe('integration', () => { let apiAddr let ipfsd @@ -40,45 +96,31 @@ describe('ipfs-http-client constructor tests', () => { ipfsd.stop(done) }) - it('opts', (done) => { - const splitted = apiAddr.split('/') - clientWorks(ipfsClient({ - host: splitted[2], - port: splitted[4], - protocol: 'http' - }), done) - }) - - it('mutliaddr, opts', (done) => { - clientWorks(ipfsClient(apiAddr, { protocol: 'http' }), done) - }) - - it('host, port', (done) => { - const splitted = apiAddr.split('/') - - clientWorks(ipfsClient(splitted[2], splitted[4]), done) - }) - - it('specify host, port and api path', (done) => { - const splitted = apiAddr.split('/') - - clientWorks(ipfsClient({ - host: splitted[2], - port: splitted[4], - 'api-path': '/api/v0/' - }), done) + it('can connect to an ipfs http api', (done) => { + clientWorks(ipfsClient(apiAddr), done) }) + }) +}) - it('host, port, opts', (done) => { - const splitted = apiAddr.split('/') - - clientWorks(ipfsClient(splitted[2], splitted[4], { protocol: 'http' }), done) - }) +function clientWorks (client, done) { + client.id((err, id) => { + expect(err).to.not.exist() - it('error in invalid mutliaddr', () => { - expect(() => ipfsClient('/dns4')).to.throw('invalid address') - expect(() => ipfsClient('/hello')).to.throw('no protocol with name') - expect(() => ipfsClient('/dns4/ipfs.io')).to.throw('multiaddr must have a valid format') - }) + expect(id).to.have.a.property('id') + expect(id).to.have.a.property('publicKey') + done() }) -}) +} + +function expectConfig (ipfs, {host, port, protocol, apiPath}) { + const conf = ipfs.util.getEndpointConfig() + expect(conf.host).to.equal(host) + expect(conf.port).to.equal(port) + // TODO: needs https://github.com/ipfs/js-ipfs-http-client/pull/935 + if (protocol) { + expect(conf.protocol).to.equal(protocol) + } + if (apiPath) { + expect(conf['api-path']).to.equal(apiPath) + } +} From 3c3d4a71cb4db61108efc87e735f569fb3a53c4f Mon Sep 17 00:00:00 2001 From: Oli Evans Date: Mon, 28 Jan 2019 14:19:38 +0000 Subject: [PATCH 5/8] fix: update constructor.spec to check apiPath License: MIT Signed-off-by: Oli Evans --- test/constructor.spec.js | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/test/constructor.spec.js b/test/constructor.spec.js index e04ef4d71..37b4dfd75 100644 --- a/test/constructor.spec.js +++ b/test/constructor.spec.js @@ -16,8 +16,9 @@ describe('ipfs-http-client constructor tests', () => { const host = 'localhost' const port = '5001' const protocol = 'http' + const apiPath = '/api/v0/' const ipfs = ipfsClient() - expectConfig(ipfs, { host, port, protocol }) + expectConfig(ipfs, { host, port, protocol, apiPath }) }) it('opts', () => { @@ -25,7 +26,7 @@ describe('ipfs-http-client constructor tests', () => { const port = '999' const protocol = 'https' const ipfs = ipfsClient({ host, port, protocol }) - expectConfig(ipfs, { host, port }) + expectConfig(ipfs, { host, port, protocol }) }) it('mutliaddr dns4 string, opts', () => { @@ -60,12 +61,11 @@ describe('ipfs-http-client constructor tests', () => { expectConfig(ipfs, { host, port }) }) - // TOOD: need to add `api-path` to `getEndpointConfig()` so we can test that setting a non-default value works - it.skip('host, port and api path', () => { + it('host, port and api path', () => { const host = '10.100.100.255' const port = '9999' - const apiPath = '/future/api/v1' - const ipfs = ipfsClient(host, port, {'api-path': apiPath}) + const apiPath = '/future/api/v1/' + const ipfs = ipfsClient(host, port, { 'api-path': apiPath }) expectConfig(ipfs, { host, port, apiPath }) }) @@ -114,13 +114,8 @@ function clientWorks (client, done) { function expectConfig (ipfs, {host, port, protocol, apiPath}) { const conf = ipfs.util.getEndpointConfig() - expect(conf.host).to.equal(host) - expect(conf.port).to.equal(port) - // TODO: needs https://github.com/ipfs/js-ipfs-http-client/pull/935 - if (protocol) { - expect(conf.protocol).to.equal(protocol) - } - if (apiPath) { - expect(conf['api-path']).to.equal(apiPath) - } + expect(conf.host).to.equal(host || 'localhost') + expect(conf.port).to.equal(port || '5001') + expect(conf.protocol).to.equal(protocol || 'http') + expect(conf['api-path']).to.equal(apiPath || '/api/v0/') } From 5031e03c34829d6bd4dae9da6e01e6da26bfe2f4 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 28 Jan 2019 14:37:44 +0000 Subject: [PATCH 6/8] chore: appease linter --- src/index.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/index.js b/src/index.js index e7ea6f01b..5be787427 100644 --- a/src/index.js +++ b/src/index.js @@ -17,13 +17,10 @@ function ipfsClient (hostOrMultiaddr, port, opts) { hostAndPort.host = split[0] hostAndPort.port = split[1] } - } else if (multiaddr.isMultiaddr(hostOrMultiaddr)) { hostAndPort = toHostAndPort(hostOrMultiaddr) - } else if (typeof hostOrMultiaddr === 'object') { hostAndPort = hostOrMultiaddr - } else if (typeof hostOrMultiaddr === 'string') { if (hostOrMultiaddr[0] === '/') { // throws if multiaddr is malformed or can't be converted to a nodeAddress From 45f58309644d8aa15854d4a7ecdbb21ff5ad64cf Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Mon, 28 Jan 2019 14:38:21 +0000 Subject: [PATCH 7/8] chore: appease linter --- test/constructor.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/constructor.spec.js b/test/constructor.spec.js index 37b4dfd75..89c0b0e15 100644 --- a/test/constructor.spec.js +++ b/test/constructor.spec.js @@ -112,7 +112,7 @@ function clientWorks (client, done) { }) } -function expectConfig (ipfs, {host, port, protocol, apiPath}) { +function expectConfig (ipfs, { host, port, protocol, apiPath }) { const conf = ipfs.util.getEndpointConfig() expect(conf.host).to.equal(host || 'localhost') expect(conf.port).to.equal(port || '5001') From 97c44ad2209baf16807b75eadaf83018c37d4ef0 Mon Sep 17 00:00:00 2001 From: Oli Evans Date: Tue, 29 Jan 2019 09:33:39 +0000 Subject: [PATCH 8/8] fix: no args constructor test for browser License: MIT Signed-off-by: Oli Evans --- test/constructor.spec.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/constructor.spec.js b/test/constructor.spec.js index 37b4dfd75..afbbaffd7 100644 --- a/test/constructor.spec.js +++ b/test/constructor.spec.js @@ -13,12 +13,13 @@ const ipfsClient = require('../src/index.js') describe('ipfs-http-client constructor tests', () => { describe('parameter permuations', () => { it('none', () => { - const host = 'localhost' - const port = '5001' - const protocol = 'http' - const apiPath = '/api/v0/' const ipfs = ipfsClient() - expectConfig(ipfs, { host, port, protocol, apiPath }) + if (typeof self !== 'undefined') { + const { hostname, port } = self.location + expectConfig(ipfs, { host: hostname, port }) + } else { + expectConfig(ipfs, {}) + } }) it('opts', () => {