From 9b1ee66c8ccfa88fd0f50d13d6d93e5d4418c2d6 Mon Sep 17 00:00:00 2001
From: Devin Binnie <52460000+devinbinnie@users.noreply.github.com>
Date: Thu, 16 Dec 2021 10:06:11 -0500
Subject: [PATCH] [MM-32946][MM-40602] Ensure URL is valid before showing
 tooltip link, allow parsed URLs that aren't valid to open in browser (#1925)

* [MM-40602] Ensure URL is valid before showing tooltip link

* Rework to allow invalid URLs to display

* [MM-32946] Allow parsable URLs and open invalid URIs in browser
---
 src/common/utils/url.ts                 | 6 +++++-
 src/main/views/MattermostView.test.js   | 6 ++++++
 src/main/views/MattermostView.ts        | 2 +-
 src/main/views/webContentEvents.test.js | 5 +++--
 src/main/views/webContentEvents.ts      | 2 ++
 5 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/src/common/utils/url.ts b/src/common/utils/url.ts
index bc5622d5..48c6ccd6 100644
--- a/src/common/utils/url.ts
+++ b/src/common/utils/url.ts
@@ -40,7 +40,11 @@ function getHost(inputURL: URL | string) {
 
 // isInternalURL determines if the target url is internal to the application.
 // - currentURL is the current url inside the webview
-function isInternalURL(targetURL: URL, currentURL: URL) {
+function isInternalURL(targetURL: URL | undefined, currentURL: URL) {
+    if (!targetURL) {
+        return false;
+    }
+
     if (targetURL.host !== currentURL.host) {
         return false;
     }
diff --git a/src/main/views/MattermostView.test.js b/src/main/views/MattermostView.test.js
index 0d8168cb..8347abf0 100644
--- a/src/main/views/MattermostView.test.js
+++ b/src/main/views/MattermostView.test.js
@@ -297,6 +297,12 @@ describe('main/views/MattermostView', () => {
             expect(mattermostView.emit).toHaveBeenCalledWith(UPDATE_TARGET_URL, 'http://server-2.com/some/other/path');
         });
 
+        it('should not throw error if URL is invalid', () => {
+            expect(() => {
+                mattermostView.handleUpdateTarget(null, 'not<a-real;;url');
+            }).not.toThrow();
+        });
+
         it('should not emit tooltip URL if internal', () => {
             mattermostView.handleUpdateTarget(null, 'http://server-1.com/path/to/channels');
             expect(mattermostView.emit).not.toHaveBeenCalled();
diff --git a/src/main/views/MattermostView.ts b/src/main/views/MattermostView.ts
index 4757f552..b5075d78 100644
--- a/src/main/views/MattermostView.ts
+++ b/src/main/views/MattermostView.ts
@@ -311,7 +311,7 @@ export class MattermostView extends EventEmitter {
     }
 
     handleUpdateTarget = (e: Event, url: string) => {
-        if (!url || !urlUtils.isInternalURL(new URL(url), this.tab.server.url)) {
+        if (url && !urlUtils.isInternalURL(urlUtils.parseURL(url), this.tab.server.url)) {
             this.emit(UPDATE_TARGET_URL, url);
         }
     }
diff --git a/src/main/views/webContentEvents.test.js b/src/main/views/webContentEvents.test.js
index 6bbf64ec..928736a7 100644
--- a/src/main/views/webContentEvents.test.js
+++ b/src/main/views/webContentEvents.test.js
@@ -183,9 +183,10 @@ describe('main/views/webContentsEvents', () => {
             expect(newWindow({url: 'devtools://aaaaaa.com'})).toStrictEqual({action: 'allow'});
         });
 
-        it('should deny invalid URI', () => {
+        it('should open invalid URIs in browser', () => {
             urlUtils.isValidURI.mockReturnValue(false);
-            expect(newWindow({url: 'http::'})).toStrictEqual({action: 'deny'});
+            expect(newWindow({url: 'https://google.com/?^'})).toStrictEqual({action: 'deny'});
+            expect(shell.openExternal).toBeCalledWith('https://google.com/?^');
         });
 
         it('should divert to allowProtocolDialog for custom protocols that are not mattermost or http', () => {
diff --git a/src/main/views/webContentEvents.ts b/src/main/views/webContentEvents.ts
index a15d5838..ff84986c 100644
--- a/src/main/views/webContentEvents.ts
+++ b/src/main/views/webContentEvents.ts
@@ -119,7 +119,9 @@ export class WebContentsEventManager {
             }
 
             // Check for valid URL
+            // Let the browser handle invalid URIs
             if (!urlUtils.isValidURI(details.url)) {
+                shell.openExternal(details.url);
                 return {action: 'deny'};
             }