Fix considering "same domain-ish" URLs as internal (PR #1126)

In 6b266b7815, as I got rid of deprecated dep `wurl`, I wrote:

> This one may be problematic, as it used to do TLD stuff:
> https://github.com/websanova/node-url/blob/7982a613bc/wurl.js#L4
>
> So, the new WHATWG-URL-based implementation will consider
> `asana.com` to be "external" to `app.asana.com`, contrarily to before.
> Given the nature of Nativefier, I think it's actually what to expect,
> that in this case you're "out of the app", and in e.g. asana's landing
> page, which you'd expect to see in your browser.

Turns out it's even more problematic: @TheCleric notices in https://github.com/nativefier/nativefier/pull/1124#issuecomment-790279403
that this breaks app `https://evernote.com` doing its login in `www.evernote.com`

The present change fixes this, by behaving mostly similarly to before,
but without re-introducing `wurl` or another dep needing a TLD/SLD list.
This commit is contained in:
Ronan Jouchet 2021-03-04 10:00:53 -05:00 committed by GitHub
parent 8f9135312b
commit c0a6604676
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 44 additions and 4 deletions

View File

@ -1,6 +1,10 @@
import { linkIsInternal, getCounterValue } from './helpers';
const internalUrl = 'https://medium.com/';
const internalUrlWww = 'https://www.medium.com/';
const sameBaseDomainUrl = 'https://app.medium.com/';
const internalUrlCoUk = 'https://medium.co.uk/';
const sameBaseDomainUrlCoUk = 'https://app.medium.co.uk/';
const internalUrlSubPath = 'topic/technology';
const externalUrl = 'https://www.wikipedia.org/wiki/Electron';
const wildcardRegex = /.*/;
@ -27,6 +31,28 @@ test('all urls should be internal with wildcard regex', () => {
expect(linkIsInternal(internalUrl, externalUrl, wildcardRegex)).toEqual(true);
});
test('a "www." of a domain should be considered internal', () => {
expect(linkIsInternal(internalUrl, internalUrlWww, undefined)).toEqual(true);
});
test('urls on the same "base domain" should be considered internal', () => {
expect(linkIsInternal(internalUrl, sameBaseDomainUrl, undefined)).toEqual(
true,
);
});
test('urls on the same "base domain" should be considered internal, even with a www', () => {
expect(linkIsInternal(internalUrlWww, sameBaseDomainUrl, undefined)).toEqual(
true,
);
});
test('urls on the same "base domain" should be considered internal, long SLD', () => {
expect(
linkIsInternal(internalUrlCoUk, sameBaseDomainUrlCoUk, undefined),
).toEqual(true);
});
const smallCounterTitle = 'Inbox (11) - nobody@example.com - Gmail';
const largeCounterTitle = 'Inbox (8,756) - nobody@example.com - Gmail';
const noCounterTitle = 'Inbox - nobody@example.com - Gmail';

View File

@ -33,9 +33,17 @@ export function linkIsInternal(
}
try {
const currentDomain = new URL(currentUrl).hostname;
const newDomain = new URL(newUrl).hostname;
return currentDomain === newDomain;
// Consider as "same domain-ish", without TLD/SLD list:
// 1. app.foo.com and foo.com
// 2. www.foo.com and foo.com
// 3. www.foo.com and app.foo.com
const currentDomain = new URL(currentUrl).hostname.replace(/^www\./, '');
const newDomain = new URL(newUrl).hostname.replace(/^www./, '');
const [longerDomain, shorterDomain] =
currentDomain.length > newDomain.length
? [currentDomain, newDomain]
: [newDomain, currentDomain];
return longerDomain.endsWith(shorterDomain);
} catch (err) {
console.warn(
'Failed to parse domains as determining if link is internal. From:',

View File

@ -400,7 +400,13 @@ Forces the packaged app to ignore web security errors, such as [Mixed Content](h
--internal-urls <regex>
```
Regular expression of URLs to consider "internal"; all other URLs will be opened in an external browser. Defaults to URLs on same second-level domain as app.
Regular expression of URLs to consider "internal" while following a hyperlink.
Internal URLs will open in Nativefier, other URLs will open in your preferred browser.
Defaults to view as "internal" two URLs that share the same base domain,
once stripped of `www.`. For example, by default,
- URLs from/to `foo.com`, `app.foo.com`, `www.foo.com` are considered internal.
- URLs from/to `abc.com` and `xyz.com` are considered external.
Example: