diff --git a/superset-frontend/packages/superset-ui-core/src/connection/normalizeBackendUrls.ts b/superset-frontend/packages/superset-ui-core/src/connection/normalizeBackendUrls.ts index ccea6be71d9..8375955d653 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/normalizeBackendUrls.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/normalizeBackendUrls.ts @@ -44,9 +44,6 @@ * already-router-relative paths are passed through unchanged. */ -const NOT_IMPLEMENTED = - 'normalizeBackendUrls is not implemented yet — landing in the green commit of the subdirectory-helpers PR.'; - /** * Field names whose values are router-relative URLs to this Superset * deployment and therefore safe to normalise. @@ -62,9 +59,9 @@ const NOT_IMPLEMENTED = * `NORMALIZER_EXCLUSIONS` below with the reason — keep that list in sync. */ export const NORMALIZED_URL_FIELDS = new Set([ - // Concrete entries are added in the green commit after the per-endpoint - // audit. The skeleton commit only ships the constant so static-invariant - // tests have a stable import target. + // Initial set — extended by follow-up commits as each endpoint is audited. + // `explore_url` is the highest-traffic field and the one Layer 3 tests pin. + 'explore_url', ]); /** @@ -116,27 +113,98 @@ export interface NormalizeOptions { } /** - * Recursively normalise URL fields in a JSON-shaped value. - * - * Returns a new value when normalisation changed anything; otherwise returns - * the input by reference so consumers can compare with `===`. + * Matches the same safe-scheme set used by `pathUtils.ensureAppRoot`. We + * deliberately keep this list in sync — the normaliser and the prefix helper + * must agree on what counts as "absolute, leave alone". */ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -- stub -export function normalizeBackendUrls( - value: T, - options: NormalizeOptions, -): T { - throw new Error(NOT_IMPLEMENTED); +const SAFE_ABSOLUTE_URL_RE = /^(?:https?|ftp|mailto|tel):/i; + +/** + * Strip a trailing slash from the configured application root so segment + * comparisons are consistent. Bootstrap data may render the root either way + * (`/superset` or `/superset/`); `applicationRoot()` already trims, but + * callers passing the value through configuration may not. + */ +function stripTrailingSlash(root: string): string { + return root.endsWith('/') ? root.slice(0, -1) : root; +} + +/** + * Decide whether `value` is a plain object that the walker should descend + * into. Class instances, Dates, Maps, etc. are returned by reference — we + * never mutate or replace those. + */ +function isPlainObject(value: unknown): value is Record { + if (value === null || typeof value !== 'object') return false; + const proto = Object.getPrototypeOf(value); + return proto === Object.prototype || proto === null; } /** * Normalise a single URL string. Exposed for use cases that read a URL * directly (e.g. bootstrap data) without going through the recursive walker. */ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -- stub export function normalizeBackendUrlString( value: string, options: NormalizeOptions, ): string { - throw new Error(NOT_IMPLEMENTED); + const root = stripTrailingSlash(options.applicationRoot); + if (!root) return value; + if (SAFE_ABSOLUTE_URL_RE.test(value)) return value; + if (value.startsWith('//')) return value; + if (value === root) return '/'; + if (value.startsWith(`${root}/`)) { + return value.slice(root.length); + } + return value; +} + +/** + * Recursively normalise URL fields in a JSON-shaped value. + * + * Returns a new value when normalisation changed anything; otherwise returns + * the input by reference so consumers can compare with `===`. + */ +export function normalizeBackendUrls(value: T, options: NormalizeOptions): T { + const root = stripTrailingSlash(options.applicationRoot); + if (!root) return value; + return walk(value, root) as T; +} + +function walk(value: unknown, root: string): unknown { + if (Array.isArray(value)) { + let changed = false; + const out: unknown[] = new Array(value.length); + for (let index = 0; index < value.length; index += 1) { + const item = value[index]; + const next = walk(item, root); + if (next !== item) changed = true; + out[index] = next; + } + return changed ? out : value; + } + + if (isPlainObject(value)) { + let changed = false; + const out: Record = {}; + for (const key of Object.keys(value)) { + const fieldValue = value[key]; + let nextValue: unknown; + if ( + NORMALIZED_URL_FIELDS.has(key) && + typeof fieldValue === 'string' + ) { + nextValue = normalizeBackendUrlString(fieldValue, { + applicationRoot: root, + }); + } else { + nextValue = walk(fieldValue, root); + } + if (nextValue !== fieldValue) changed = true; + out[key] = nextValue; + } + return changed ? out : value; + } + + return value; } diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.subdirectory.test.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.subdirectory.test.tsx index 090302f868a..e24356f1b65 100644 --- a/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.subdirectory.test.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.subdirectory.test.tsx @@ -143,6 +143,7 @@ describe('SliceHeaderControls — Cmd-click "Edit chart" under subdirectory depl expect(openSpy).toHaveBeenCalledWith( '/explore/?dashboard_page_id=abc&slice_id=371', '_blank', + 'noopener noreferrer', ); }); @@ -157,6 +158,7 @@ describe('SliceHeaderControls — Cmd-click "Edit chart" under subdirectory depl expect(openSpy).toHaveBeenCalledWith( '/superset/explore/?dashboard_page_id=abc&slice_id=371', '_blank', + 'noopener noreferrer', ); }); }); diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx index 508b83c0bc1..644e7a17cec 100644 --- a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx @@ -57,6 +57,7 @@ import { useDrillDetailMenuItems } from 'src/components/Chart/useDrillDetailMenu import { LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE } from 'src/logger/LogUtils'; import { MenuKeys, RootState } from 'src/dashboard/types'; import DrillDetailModal from 'src/components/Chart/DrillDetail/DrillDetailModal'; +import { openInNewTab } from 'src/utils/navigationUtils'; import { usePermissions } from 'src/hooks/usePermissions'; import { useDatasetDrillInfo } from 'src/hooks/apiResources/datasets'; import { ResourceStatus } from 'src/hooks/apiResources/apiResources'; @@ -263,7 +264,7 @@ const SliceHeaderControls = ( props.logExploreChart?.(props.slice.slice_id); if (domEvent.metaKey || domEvent.ctrlKey) { domEvent.preventDefault(); - window.open(props.exploreUrl, '_blank'); + openInNewTab(props.exploreUrl); } else { history.push(props.exploreUrl); } diff --git a/superset-frontend/src/utils/navigationUtils.ts b/superset-frontend/src/utils/navigationUtils.ts index f9682d46e54..657686de6c5 100644 --- a/superset-frontend/src/utils/navigationUtils.ts +++ b/superset-frontend/src/utils/navigationUtils.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import type { AnchorHTMLAttributes, ReactElement } from 'react'; +import { createElement, type AnchorHTMLAttributes, type ReactElement } from 'react'; import { ensureAppRoot } from './pathUtils'; // ============================================================================= @@ -34,8 +34,12 @@ import { ensureAppRoot } from './pathUtils'; // `navigationUtils.invariants.test.ts`) enforces that boundary. // ============================================================================= -const NOT_IMPLEMENTED = - 'navigationUtils helper not implemented yet — landing in the green commit of the subdirectory-helpers PR.'; +/** + * Features passed to `window.open` for new-tab navigation. `noopener` and + * `noreferrer` are mandatory — without them the opened page can drive the + * opener via `window.opener` (reverse tabnabbing) and read the referrer. + */ +const NEW_TAB_FEATURES = 'noopener noreferrer'; /** * Open a router-relative path in a new browser tab. @@ -43,9 +47,8 @@ const NOT_IMPLEMENTED = * The path is automatically prefixed with the application root so the new tab * lands inside Superset on subdirectory deployments. */ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -- stub export function openInNewTab(path: string): void { - throw new Error(NOT_IMPLEMENTED); + window.open(ensureAppRoot(path), '_blank', NEW_TAB_FEATURES); } /** @@ -55,9 +58,8 @@ export function openInNewTab(path: string): void { * destination is outside the React Router tree (e.g. a backend-rendered page) * or when a hard reload is required. */ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -- stub export function redirect(path: string): void { - throw new Error(NOT_IMPLEMENTED); + window.location.href = ensureAppRoot(path); } /** @@ -65,9 +67,8 @@ export function redirect(path: string): void { * No new history entry is pushed. Use sparingly — most navigation should go * through React Router's `history.replace`. */ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -- stub export function redirectReplace(path: string): void { - throw new Error(NOT_IMPLEMENTED); + window.location.replace(ensureAppRoot(path)); } /** @@ -75,9 +76,8 @@ export function redirectReplace(path: string): void { * router-relative path. Use for clipboard / share / email targets that need * to round-trip through external systems back to this Superset deployment. */ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -- stub export function getShareableUrl(path: string): string { - throw new Error(NOT_IMPLEMENTED); + return `${window.location.origin}${ensureAppRoot(path)}`; } /** @@ -87,11 +87,11 @@ export function getShareableUrl(path: string): string { * runtime. Static `` literals are fine — the static- * invariant test only flags non-literal hrefs. */ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -- stub export function AppLink( props: AnchorHTMLAttributes & { href: string }, ): ReactElement { - throw new Error(NOT_IMPLEMENTED); + const { href, ...rest } = props; + return createElement('a', { ...rest, href: ensureAppRoot(href) }); } // =============================================================================