Repository: jiahaog/nativefier
File: app/src/helpers/helpers.ts (TypeScript)

Code before:
): boolean {
  if (newUrl === 'about:blank') {
    return true;
  }

  if (isInternalLoginPage(newUrl)) {
    return true;
  }

  if (internalUrlRegex) {
    const regex = RegExp(internalUrlRegex);
    return regex.test(newUrl);
  }

  try {
    // 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

    // Only use the tld and the main domain for domain-ish test
    // Enables domain-ish equality for blog.foo.com and shop.foo.com
    // Also makes the endWith check basically obsolete, === could be used.
    const currentDomain = new URL(currentUrl).hostname
      .split('.')
      .slice(-2)
      .join('.');
    const newDomain = new URL(newUrl).hostname.split('.').slice(-2).join('.');
    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:',
      currentUrl,
      'To:',
      newUrl,
      err,
    );
    return false;
  }
}

export function shouldInjectCss(): boolean {
  try {
    fs.accessSync(INJECT_CSS_PATH);
    return true;
  } catch (e) {
    return false;

Review comment (bug):
This logic will break on [Second-Level Domains](https://en.wikipedia.org/wiki/Second-level_domain) like `.co.uk`. `https://listen.tidal.com/`, try using your code on domains under a SLD like `.co.uk`. What happens in these case is that:

```
> new URL('https://listen.tidal.co.uk').hostname.split('.').slice(-2)
[ 'co', 'uk' ]
```
, so your code will generate false positives :grimacing: :
```
> new URL('https://entirely.different.domain.co.uk').hostname.split('.').slice(-2)
[ 'co', 'uk' ]
```

I added a test covering this case: https://github.com/nativefier/nativefier/commit/1ad1844619d561030ec1e9e13a5e6ce5eb15ceed and I merged it into your PR, see: your code fails this test.

There is no "clean" solution to this. The only way to do it is to use a lib that packages the list of SLDs, but I want to avoid this, because a good dependency is one that doesn't exist, and because this list is an ever changing mess. Thus the current logic with a simple special case for `www.`

Right now, you can work around the issue passing an `--internal-urls`. On Nativefier side, until we see more cases that the suggestion I'm about to write wouldn't solve, I think I'd prefer solving it by considering your page internal as ***login*** page, by tweaking `isInternalLoginPage`. And here,
1. One option would be to add `login.tidal.com` to the allowlist...
2. ... but I think we're now ripe to make `isInternalLoginPage` return true for non-specific domains matching `(login|auth|account|accounts|signin).[a-zA-Z0-9_-\\.]+` (and remove the manually-allowlisted domains matching this generic regex). Now that we have experience with our allowlist and we know it works fine, I think it makes sense to widen it to generic hostnames looking like login pages. Objections to this, @rippin93 @TheCleric ?

Code after:
): boolean {
  if (newUrl === 'about:blank') {
    return true;
  }

  if (isInternalLoginPage(newUrl)) {
    return true;
  }

  if (internalUrlRegex) {
    const regex = RegExp(internalUrlRegex);
    if (regex.test(newUrl)) {
      return true;
    }
  }

  try {
    // 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

    // Only use the tld and the main domain for domain-ish test
    // Enables domain-ish equality for blog.foo.com and shop.foo.com
    return domainify(currentUrl) === domainify(newUrl);
  } catch (err) {
    console.warn(
      'Failed to parse domains as determining if link is internal. From:',
      currentUrl,
      'To:',
      newUrl,
      err,
    );
    return false;
  }
}

/**
 * Helper to determine domain-ish equality for many cases, the trivial ones
 * and the trickier ones, e.g. `blog.foo.com` and `shop.foo.com`,
 * in a way that is "good enough", and doesn't need a list of SLDs.
 * See chat at https://github.com/nativefier/nativefier/pull/1171#pullrequestreview-649132523
 */
function domainify(url: string): string {
  // So here's what we're doing here:
  // Get the hostname from the url
  const hostname = new URL(url).hostname;
  // Drop the first section if the domain
  const domain = hostname.split('.').slice(1).join('.');
  // Check the length, if it's too short, the hostname was probably the domain
  // Or if the domain doesn't have a . in it we went too far