Hardened isNotURL / isURL code in urlutil.js

js/lib/urlutil.js

@@ -99,41 +99,42 @@ const UrlUtil = {
     if (input === undefined || input === null) {
       return true
     }
-
+    if (typeof input !== 'string') {
+      return true
+    }
     // for cases where we have scheme and we dont want spaces in domain names
     const caseDomain = /^[\w]{2,5}:\/\/[^\s\/]+\//
     // for cases, quoted strings
     const case1Reg = /^".*"$/
-    // for cases, ?abc, "a? b", ".abc" and "abc." which should searching query
-    const case2Reg = /^(\?)|(\?.+\s)|^(\.)|(\.)$/
+    // for cases:
+    // - starts with "?" or "."
+    // - contains "? "
+    // - ends with "." (and was not preceded by a /)
+    const case2Reg = /(^\?)|(\?.+\s)|(^\.)|(^[^\/]*\.$)/
     // for cases, pure string
     const case3Reg = /[\?\.\/\s:]/
     // for cases, data:uri, view-source:uri and about
-    const case4Reg = /^data:|view-source:|mailto:|about:|chrome-extension:|magnet:.*/
+    const case4Reg = /^(data|view-source|mailto|about|chrome-extension|magnet):.*/
 
     let str = input.trim()
-    let scheme = this.getScheme(str)
+    const scheme = this.getScheme(str)
 
     if (str.toLowerCase() === 'localhost') {
       return false
     }
-
     if (case1Reg.test(str)) {
       return true
     }
-
     if (case2Reg.test(str) || !case3Reg.test(str) ||
         (scheme === undefined && /\s/g.test(str))) {
       return true
     }
     if (case4Reg.test(str)) {
       return !this.canParseURL(str)
     }
-
     if (scheme && (scheme !== 'file://')) {
       return !caseDomain.test(str + '/')
     }
-
     str = this.prependScheme(str)
     return !this.canParseURL(str)
   },

test/components/navigationBarTest.js

