Safely open in a new tab for a subpath scenario (#46684)

* Introduce subpath-safe URL utils

* Open data source links from a notebook safely on a subpath

* Open browse model links safely on a subpath

* Uniform naming

* Use the existing URL helper

* Trim url

* Add unit tests

* Add one more test case

* Fix test name 

* Make URLs non-optional strings

frontend/src/metabase/browse/components/ModelsTable.tsx

@@ -210,8 +210,10 @@ const TBodyRow = ({
           return;
         }
         const url = Urls.model({ id, name });
+        const subpathSafeUrl = Urls.getSubpathSafeUrl(url);
+
         if ((e.ctrlKey || e.metaKey) && e.button === 0) {
-          window.open(url, "_blank");
+          Urls.openInNewTab(subpathSafeUrl);
         } else {
           dispatch(push(url));
         }

frontend/src/metabase/lib/urls/utils.ts

+import api from "metabase/lib/api";
+
 export function appendSlug(path: string | number, slug?: string) {
   return slug ? `${path}-${slug}` : String(path);
 }
@@ -28,3 +30,16 @@ export function getEncodedUrlSearchParams(query: Record<string, unknown>) {
     })
     .join("&");
 }
+
+export function getSubpathSafeUrl(url: string) {
+  return api.basename + url;
+}
+
+/**
+ * Metabase can be deployed on a subpath!
+ * If you're opening internal links in a new tab, make sure you're using subpath-safe URLs.
+ * @see {@link getSubpathSafeUrl}
+ */
+export const openInNewTab = (url: string) => {
+  window.open(url, "_blank");
+};

frontend/src/metabase/lib/urls/utils.unit.spec.ts

+import api from "metabase/lib/api";
+
+import { getSubpathSafeUrl, openInNewTab } from "./utils";
+
+const fakeBasename = "foobar";
+const originalBasename = api.basename;
+
+const mockWindowOpen = jest.spyOn(window, "open").mockImplementation();
+
+describe("utils", () => {
+  beforeEach(() => {
+    api.basename = fakeBasename;
+  });
+
+  afterEach(() => {
+    api.basename = originalBasename;
+    mockWindowOpen.mockClear();
+  });
+
+  describe("getSubpathSafeUrl", () => {
+    it("should return basename if url is an empty string", () => {
+      expect(getSubpathSafeUrl("")).toBe(fakeBasename);
+    });
+
+    it("should return subpath-safe url", () => {
+      expect(getSubpathSafeUrl("/baz")).toBe(`${fakeBasename}/baz`);
+    });
+  });
+
+  describe("openInNewTab", () => {
+    it.each(["", "/", "/baz"])(
+      "should open the provided link in a new tab",
+      url => {
+        openInNewTab(url);
+
+        expect(mockWindowOpen).toHaveBeenCalledTimes(1);
+        expect(mockWindowOpen).toHaveBeenCalledWith(url, "_blank");
+      },
+    );
+  });
+});

frontend/src/metabase/query_builder/components/notebook/NotebookDataPicker/NotebookDataPicker.tsx

@@ -9,6 +9,7 @@ import {
 import { METAKEY } from "metabase/lib/browser";
 import { useDispatch, useStore } from "metabase/lib/redux";
 import { checkNotNull } from "metabase/lib/types";
+import * as Urls from "metabase/lib/urls";
 import { loadMetadataForTable } from "metabase/questions/actions";
 import { getMetadata } from "metabase/selectors/metadata";
 import type { IconName } from "metabase/ui";
@@ -18,7 +19,7 @@ import type { DatabaseId, TableId } from "metabase-types/api";
 
 import { NotebookCell } from "../NotebookCell";
 
-import { getUrl, openInNewTab } from "./utils";
+import { getUrl } from "./utils";
 
 interface NotebookDataPickerProps {
   title: string;
@@ -70,29 +71,28 @@ export function NotebookDataPicker({
     onChangeRef.current?.(table, metadataProvider);
   };
 
+  const openDataSourceInNewTab = () => {
+    const url = getUrl({ query, table, stageIndex });
+
+    if (!url) {
+      return;
+    }
+
+    const subpathSafeUrl = Urls.getSubpathSafeUrl(url);
+    Urls.openInNewTab(subpathSafeUrl);
+  };
+
   const handleClick = (event: MouseEvent<HTMLButtonElement>) => {
     const isCtrlOrMetaClick =
       (event.ctrlKey || event.metaKey) && event.button === 0;
 
-    if (isCtrlOrMetaClick) {
-      const url = getUrl({ query, table, stageIndex });
-
-      openInNewTab(url);
-    } else {
-      setIsOpen(true);
-    }
+    isCtrlOrMetaClick ? openDataSourceInNewTab() : setIsOpen(true);
   };
 
   const handleAuxClick = (event: MouseEvent<HTMLButtonElement>) => {
     const isMiddleClick = event.button === 1;
 
-    if (isMiddleClick) {
-      const url = getUrl({ query, table, stageIndex });
-
-      openInNewTab(url);
-    } else {
-      setIsOpen(true);
-    }
+    isMiddleClick ? openDataSourceInNewTab() : setIsOpen(true);
   };
 
   return (

frontend/src/metabase/query_builder/components/notebook/NotebookDataPicker/utils.ts

@@ -36,11 +36,3 @@ export const getUrl = ({
     return Urls.tableRowsQuery(databaseId, tableId);
   }
 };
-
-export const openInNewTab = (link?: string) => {
-  if (!link) {
-    return;
-  }
-
-  window.open(link, "_blank");
-};