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 { 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'}; }