@@ -885,44 +885,66 @@ describe('navigationBar tests', function () {
     })
 
     describe('with url input value', function () {
-      Brave.beforeAll(this)
-
-      before(function * () {
-        this.page1 = Brave.server.url('page1.html')
+      describe('with regards to the webview', function () {
+        Brave.beforeAll(this)
+
+        before(function * () {
+          this.page1 = Brave.server.url('page1.html')
+
+          yield setup(this.app.client)
+          // wait for the urlInput to be fully initialized
+          yield this.app.client.waitForExist(urlInput)
+          yield this.app.client.keys(this.page1)
+          // hit enter
+          yield this.app.client.keys(Brave.keys.ENTER)
+        })
 
-        yield setup(this.app.client)
-        // wait for the urlInput to be fully initialized
-        yield this.app.client.waitForExist(urlInput)
-        yield this.app.client.keys(this.page1)
-        // hit enter
-        yield this.app.client.keys(Brave.keys.ENTER)
-      })
+        it('webview has focus', function * () {
+          yield this.app.client.waitForElementFocus(activeWebview)
+        })
 
-      it('webview has focus', function * () {
-        yield this.app.client.waitForElementFocus(activeWebview)
-      })
+        it('webview loads url', function * () {
+          var page1 = this.page1
+          yield this.app.client.waitUntil(function () {
+            return this.getAttribute(activeWebview, 'src').then((src) => src === page1)
+          })
+        })
 
-      it('webview loads url', function * () {
-        var page1 = this.page1
-        yield this.app.client.waitUntil(function () {
-          return this.getAttribute(activeWebview, 'src').then((src) => src === page1)
+        it('urlbar shows webview url when focused', function * () {
+          var page1 = this.page1
+          yield blur(this.app.client)
+          yield this.app.client.waitUntil(function () {
+            return this.isExisting(urlInput).then((exists) => exists === false)
+          })
+          yield this.app.client
+            .ipcSend('shortcut-focus-url')
+          yield this.app.client.waitUntil(function () {
+            return this.getValue(urlInput).then((val) => val === page1)
+          })
+          yield this.app.client.keys('abc')
+          yield this.app.client.waitUntil(function () {
+            return this.getValue(urlInput).then((val) => val === 'abc')
+          })
         })
       })
 
-      it('urlbar shows webview url when focused', function * () {
-        var page1 = this.page1
-        yield blur(this.app.client)
-        yield this.app.client.waitUntil(function () {
-          return this.isExisting(urlInput).then((exists) => exists === false)
-        })
-        yield this.app.client
-          .ipcSend('shortcut-focus-url')
-        yield this.app.client.waitUntil(function () {
-          return this.getValue(urlInput).then((val) => val === page1)
+      describe('when following URLs', function () {
+        Brave.beforeEach(this)
+
+        beforeEach(function * () {
+          yield setup(this.app.client)
+          yield this.app.client.waitForExist(urlInput)
         })
-        yield this.app.client.keys('abc')
-        yield this.app.client.waitUntil(function () {
-          return this.getValue(urlInput).then((val) => val === 'abc')
+
+        it('goes to the page (instead of search for the URL)', function * () {
+          const url = 'https://brave.com/page/cc?_ri_=3vv-8-e.'
+          yield this.app.client.keys(url)
+          yield this.app.client.keys(Brave.keys.ENTER)
+          yield this.app.client.waitUntil(function () {
+            return this.getValue(urlInput).then((val) => {
+              return val === url
+            })
+          })
         })
       })
     })

test/unit/lib/urlutilTest.js

@@ -42,58 +42,106 @@ describe('urlutil', function () {
   })
 
   describe('isNotURL', function () {
-    it('returns true when input is null', function () {
-      assert.equal(UrlUtil.isNotURL(null), true)
-    })
-    it('returns true when input is undefined', function () {
-      assert.equal(UrlUtil.isNotURL(), true)
-    })
-    it('returns false when input is "localhost"', function () {
-      assert.equal(UrlUtil.isNotURL('localhost'), false)
-    })
-    it('returns true when input is a quoted string', function () {
-      assert.equal(UrlUtil.isNotURL('"brave.com"'), true)
-    })
-    it('returns true when input is a pure string (no TLD)', function () {
-      assert.equal(UrlUtil.isNotURL('brave'), true)
-    })
-    it('returns false when input is a string with whitespace but has schema', function () {
-      assert.equal(UrlUtil.isNotURL('https://wwww.brave.com/test space.jpg'), false)
-    })
-    it('returns true when input is a string with schema but invalid domain name', function () {
-      assert.equal(UrlUtil.isNotURL('https://www.bra ve.com/test space.jpg'), true)
-    })
-    it('returns true when input contains more than one word', function () {
-      assert.equal(UrlUtil.isNotURL('brave is cool'), true)
-    })
-    it('returns false when input has custom protocol', function () {
-      assert.equal(UrlUtil.isNotURL('brave://test'), false)
-    })
-    it('returns true when input has space in schema', function () {
-      assert.equal(UrlUtil.isNotURL('https ://brave.com'), true)
-    })
-    it('returns false when input is chrome-extension', function () {
-      assert.equal(UrlUtil.isNotURL('chrome-extension://fmfcbgogabcbclcofgocippekhfcmgfj/cast_sender.js'), false)
-    })
-    it('returns false when input is mailto', function () {
-      assert.equal(UrlUtil.isNotURL('mailto:brian@brave.com'), false)
-    })
-    describe('search query', function () {
-      it('returns true when input starts with ?', function () {
-        assert.equal(UrlUtil.isNotURL('?brave'), true)
+    describe('returns false when input:', function () {
+      it('is a valid URL', function () {
+        assert.equal(UrlUtil.isNotURL('brave.com'), false)
+      })
+      it('is an absolute file path without scheme', function () {
+        assert.equal(UrlUtil.isNotURL('/file/path/to/file'), false)
+      })
+      it('is an absolute file path with scheme', function () {
+        assert.equal(UrlUtil.isNotURL('file:///file/path/to/file'), false)
+      })
+      describe('for special pages', function () {
+        it('is a data URI', function () {
+          assert.equal(UrlUtil.isNotURL('data:text/html,hi'), false)
+        })
+        it('is a view source URL', function () {
+          assert.equal(UrlUtil.isNotURL('view-source://url-here'), false)
+        })
+        it('is a mailto link', function () {
+          assert.equal(UrlUtil.isNotURL('mailto:brian@brave.com'), false)
+        })
+        it('is an about page', function () {
+          assert.equal(UrlUtil.isNotURL('about:preferences'), false)
+        })
+        it('is a chrome-extension page', function () {
+          assert.equal(UrlUtil.isNotURL('chrome-extension://fmfcbgogabcbclcofgocippekhfcmgfj/cast_sender.js'), false)
+        })
+        it('is a magnet URL', function () {
+          assert.equal(UrlUtil.isNotURL('magnet:?xt=urn:sha1:YNCKHTQCWBTRNJIV4WNAE52SJUQCZO5C'), false)
+        })
+      })
+      it('contains a hostname and port number', function () {
+        assert.equal(UrlUtil.isNotURL('someBraveServer:8089'), false)
+      })
+      it('starts or ends with whitespace', function () {
+        assert.equal(UrlUtil.isNotURL('  http://brave.com  '), false)
+        assert.equal(UrlUtil.isNotURL('\n\nhttp://brave.com\n\n'), false)
+        assert.equal(UrlUtil.isNotURL('\t\thttp://brave.com\t\t'), false)
       })
-      it('returns true when input has a question mark followed by a space', function () {
-        assert.equal(UrlUtil.isNotURL('? brave'), true)
+      it('is a URL which contains basic auth user/pass', function () {
+        assert.equal(UrlUtil.isNotURL('http://username:password@example.com'), false)
       })
-      it('returns true when input starts with .', function () {
-        assert.equal(UrlUtil.isNotURL('.brave'), true)
+      it('is localhost (case-insensitive)', function () {
+        assert.equal(UrlUtil.isNotURL('LoCaLhOsT'), false)
       })
-      it('returns true when input end with .', function () {
-        assert.equal(UrlUtil.isNotURL('brave.'), true)
+      it('ends with period (input contains a forward slash)', function () {
+        assert.equal(UrlUtil.isNotURL('https://brave.com/test/cc?_ri_=3vv-8-e.'), false)
+      })
+      it('is a string with whitespace but has schema', function () {
+        assert.equal(UrlUtil.isNotURL('https://wwww.brave.com/test space.jpg'), false)
+      })
+      it('has custom protocol', function () {
+        assert.equal(UrlUtil.isNotURL('brave://test'), false)
       })
     })
-    it('returns false when input is a valid URL', function () {
-      assert.equal(UrlUtil.isNotURL('brave.com'), false)
+
+    describe('returns true when input:', function () {
+      it('is null or undefined', function () {
+        assert.equal(UrlUtil.isNotURL(), true)
+        assert.equal(UrlUtil.isNotURL(null), true)
+      })
+      it('is not a string', function () {
+        assert.equal(UrlUtil.isNotURL(false), true)
+        assert.equal(UrlUtil.isNotURL(333.449), true)
+      })
+      it('is a quoted string', function () {
+        assert.equal(UrlUtil.isNotURL('"search term here"'), true)
+      })
+      it('is a pure string (no TLD)', function () {
+        assert.equal(UrlUtil.isNotURL('brave'), true)
+      })
+      describe('search query', function () {
+        it('starts with ?', function () {
+          assert.equal(UrlUtil.isNotURL('?brave'), true)
+        })
+        it('has a question mark followed by a space', function () {
+          assert.equal(UrlUtil.isNotURL('? brave'), true)
+        })
+        it('starts with .', function () {
+          assert.equal(UrlUtil.isNotURL('.brave'), true)
+        })
+        it('ends with . (input does NOT contain a forward slash)', function () {
+          assert.equal(UrlUtil.isNotURL('brave.'), true)
+        })
+      })
+      it('is a string with schema but invalid domain name', function () {
+        assert.equal(UrlUtil.isNotURL('https://www.bra ve.com/test space.jpg'), true)
+      })
+      it('contains more than one word', function () {
+        assert.equal(UrlUtil.isNotURL('brave is cool'), true)
+      })
+      it('is not an about page / view source / data URI / mailto / etc', function () {
+        assert.equal(UrlUtil.isNotURL('not-a-chrome-extension:'), true)
+        assert.equal(UrlUtil.isNotURL('mailtoo:'), true)
+      })
+      it('is a URL (without protocol) which contains basic auth user/pass', function () {
+        assert.equal(UrlUtil.isNotURL('username:password@example.com'), true)
+      })
+      it('has space in schema', function () {
+        assert.equal(UrlUtil.isNotURL('https ://brave.com'), true)
+      })
     })
   })
 
@@ -110,20 +158,10 @@ describe('urlutil', function () {
   })
 
   describe('isURL', function () {
-    it('absolute file path without scheme', function () {
-      assert.equal(UrlUtil.isURL('/file/path/to/file'), true)
-    })
-    it('absolute file path with scheme', function () {
-      assert.equal(UrlUtil.isURL('file:///file/path/to/file'), true)
-    })
-    it('detects data URI', function () {
-      assert.equal(UrlUtil.isURL('data:text/html,hi'), true)
-    })
-    it('someBraveServer:8089', function () {
-      assert.equal(UrlUtil.isURL('someBraveServer:8089'), true)
-    })
-    it('localhost', function () {
-      assert.equal(UrlUtil.isURL('localhost:8089'), true)
+    it('returns !isNotURL', function () {
+      assert.equal(UrlUtil.isURL('brave.com'), !UrlUtil.isNotURL('brave.com'))
+      assert.equal(UrlUtil.isURL('brave is cool'), !UrlUtil.isNotURL('brave is cool'))
+      assert.equal(UrlUtil.isURL('mailto:brian@brave.com'), !UrlUtil.isNotURL('mailto:brian@brave.com'))
     })
   })