From 9945a5dffe937ed0eb073927edcea6c0174d937b Mon Sep 17 00:00:00 2001 From: Henry Bridge Date: Sun, 6 Feb 2022 17:40:51 -0500 Subject: [PATCH] Add flag --strict-internal-urls to disable domain and subpath matching (PR #1340) I created this so that Google Meet links don't open in my Google Calendar app for me, but it looks like others have a similar issue (e.g. issue #1304). --- API.md | 12 ++++- app/src/helpers/helpers.test.ts | 82 +++++++++++++++++++++++++------ app/src/helpers/helpers.ts | 5 ++ app/src/helpers/windowEvents.ts | 4 +- shared/src/options/model.ts | 4 ++ src/build/prepareElectronApp.ts | 1 + src/cli.test.ts | 1 + src/cli.ts | 10 +++- src/options/fields/fields.test.ts | 1 + src/options/optionsMain.test.ts | 1 + src/options/optionsMain.ts | 1 + 11 files changed, 102 insertions(+), 20 deletions(-) diff --git a/API.md b/API.md index c27e0fd..a89dd08 100644 --- a/API.md +++ b/API.md @@ -58,6 +58,7 @@ - [[block-external-urls]](#block-external-urls) - [[internal-urls]](#internal-urls) - [[internal-login-pages]](#internal-login-pages) + - [[strict-internal-urls]](#strict-internal-urls) - [[proxy-rules]](#proxy-rules) - [Auth Options](#auth-options) - [[basic-auth-username] and [basic-auth-password]](#basic-auth-username-and-basic-auth-password) @@ -768,7 +769,7 @@ Example of `--internal-urls` causing all links to Google to be considered intern nativefier https://google.com --internal-urls ".*?\.google\.*?" ``` -Or, if you never expect Nativefier to open an "external" page in your OS browser, +To turn off base domain matching, use [`--strict-internal-urls`](#strict-internal-urls). Or, if you never expect Nativefier to open an "external" page in your OS browser, ```bash nativefier https://google.com --internal-urls ".*?" @@ -800,6 +801,15 @@ based domains such as `.co.uk` as well If you think this list is missing a login page that you think should be internal, feel free to submit an [issue](https://github.com/nativefier/nativefier/issues/new?assignees=&labels=bug&template=bug_report.md&title=[New%20internal%20login%20page%20request]%20Your%20login%20page%20here) or even better a pull request! +#### [strict-internal-urls] + +``` +--strict-internal-urls +``` + +Disables base domain matching when determining if a link is internal. Only the `--internal-urls` regex and login pages will be matched against, so `app.foo.com` will be external to `www.foo.com` unless it matches the `--internal-urls` regex. + + #### [proxy-rules] ``` diff --git a/app/src/helpers/helpers.test.ts b/app/src/helpers/helpers.test.ts index 070520b..f3ace63 100644 --- a/app/src/helpers/helpers.test.ts +++ b/app/src/helpers/helpers.test.ts @@ -6,50 +6,78 @@ import { const internalUrl = 'https://medium.com/'; const internalUrlWww = 'https://www.medium.com/'; +const internalUrlSubPathRegex = /https\:\/\/www.medium.com\/.*/; const sameBaseDomainUrl = 'https://app.medium.com/'; const internalUrlCoUk = 'https://medium.co.uk/'; const differentBaseDomainUrlCoUk = 'https://other.domain.co.uk/'; const sameBaseDomainUrlCoUk = 'https://app.medium.co.uk/'; const sameBaseDomainUrlTidalListen = 'https://listen.tidal.com/'; const sameBaseDomainUrlTidalLogin = 'https://login.tidal.com/'; +const sameBaseDomainUrlTidalRegex = /https\:\/\/(login|listen).tidal.com\/.*/; const internalUrlSubPath = 'topic/technology'; const externalUrl = 'https://www.wikipedia.org/wiki/Electron'; const wildcardRegex = /.*/; -test('the original url should be internal', () => { - expect(linkIsInternal(internalUrl, internalUrl, undefined)).toEqual(true); +test('the original url should be internal without --strict-internal-urls', () => { + expect(linkIsInternal(internalUrl, internalUrl, undefined, undefined)).toEqual(true); }); -test('sub-paths of the original url should be internal', () => { +test('the original url should be internal with --strict-internal-urls off', () => { + expect(linkIsInternal(internalUrl, internalUrl, undefined, false)).toEqual(true); +}); + +test('the original url should be internal with --strict-internal-urls on', () => { + expect(linkIsInternal(internalUrl, internalUrl, undefined, true)).toEqual(true); +}); + +test('sub-paths of the original url should be internal with --strict-internal-urls off', () => { expect( - linkIsInternal(internalUrl, internalUrl + internalUrlSubPath, undefined), + linkIsInternal(internalUrl, internalUrl + internalUrlSubPath, undefined, false), ).toEqual(true); }); -test("'about:blank' should be internal", () => { - expect(linkIsInternal(internalUrl, 'about:blank', undefined)).toEqual(true); +test('sub-paths of the original url should not be internal with --strict-internal-urls on', () => { + expect( + linkIsInternal(internalUrl, internalUrl + internalUrlSubPath, undefined, true), + ).toEqual(false); +}); + +test('sub-paths of the original url should be internal with using a regex and --strict-internal-urls on', () => { + expect( + linkIsInternal(internalUrl, internalUrl + internalUrlSubPath, internalUrlSubPathRegex, true), + ).toEqual(false); +}); + +test("'about:blank' should always be internal", () => { + expect(linkIsInternal(internalUrl, 'about:blank', undefined, true)).toEqual(true); }); test('urls from different sites should not be internal', () => { - expect(linkIsInternal(internalUrl, externalUrl, undefined)).toEqual(false); + expect(linkIsInternal(internalUrl, externalUrl, undefined, false)).toEqual(false); }); test('all urls should be internal with wildcard regex', () => { - expect(linkIsInternal(internalUrl, externalUrl, wildcardRegex)).toEqual(true); + expect(linkIsInternal(internalUrl, externalUrl, wildcardRegex, true)).toEqual(true); }); test('a "www." of a domain should be considered internal', () => { - expect(linkIsInternal(internalUrl, internalUrlWww, undefined)).toEqual(true); + expect(linkIsInternal(internalUrl, internalUrlWww, undefined, false)).toEqual(true); }); test('urls on the same "base domain" should be considered internal', () => { - expect(linkIsInternal(internalUrl, sameBaseDomainUrl, undefined)).toEqual( + expect(linkIsInternal(internalUrl, sameBaseDomainUrl, undefined, false)).toEqual( true, ); }); +test('urls on the same "base domain" should NOT be considered internal using --strict-internal-urls', () => { + expect(linkIsInternal(internalUrl, sameBaseDomainUrl, undefined, true)).toEqual( + false, + ); +}); + test('urls on the same "base domain" should be considered internal, even with a www', () => { - expect(linkIsInternal(internalUrlWww, sameBaseDomainUrl, undefined)).toEqual( + expect(linkIsInternal(internalUrlWww, sameBaseDomainUrl, undefined, false)).toEqual( true, ); }); @@ -60,19 +88,43 @@ test('urls on the same "base domain" should be considered internal, even with di sameBaseDomainUrlTidalListen, sameBaseDomainUrlTidalLogin, undefined, + false ), ).toEqual(true); }); +test('urls should support sub domain matching with a regex', () => { + expect( + linkIsInternal( + sameBaseDomainUrlTidalListen, + sameBaseDomainUrlTidalLogin, + sameBaseDomainUrlTidalRegex, + false + ), + ).toEqual(true); +}); + +test('urls on the same "base domain" should NOT be considered internal with different sub domains when using --strict-internal-urls', () => { + expect( + linkIsInternal( + sameBaseDomainUrlTidalListen, + sameBaseDomainUrlTidalLogin, + undefined, + true + ), + ).toEqual(false); +}); + + test('urls on the same "base domain" should be considered internal, long SLD', () => { expect( - linkIsInternal(internalUrlCoUk, sameBaseDomainUrlCoUk, undefined), + linkIsInternal(internalUrlCoUk, sameBaseDomainUrlCoUk, undefined, false), ).toEqual(true); }); test('urls on the a different "base domain" are considered NOT internal, long SLD', () => { expect( - linkIsInternal(internalUrlCoUk, differentBaseDomainUrlCoUk, undefined), + linkIsInternal(internalUrlCoUk, differentBaseDomainUrlCoUk, undefined, false), ).toEqual(false); }); @@ -119,7 +171,7 @@ const testLoginPages = [ test.each(testLoginPages)( '%s login page should be internal', (loginUrl: string) => { - expect(linkIsInternal(internalUrl, loginUrl, undefined)).toEqual(true); + expect(linkIsInternal(internalUrl, loginUrl, undefined, false)).toEqual(true); }, ); @@ -139,7 +191,7 @@ const testNonLoginPages = [ test.each(testNonLoginPages)( '%s page should not be internal', (url: string) => { - expect(linkIsInternal(internalUrl, url, undefined)).toEqual(false); + expect(linkIsInternal(internalUrl, url, undefined, false)).toEqual(false); }, ); diff --git a/app/src/helpers/helpers.ts b/app/src/helpers/helpers.ts index b374e24..d1ff440 100644 --- a/app/src/helpers/helpers.ts +++ b/app/src/helpers/helpers.ts @@ -116,6 +116,7 @@ export function linkIsInternal( currentUrl: string, newUrl: string, internalUrlRegex: string | RegExp | undefined, + isStrictInternalUrlsEnabled: boolean | undefined, ): boolean { log.debug('linkIsInternal', { currentUrl, newUrl, internalUrlRegex }); if (newUrl.split('#')[0] === 'about:blank') { @@ -133,6 +134,10 @@ export function linkIsInternal( } } + if (isStrictInternalUrlsEnabled) { + return currentUrl == newUrl; + } + try { // Consider as "same domain-ish", without TLD/SLD list: // 1. app.foo.com and foo.com diff --git a/app/src/helpers/windowEvents.ts b/app/src/helpers/windowEvents.ts index 77ea431..d99704f 100644 --- a/app/src/helpers/windowEvents.ts +++ b/app/src/helpers/windowEvents.ts @@ -73,7 +73,7 @@ export function onNewWindowHelper( parent, }); try { - if (!linkIsInternal(options.targetUrl, urlToGo, options.internalUrls)) { + if (!linkIsInternal(options.targetUrl, urlToGo, options.internalUrls, options.strictInternalUrls)) { preventDefault(); if (options.blockExternalUrls) { return new Promise((resolve) => { @@ -121,7 +121,7 @@ export function onWillNavigate( urlToGo: string, ): Promise { log.debug('onWillNavigate', { options, event, urlToGo }); - if (!linkIsInternal(options.targetUrl, urlToGo, options.internalUrls)) { + if (!linkIsInternal(options.targetUrl, urlToGo, options.internalUrls, options.strictInternalUrls)) { event.preventDefault(); if (options.blockExternalUrls) { return new Promise((resolve) => { diff --git a/shared/src/options/model.ts b/shared/src/options/model.ts index 902d307..8c7fa24 100644 --- a/shared/src/options/model.ts +++ b/shared/src/options/model.ts @@ -57,6 +57,7 @@ export interface AppOptions { quiet?: boolean; showMenuBar: boolean; singleInstance: boolean; + strictInternalUrls: boolean; titleBarStyle?: TitleBarValue; tray: TrayValue; userAgent?: string; @@ -114,6 +115,7 @@ export type OutputOptions = NativefierOptions & { name: string; nativefierVersion: string; oldBuildWarningText: string; + strictInternalUrls: boolean; targetUrl: string; userAgent?: string; zoom?: number; @@ -183,6 +185,7 @@ export type RawOptions = { quiet?: boolean; showMenuBar?: boolean; singleInstance?: boolean; + strictInternalUrls?: boolean; targetUrl?: string; titleBarStyle?: TitleBarValue; tray?: TrayValue; @@ -205,6 +208,7 @@ export type WindowOptions = { browserwindowOptions?: BrowserWindowOptions; insecure: boolean; internalUrls?: string | RegExp; + strictInternalUrls?: boolean; name: string; proxyRules?: string; show?: boolean; diff --git a/src/build/prepareElectronApp.ts b/src/build/prepareElectronApp.ts index e3f9cd5..637d514 100644 --- a/src/build/prepareElectronApp.ts +++ b/src/build/prepareElectronApp.ts @@ -80,6 +80,7 @@ function pickElectronAppArgs(options: AppOptions): OutputOptions { quiet: options.packager.quiet, showMenuBar: options.nativefier.showMenuBar, singleInstance: options.nativefier.singleInstance, + strictInternalUrls: options.nativefier.strictInternalUrls, targetUrl: options.packager.targetUrl, titleBarStyle: options.nativefier.titleBarStyle, tray: options.nativefier.tray, diff --git a/src/cli.test.ts b/src/cli.test.ts index 1751c52..ef67b8b 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -221,6 +221,7 @@ describe('initArgs + parseArgs', () => { { arg: 'portable', shortArg: '' }, { arg: 'show-menu-bar', shortArg: 'm' }, { arg: 'single-instance', shortArg: '' }, + { arg: 'strict-internal-urls', shortArg: '' }, { arg: 'verbose', shortArg: '' }, { arg: 'widevine', shortArg: '' }, ])('test boolean arg %s', ({ arg, shortArg }) => { diff --git a/src/cli.ts b/src/cli.ts index 5fd5629..8fdc35d 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -348,16 +348,22 @@ export function initArgs(argv: string[]): yargs.Argv { .option('internal-urls', { defaultDescription: 'URLs sharing the same base domain', description: - 'regex of URLs to consider "internal"; all other URLs will be opened in an external browser', + `regex of URLs to consider "internal"; by default matches based on domain (see '--strict-internal-urls'); all other URLs will be opened in an external browser`, type: 'string', }) + .option('strict-internal-urls', { + default: false, + description: + 'disable domain-based matching on internal URLs', + type: 'boolean', + }) .option('proxy-rules', { description: 'proxy rules; see https://www.electronjs.org/docs/api/session#sessetproxyconfig', type: 'string', }) .group( - ['block-external-urls', 'internal-urls', 'proxy-rules'], + ['block-external-urls', 'internal-urls', 'strict-internal-urls', 'proxy-rules'], decorateYargOptionGroup('URL Handling Options'), ) // Auth Options diff --git a/src/options/fields/fields.test.ts b/src/options/fields/fields.test.ts index 652bd92..1080c3e 100644 --- a/src/options/fields/fields.test.ts +++ b/src/options/fields/fields.test.ts @@ -46,6 +46,7 @@ describe('fields', () => { proxyRules: undefined, showMenuBar: false, singleInstance: false, + strictInternalUrls: false, titleBarStyle: undefined, tray: 'false', userAgent: undefined, diff --git a/src/options/optionsMain.test.ts b/src/options/optionsMain.test.ts index def6c06..d0435c9 100644 --- a/src/options/optionsMain.test.ts +++ b/src/options/optionsMain.test.ts @@ -46,6 +46,7 @@ const mockedAsyncConfig: AppOptions = { proxyRules: undefined, showMenuBar: false, singleInstance: false, + strictInternalUrls: false, titleBarStyle: undefined, tray: 'false', userAgent: undefined, diff --git a/src/options/optionsMain.ts b/src/options/optionsMain.ts index 4bedab2..9ab55a3 100644 --- a/src/options/optionsMain.ts +++ b/src/options/optionsMain.ts @@ -105,6 +105,7 @@ export async function getOptions(rawOptions: RawOptions): Promise { proxyRules: rawOptions.proxyRules, showMenuBar: rawOptions.showMenuBar ?? false, singleInstance: rawOptions.singleInstance ?? false, + strictInternalUrls: rawOptions.strictInternalUrls ?? false, titleBarStyle: rawOptions.titleBarStyle, tray: rawOptions.tray ?? 'false', userAgent: rawOptions.userAgent,