fix: IP support on opt-in and opt-out lists (#945)

Co-authored-by: Marcin Rataj <lidel@lidel.org>

add-on/src/lib/options.js

@@ -1,5 +1,6 @@
 'use strict'
 
+const isIP = require('is-ip')
 const isFQDN = require('is-fqdn')
 const { hasChromeSocketsForTcp } = require('./runtime-checks')
 
@@ -117,11 +118,23 @@ function guiURLString (url, opts) {
 exports.safeURL = safeURL
 exports.guiURLString = guiURLString
 
+// ensure value is a valid URL.hostname (FQDN || ipv4 || ipv6 WITH brackets)
+exports.isHostname = x => isFQDN(x) || isIP.v4(x) || (!isIP.v6(x) && isIP.v6(x.replace(/[[\]]+/g, '')))
+
 // convert JS array to multiline textarea
 function hostArrayCleanup (array) {
   array = array.map(host => host.trim().toLowerCase())
+  // normalize/extract hostnames (just domain/ip, drop ports etc), if provided
+  array = array.map(x => {
+    try {
+      if (isIP.v6(x)) x = `[${x}]`
+      return new URL(`http://${x}`).hostname
+    } catch (_) {
+      return undefined
+    }
+  })
+  array = array.filter(Boolean).filter(exports.isHostname)
   array = [...new Set(array)] // dedup
-  array = array.filter(Boolean).filter(isFQDN)
   array.sort()
   return array
 }

add-on/src/lib/state.js

@@ -1,8 +1,7 @@
 'use strict'
 /* eslint-env browser, webextensions */
 
-const isFQDN = require('is-fqdn')
-const { safeURL } = require('./options')
+const { safeURL, isHostname } = require('./options')
 const { braveJsIpfsWebuiCid } = require('./precache')
 const offlinePeerCount = -1
 
@@ -32,11 +31,11 @@ function initState (options, overrides) {
   state.activeIntegrations = (url) => {
     if (!state.active) return false
     try {
-      const fqdn = isFQDN(url) ? url : new URL(url).hostname
+      const hostname = isHostname(url) ? url : new URL(url).hostname
       // opt-out has more weight, we also match parent domains
-      const disabledDirectlyOrIndirectly = state.disabledOn.some(host => fqdn.endsWith(host))
+      const disabledDirectlyOrIndirectly = state.disabledOn.some(optout => hostname.endsWith(optout))
       // ..however direct opt-in should overwrite parent's opt-out
-      const enabledDirectly = state.enabledOn.some(host => host === fqdn)
+      const enabledDirectly = state.enabledOn.some(optin => optin === hostname)
       return !(disabledDirectlyOrIndirectly && !enabledDirectly)
     } catch (_) {
       return false

package.json

@@ -139,6 +139,7 @@
     "ipfs-http-client": "48.0.0",
     "ipfs-postmsg-proxy": "3.1.1",
     "is-fqdn": "2.0.1",
+    "is-ip": "3.1.0",
     "is-ipfs": "2.0.0",
     "it-all": "1.0.4",
     "it-concat": "1.0.2",

test/functional/lib/options.test.js

@@ -3,10 +3,12 @@ const { describe, it, beforeEach, after } = require('mocha')
 const { expect } = require('chai')
 const sinon = require('sinon')
 const browser = require('sinon-chrome')
-const { storeMissingOptions, optionDefaults, hostTextToArray, hostArrayToText } = require('../../../add-on/src/lib/options')
+const { storeMissingOptions, optionDefaults, isHostname, hostTextToArray, hostArrayToText } = require('../../../add-on/src/lib/options')
+const { URL } = require('url')
 
 describe('storeMissingOptions()', function () {
   beforeEach(() => {
+    global.URL = URL
     browser.flush()
   })
 
@@ -59,10 +61,28 @@ describe('storeMissingOptions()', function () {
   })
 })
 
+describe('isHostname()', function () {
+  it('should return false for invalid URL.hostname', () => {
+    expect(isHostname('random text with whitespaces')).to.equal(false)
+  })
+  it('should return true for a valid URL.hostname (FQDN)', () => {
+    expect(isHostname('example.com')).to.equal(true)
+  })
+  it('should return true for a valid URL.hostname (ipv4)', () => {
+    expect(isHostname('192.168.1.1')).to.equal(true)
+  })
+  it('should return true for valid URL.hostname (ipv6 in brackets)', () => {
+    expect(isHostname('[fe80::bb67:770c:8a97:1]')).to.equal(true)
+  })
+  it('should return false for invalid URL.hostname (ipv6 without brackets)', () => {
+    expect(isHostname('fe80::bb67:770c:8a97:1')).to.equal(false)
+  })
+})
+
 describe('hostTextToArray()', function () {
   it('should sort, dedup hostnames, drop non-FQDNs and produce an array', () => {
-    const text = 'zombo.com\n two.com  \n totally not a FQDN \none.pl \nTWO.com\n\n'
-    const array = ['one.pl', 'two.com', 'zombo.com']
+    const text = 'zombo.com\n TwO.com \n 192.168.1.1:58080 \n192.168.1.2\n[fe80::bb67:770c:8a97:1]:58080\nfe80::bb67:770c:8a97:2\n[fe80::bb67:770c:8a97:3]\n totally not a FQDN \none.pl \nTWO.com\n\n'
+    const array = ['192.168.1.1', '192.168.1.2', '[fe80::bb67:770c:8a97:1]', '[fe80::bb67:770c:8a97:2]', '[fe80::bb67:770c:8a97:3]', 'one.pl', 'two.com', 'zombo.com']
     expect(hostTextToArray(text)).to.be.an('array').to.have.ordered.members(array)
   })
 })

yarn.lock

@@ -8081,20 +8081,20 @@ is-interactive@^1.0.0:
   resolved "https://registry.yarnpkg.com/is-interactive/-/is-interactive-1.0.0.tgz#cea6e6ae5c870a7b0a0004070b7b587e0252912e"
   integrity sha512-2HvIEKRoqS62guEC+qBjpvRubdX910WCMuJTZ+I9yvqKU2/12eSL549HMwtabb4oupdj2sMP50k+XJfB/8JE6w==
 
+is-ip@3.1.0, is-ip@^3.1.0:
+  version "3.1.0"
+  resolved "https://registry.yarnpkg.com/is-ip/-/is-ip-3.1.0.tgz#2ae5ddfafaf05cb8008a62093cf29734f657c5d8"
+  integrity sha512-35vd5necO7IitFPjd/YBeqwWnyDWbuLH9ZXQdMfDA8TEo7pv5X8yfrvVO3xbJbLUlERCMvf6X0hTUamQxCYJ9Q==
+  dependencies:
+    ip-regex "^4.0.0"
+
 is-ip@^2.0.0:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/is-ip/-/is-ip-2.0.0.tgz#68eea07e8a0a0a94c2d080dd674c731ab2a461ab"
   integrity sha1-aO6gfooKCpTC0IDdZ0xzGrKkYas=
   dependencies:
     ip-regex "^2.0.0"
 
-is-ip@^3.1.0:
-  version "3.1.0"
-  resolved "https://registry.yarnpkg.com/is-ip/-/is-ip-3.1.0.tgz#2ae5ddfafaf05cb8008a62093cf29734f657c5d8"
-  integrity sha512-35vd5necO7IitFPjd/YBeqwWnyDWbuLH9ZXQdMfDA8TEo7pv5X8yfrvVO3xbJbLUlERCMvf6X0hTUamQxCYJ9Q==
-  dependencies:
-    ip-regex "^4.0.0"
-
 is-ipfs@2.0.0, is-ipfs@^2.0.0:
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/is-ipfs/-/is-ipfs-2.0.0.tgz#c046622e4daf5435b671aeb9739a832107e06805"