From 5da0bcd0dd15579c85c8f2d3ebd1bfef48a3b9f1 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 9 Apr 2019 17:38:01 +0200 Subject: [PATCH] fix: https multiaddr support in constructor (#965) Passing HTTPS multiaddr as a string to constructor did not work, protocol was ignored and unencrypted HTTP was used. This commit improves parsing of multiaddr constructor parameter and adds tests to ensure HTTPS is respected in all scenarios. It also updates iso-stream-http to v0.1.2 which includes fix for "https does not exist" License: MIT Signed-off-by: Marcin Rataj --- package.json | 2 +- src/index.js | 37 ++++++++++++++++++++++++------------- test/constructor.spec.js | 38 +++++++++++++++++++++++++++++++++----- 3 files changed, 58 insertions(+), 19 deletions(-) diff --git a/package.json b/package.json index 7fe2d6c0c7..40bbcef66f 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "is-ipfs": "~0.6.0", "is-pull-stream": "0.0.0", "is-stream": "^1.1.0", - "iso-stream-http": "~0.1.1", + "iso-stream-http": "~0.1.2", "iso-url": "~0.4.6", "just-kebab-case": "^1.1.0", "just-map-keys": "^1.1.0", diff --git a/src/index.js b/src/index.js index feb49e0bdf..ddd0e3e37a 100644 --- a/src/index.js +++ b/src/index.js @@ -13,28 +13,26 @@ const loadCommands = require('./utils/load-commands') const getConfig = require('./utils/default-config') const sendRequest = require('./utils/send-request') -function ipfsClient (hostOrMultiaddr, port, opts) { +function ipfsClient (hostOrMultiaddr, port, userOptions) { // convert all three params to objects that we can merge. - let hostAndPort = {} + let options = {} 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] + options = urlToOptions(self.location) } } else if (multiaddr.isMultiaddr(hostOrMultiaddr)) { - hostAndPort = toHostAndPort(hostOrMultiaddr) + options = maToOptions(hostOrMultiaddr) } else if (typeof hostOrMultiaddr === 'object') { - hostAndPort = hostOrMultiaddr + options = 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)) + options = maToOptions(multiaddr(hostOrMultiaddr)) } else { // hostOrMultiaddr is domain or ip address as a string - hostAndPort.host = hostOrMultiaddr + options.host = hostOrMultiaddr } } @@ -42,7 +40,7 @@ function ipfsClient (hostOrMultiaddr, port, opts) { port = { port: port } } - const config = Object.assign(getConfig(), hostAndPort, port, opts) + const config = Object.assign(getConfig(), options, port, userOptions) const requestAPI = sendRequest(config) const cmds = loadCommands(requestAPI, config) cmds.send = requestAPI @@ -50,12 +48,25 @@ function ipfsClient (hostOrMultiaddr, port, opts) { return cmds } -// throws if multiaddr can't be converted to a nodeAddress -function toHostAndPort (multiaddr) { +function maToOptions (multiaddr) { + // ma.nodeAddress() throws if multiaddr can't be converted to a nodeAddress const nodeAddr = multiaddr.nodeAddress() + const protos = multiaddr.protos() + // only http and https are allowed as protocol, + // anything else will be replaced with http + const exitProtocol = protos[protos.length - 1].name return { host: nodeAddr.address, - port: nodeAddr.port + port: nodeAddr.port, + protocol: exitProtocol.startsWith('http') ? exitProtocol : 'http' + } +} + +function urlToOptions (url) { + return { + host: url.hostname, + port: url.port || (url.protocol.startsWith('https') ? 443 : 80), + protocol: url.protocol.startsWith('http') ? url.protocol.split(':')[0] : 'http' } } diff --git a/test/constructor.spec.js b/test/constructor.spec.js index 68e1339c82..af77d18e12 100644 --- a/test/constructor.spec.js +++ b/test/constructor.spec.js @@ -30,7 +30,25 @@ describe('ipfs-http-client constructor tests', () => { expectConfig(ipfs, { host, port, protocol }) }) - it('mutliaddr dns4 string, opts', () => { + it('multiaddr dns4 string (implicit http)', () => { + const host = 'foo.com' + const port = '1001' + const protocol = 'http' // default to http if not specified in multiaddr + const addr = `/dns4/${host}/tcp/${port}` + const ipfs = ipfsClient(addr) + expectConfig(ipfs, { host, port, protocol }) + }) + + it('multiaddr dns4 string (explicit https)', () => { + const host = 'foo.com' + const port = '1001' + const protocol = 'https' + const addr = `/dns4/${host}/tcp/${port}/${protocol}` + const ipfs = ipfsClient(addr) + expectConfig(ipfs, { host, port, protocol }) + }) + + it('multiaddr dns4 string, explicit https in opts', () => { const host = 'foo.com' const port = '1001' const protocol = 'https' @@ -39,15 +57,25 @@ describe('ipfs-http-client constructor tests', () => { expectConfig(ipfs, { host, port, protocol }) }) - it('mutliaddr ipv4 string', () => { + it('multiaddr ipv4 string (implicit http)', () => { const host = '101.101.101.101' const port = '1001' + const protocol = 'http' const addr = `/ip4/${host}/tcp/${port}` const ipfs = ipfsClient(addr) - expectConfig(ipfs, { host, port }) + expectConfig(ipfs, { host, port, protocol }) + }) + + it('multiaddr ipv4 string (explicit https)', () => { + const host = '101.101.101.101' + const port = '1001' + const protocol = 'https' + const addr = `/ip4/${host}/tcp/${port}/${protocol}` + const ipfs = ipfsClient(addr) + expectConfig(ipfs, { host, port, protocol }) }) - it('mutliaddr instance', () => { + it('multiaddr instance', () => { const host = 'ace.place' const port = '1001' const addr = multiaddr(`/dns4/${host}/tcp/${port}`) @@ -70,7 +98,7 @@ describe('ipfs-http-client constructor tests', () => { expectConfig(ipfs, { host, port, apiPath }) }) - it('throws on invalid mutliaddr', () => { + it('throws on invalid multiaddr', () => { 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')