fix: https multiaddr support in constructor (ipfs#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 <lidel@lidel.org>

Author: lidel

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

src/index.js

@@ -13,49 +13,60 @@ 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
     }
   }
 
   if (port && typeof port !== 'object') {
     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
 
   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'
   }
 }
 

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