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).
This commit is contained in:
Henry Bridge 2022-02-06 17:40:51 -05:00 committed by GitHub
parent 372c1d0b13
commit 9945a5dffe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 102 additions and 20 deletions

12
API.md
View File

@ -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]
```

View File

@ -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);
},
);

View File

@ -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

View File

@ -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<void> {
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) => {

View File

@ -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;

View File

@ -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,

View File

@ -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 }) => {

View File

@ -348,16 +348,22 @@ export function initArgs(argv: string[]): yargs.Argv<RawOptions> {
.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

View File

@ -46,6 +46,7 @@ describe('fields', () => {
proxyRules: undefined,
showMenuBar: false,
singleInstance: false,
strictInternalUrls: false,
titleBarStyle: undefined,
tray: 'false',
userAgent: undefined,

View File

@ -46,6 +46,7 @@ const mockedAsyncConfig: AppOptions = {
proxyRules: undefined,
showMenuBar: false,
singleInstance: false,
strictInternalUrls: false,
titleBarStyle: undefined,
tray: 'false',
userAgent: undefined,

View File

@ -105,6 +105,7 @@ export async function getOptions(rawOptions: RawOptions): Promise<AppOptions> {
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,