From f916fb2924e525b879ac060526bd3ccfb80ef47e Mon Sep 17 00:00:00 2001
From: Shivam Mishra <scm.mymail@gmail.com>
Date: Wed, 28 May 2025 17:56:32 +0530
Subject: [PATCH] fix: handle empty customDomain when checking for
 `isInternalLink` (#11609)

This PR improves the portal's internal link detection logic to be more
robust when handling empty or undefined configuration values.
Previously, the code could fail when `customDomain` was empty, causing
external links to incorrectly behave as internal links. The fix
introduces a new `isSameOrigin` helper function that safely compares
URLs using proper URL parsing and origin comparison, gracefully handling
edge cases like missing domains, relative paths, and malformed URLs.
This ensures external links consistently open in new tabs regardless of
portal configuration completeness.
---
 app/javascript/portal/portalHelpers.js     | 63 ++++++++++++++++++----
 app/javascript/portal/specs/portal.spec.js | 45 +++++++++++++++-
 2 files changed, 98 insertions(+), 10 deletions(-)

diff --git a/app/javascript/portal/portalHelpers.js b/app/javascript/portal/portalHelpers.js
index 9cb28e63a..68e2cb6a8 100644
--- a/app/javascript/portal/portalHelpers.js
+++ b/app/javascript/portal/portalHelpers.js
@@ -25,15 +25,56 @@ export const getHeadingsfromTheArticle = () => {
   return rows;
 };
 
+/**
+ * Converts various input formats to URL objects.
+ * Handles URL objects, domain strings, relative paths, and full URLs.
+ * @param {string|URL} input - Input to convert to URL object
+ * @returns {URL|null} URL object or null if input is invalid
+ */
+const toURL = input => {
+  if (!input) return null;
+  if (input instanceof URL) return input;
+
+  if (
+    typeof input === 'string' &&
+    !input.includes('://') &&
+    !input.startsWith('/')
+  ) {
+    return new URL(`https://${input}`);
+  }
+
+  if (typeof input === 'string' && input.startsWith('/')) {
+    return new URL(input, window.location.origin);
+  }
+
+  return new URL(input);
+};
+
+/**
+ * Determines if two URLs belong to the same host by comparing their normalized URL objects.
+ * Handles various input formats including URL objects, domain strings, relative paths, and full URLs.
+ * Returns false if either URL cannot be parsed or normalized.
+ * @param {string|URL} url1 - First URL to compare
+ * @param {string|URL} url2 - Second URL to compare
+ * @returns {boolean} True if both URLs have the same host, false otherwise
+ */
+const isSameHost = (url1, url2) => {
+  try {
+    const urlObj1 = toURL(url1);
+    const urlObj2 = toURL(url2);
+
+    if (!urlObj1 || !urlObj2) return false;
+
+    return urlObj1.hostname === urlObj2.hostname;
+  } catch (error) {
+    return false;
+  }
+};
+
 export const openExternalLinksInNewTab = () => {
   const { customDomain, hostURL } = window.portalConfig;
-  const isSameHost =
-    window.location.href.includes(customDomain) ||
-    window.location.href.includes(hostURL);
-
-  // Modify external links only on articles page
   const isOnArticlePage =
-    isSameHost && document.querySelector('#cw-article-content') !== null;
+    document.querySelector('#cw-article-content') !== null;
 
   document.addEventListener('click', event => {
     if (!isOnArticlePage) return;
@@ -41,10 +82,14 @@ export const openExternalLinksInNewTab = () => {
     const link = event.target.closest('a');
 
     if (link) {
+      const currentLocation = window.location.href;
+      const linkHref = link.href;
+
+      // Check against current location and custom domains
       const isInternalLink =
-        link.hostname === window.location.hostname ||
-        link.href.includes(customDomain) ||
-        link.href.includes(hostURL);
+        isSameHost(linkHref, currentLocation) ||
+        (customDomain && isSameHost(linkHref, customDomain)) ||
+        (hostURL && isSameHost(linkHref, hostURL));
 
       if (!isInternalLink) {
         link.target = '_blank';
diff --git a/app/javascript/portal/specs/portal.spec.js b/app/javascript/portal/specs/portal.spec.js
index 861950a57..5205c5d45 100644
--- a/app/javascript/portal/specs/portal.spec.js
+++ b/app/javascript/portal/specs/portal.spec.js
@@ -103,7 +103,6 @@ describe('openExternalLinksInNewTab', () => {
     openExternalLinksInNewTab();
 
     const link = simulateClick('#external');
-
     expect(link.target).toBe('_blank');
     expect(link.rel).toBe('noopener noreferrer');
   });
@@ -139,4 +138,48 @@ describe('openExternalLinksInNewTab', () => {
     expect(link.target).toBe('_blank');
     expect(link.rel).toBe('noopener noreferrer');
   });
+
+  it('opens external links in a new tab even if customDomain is empty', () => {
+    window = dom.window;
+    window.portalConfig = {
+      hostURL: 'app.chatwoot.com',
+    };
+
+    global.window = window;
+
+    openExternalLinksInNewTab();
+
+    const link = simulateClick('#external');
+    const internal = simulateClick('#internal');
+    const custom = simulateClick('#custom');
+
+    expect(link.target).toBe('_blank');
+    expect(link.rel).toBe('noopener noreferrer');
+
+    expect(internal.target).not.toBe('_blank');
+    // this will be blank since the configs customDomain is empty
+    // which is a fair expectation
+    expect(custom.target).toBe('_blank');
+  });
+
+  it('opens external links in a new tab even if hostURL is empty', () => {
+    window = dom.window;
+    window.portalConfig = {
+      customDomain: 'custom.domain.com',
+    };
+
+    global.window = window;
+
+    openExternalLinksInNewTab();
+
+    const link = simulateClick('#external');
+    const internal = simulateClick('#internal');
+    const custom = simulateClick('#custom');
+
+    expect(link.target).toBe('_blank');
+    expect(link.rel).toBe('noopener noreferrer');
+
+    expect(internal.target).not.toBe('_blank');
+    expect(custom.target).not.toBe('_blank');
+  });
 });