diff --git a/UPDATING.md b/UPDATING.md index 3d42f2b3d4e1..9558c2974211 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -24,6 +24,8 @@ assists people when migrating to a new version. ## Next +- [39925](https://github.com/apache/superset/pull/39925): URL prefixing for `SUPERSET_APP_ROOT` subdirectory deployments is now handled automatically by helpers in `src/utils/navigationUtils` (`openInNewTab`, `redirect`, `getShareableUrl`, ``). Direct imports of `ensureAppRoot` / `makeUrl` from `src/utils/pathUtils` are forbidden outside `navigationUtils.ts` (enforced by a static-invariant test); contributors writing new code should use the focused helpers instead. No runtime behaviour change for existing callers — all 19 prior call sites have been migrated and four pre-existing double-prefix and missing-prefix bugs are fixed as part of the migration. + ### Granular Export Controls A new feature flag `GRANULAR_EXPORT_CONTROLS` introduces three fine-grained permissions that replace the legacy `can_csv` permission: diff --git a/superset-frontend/packages/superset-ui-core/src/connection/normalizeBackendUrls.ts b/superset-frontend/packages/superset-ui-core/src/connection/normalizeBackendUrls.ts new file mode 100644 index 000000000000..654ea76e863e --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/src/connection/normalizeBackendUrls.ts @@ -0,0 +1,126 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * Strips the configured application root from URL fields in API responses so + * the frontend always speaks router-relative paths. Without normalisation, + * `SupersetClient` and `` would re-prefix backend-supplied URLs and + * produce `/foo/foo/...`. + */ + +/** Field names known to be router-relative URLs to this Superset instance. */ +export const NORMALIZED_URL_FIELDS = new Set(['explore_url']); + +/** + * URL-shaped fields that look normalisable but are deliberately left alone + * (external destinations, CDN hosts, OAuth endpoints, deployment-dependent + * targets). Informational only — keep in sync with the negative tests. + */ +export const NORMALIZER_EXCLUSIONS: ReadonlyArray<{ + field: string; + reason: string; +}> = [ + { field: 'bug_report_url', reason: 'External (GitHub)' }, + { field: 'documentation_url', reason: 'External (docs site)' }, + { field: 'external_url', reason: 'External by name' }, + { field: 'bundle_url', reason: 'CDN / static asset host' }, + { field: 'tracking_url', reason: 'External (analytics)' }, + { field: 'user_login_url', reason: 'OAuth / SSO endpoint, may be external' }, + { field: 'user_logout_url', reason: 'OAuth / SSO endpoint, may be external' }, + { field: 'user_info_url', reason: 'OAuth / SSO endpoint, may be external' }, + { field: 'thumbnail_url', reason: 'Storage host varies (S3 / local)' }, + { field: 'creator_url', reason: 'User-profile destination varies' }, +]; + +export interface NormalizeOptions { + /** Application root to strip. Empty string disables normalisation. */ + applicationRoot: string; +} + +const SAFE_ABSOLUTE_URL_RE = /^(?:https?|ftp|mailto|tel):/i; + +function stripTrailingSlash(root: string): string { + return root.endsWith('/') ? root.slice(0, -1) : root; +} + +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 (used directly when walking is overkill). */ +export function normalizeBackendUrlString( + value: string, + options: NormalizeOptions, +): string { + 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; +} + +function walk(value: unknown, root: string): unknown { + if (Array.isArray(value)) { + let changed = false; + const out: unknown[] = []; + for (let index = 0; index < value.length; index += 1) { + const item = value[index]; + const next = walk(item, root); + if (next !== item) changed = true; + out.push(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]; + const nextValue = + NORMALIZED_URL_FIELDS.has(key) && typeof fieldValue === 'string' + ? normalizeBackendUrlString(fieldValue, { applicationRoot: root }) + : walk(fieldValue, root); + if (nextValue !== fieldValue) changed = true; + out[key] = nextValue; + } + return changed ? out : value; + } + + return value; +} + +/** + * Recursively normalise URL fields in a JSON-shaped value. Returns the input + * by reference when nothing changed, so callers 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; +} diff --git a/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientAppRootContract.test.ts b/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientAppRootContract.test.ts new file mode 100644 index 000000000000..f3f425534b39 --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientAppRootContract.test.ts @@ -0,0 +1,63 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { SupersetClientClass } from '@superset-ui/core'; + +// SupersetClient is expected to apply the configured appRoot exactly once. +// Callers must pass router-relative endpoints; pre-prefixing causes the +// double-prefix bug documented below. + +describe('SupersetClient applies the application root exactly once', () => { + const buildClient = () => + new SupersetClientClass({ + protocol: 'https:', + host: 'config_host', + appRoot: '/superset', + }); + + test('endpoint without leading slash is concatenated correctly', () => { + expect(buildClient().getUrl({ endpoint: 'api/v1/chart' })).toBe( + 'https://config_host/superset/api/v1/chart', + ); + }); + + test('endpoint with leading slash is normalised to a single root segment', () => { + expect(buildClient().getUrl({ endpoint: '/api/v1/chart' })).toBe( + 'https://config_host/superset/api/v1/chart', + ); + }); + + // Documents the double-prefix symptom: wrapping the endpoint in + // ensureAppRoot before passing it to SupersetClient duplicates the root. + // navigationUtils.invariants.test.ts catches this pattern statically. + test('does not double-apply the application root when caller pre-prefixes', () => { + expect(buildClient().getUrl({ endpoint: '/superset/api/v1/chart' })).toBe( + 'https://config_host/superset/superset/api/v1/chart', + ); + }); + + test('empty application root produces no prefix segment', () => { + const client = new SupersetClientClass({ + protocol: 'https:', + host: 'config_host', + }); + expect(client.getUrl({ endpoint: '/api/v1/chart' })).toBe( + 'https://config_host/api/v1/chart', + ); + }); +}); diff --git a/superset-frontend/packages/superset-ui-core/test/connection/normalizeBackendUrls.test.ts b/superset-frontend/packages/superset-ui-core/test/connection/normalizeBackendUrls.test.ts new file mode 100644 index 000000000000..ee38ab8fada5 --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/test/connection/normalizeBackendUrls.test.ts @@ -0,0 +1,149 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { + normalizeBackendUrls, + normalizeBackendUrlString, + NORMALIZED_URL_FIELDS, +} from '../../src/connection/normalizeBackendUrls'; + +const PREFIX = '/superset'; + +describe('normalizeBackendUrls', () => { + test('strips application root from a recognised URL field', () => { + const input = { id: 1, explore_url: '/superset/explore/?slice_id=1' }; + const output = normalizeBackendUrls(input, { applicationRoot: PREFIX }); + expect(output).toEqual({ id: 1, explore_url: '/explore/?slice_id=1' }); + }); + + // The negative cases below prove the normaliser is conservative: it doesn't + // mutate user content, external URLs, or path segments that merely share + // text with the configured root. + test('leaves non-allow-listed fields untouched even when path-shaped', () => { + const input = { description: '/superset/just-text-from-a-user' }; + expect(normalizeBackendUrls(input, { applicationRoot: PREFIX })).toEqual( + input, + ); + }); + + test('leaves absolute URLs untouched in recognised fields', () => { + const input = { explore_url: 'https://other.example.com/superset/foo' }; + expect(normalizeBackendUrls(input, { applicationRoot: PREFIX })).toEqual( + input, + ); + }); + + test('leaves protocol-relative URLs untouched', () => { + const input = { explore_url: '//cdn.example.com/superset/foo' }; + expect(normalizeBackendUrls(input, { applicationRoot: PREFIX })).toEqual( + input, + ); + }); + + test('does not strip a similar-but-different prefix segment', () => { + // /superset-public/... shares text with /superset but is a different path + // segment. Only /superset followed by / or end-of-string counts. + const input = { explore_url: '/superset-public/explore/?slice_id=1' }; + expect(normalizeBackendUrls(input, { applicationRoot: PREFIX })).toEqual( + input, + ); + }); + + test('is a no-op when application root is empty', () => { + const input = { explore_url: '/explore/?slice_id=1' }; + expect(normalizeBackendUrls(input, { applicationRoot: '' })).toEqual(input); + }); +}); + +describe('normalizeBackendUrlString', () => { + test('strips application root from a router-relative path', () => { + expect( + normalizeBackendUrlString('/superset/sqllab', { + applicationRoot: PREFIX, + }), + ).toBe('/sqllab'); + }); + + test('passes absolute URLs through unchanged', () => { + expect( + normalizeBackendUrlString('https://external.example.com/foo', { + applicationRoot: PREFIX, + }), + ).toBe('https://external.example.com/foo'); + }); +}); + +test('NORMALIZED_URL_FIELDS is a Set for O(1) lookup', () => { + expect(NORMALIZED_URL_FIELDS).toBeInstanceOf(Set); +}); + +describe('normalizeBackendUrls (recursion + identity)', () => { + test('descends into arrays and normalises matching fields per element', () => { + const input = [ + { explore_url: '/superset/explore/?id=1' }, + { explore_url: '/superset/explore/?id=2' }, + ]; + expect(normalizeBackendUrls(input, { applicationRoot: PREFIX })).toEqual([ + { explore_url: '/explore/?id=1' }, + { explore_url: '/explore/?id=2' }, + ]); + }); + + test('descends into nested objects', () => { + const input = { + result: { chart: { explore_url: '/superset/explore/?id=1' } }, + }; + expect(normalizeBackendUrls(input, { applicationRoot: PREFIX })).toEqual({ + result: { chart: { explore_url: '/explore/?id=1' } }, + }); + }); + + test('returns input by reference when nothing changed', () => { + const input = { explore_url: '/explore/?id=1' }; + const output = normalizeBackendUrls(input, { applicationRoot: PREFIX }); + expect(output).toBe(input); + }); + + test('is idempotent: normalize(normalize(x)) === normalize(x)', () => { + const input = { explore_url: '/superset/explore/?id=1' }; + const once = normalizeBackendUrls(input, { applicationRoot: PREFIX }); + const twice = normalizeBackendUrls(once, { applicationRoot: PREFIX }); + expect(twice).toEqual(once); + }); + + test('strips a value that equals the application root exactly', () => { + expect( + normalizeBackendUrlString('/superset', { applicationRoot: PREFIX }), + ).toBe('/'); + }); + + test('tolerates a trailing slash on applicationRoot', () => { + expect( + normalizeBackendUrlString('/superset/foo', { + applicationRoot: '/superset/', + }), + ).toBe('/foo'); + }); + + test('does not descend into class instances (Date, Map)', () => { + const date = new Date('2026-01-01'); + const input = { created_at: date }; + const output = normalizeBackendUrls(input, { applicationRoot: PREFIX }); + expect(output.created_at).toBe(date); + }); +}); diff --git a/superset-frontend/spec/helpers/sourceTreeScanner.ts b/superset-frontend/spec/helpers/sourceTreeScanner.ts new file mode 100644 index 000000000000..c5798592d0c7 --- /dev/null +++ b/superset-frontend/spec/helpers/sourceTreeScanner.ts @@ -0,0 +1,183 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { readdirSync, readFileSync, statSync } from 'fs'; +import { join, relative, resolve, sep } from 'path'; + +const DEFAULT_ROOTS = ['src', 'packages/superset-ui-core/src']; + +const ALWAYS_SKIP_SEGMENTS = new Set([ + 'node_modules', + 'dist', + 'build', + 'coverage', + '__mocks__', + 'cypress-base', + 'playwright', +]); + +const ALWAYS_SKIP_SUFFIXES = [ + '.test.ts', + '.test.tsx', + '.stories.ts', + '.stories.tsx', +]; + +const SOURCE_EXTENSIONS = ['.ts', '.tsx']; + +export interface ScanOptions { + /** Workspace-relative directories to scan. Defaults to the source tree. */ + roots?: string[]; + /** Extra path segments to skip on top of {@link ALWAYS_SKIP_SEGMENTS}. */ + ignoreSegments?: string[]; + /** Regex run against each line of each file. */ + pattern: RegExp; + /** Workspace-relative paths (forward slashes) exempt from this scan. */ + allowlist?: string[]; +} + +export interface ScanHit { + /** Workspace-relative path with forward slashes. */ + file: string; + /** 1-based line number. */ + line: number; + /** The text of the matching line, trimmed. */ + text: string; + /** The substring captured by `pattern`. */ + match: string; +} + +// __dirname resolves to /spec/helpers regardless of cwd. +const WORKSPACE_ROOT = resolve(__dirname, '..', '..'); + +function isSourceFile(name: string): boolean { + return ( + SOURCE_EXTENSIONS.some(ext => name.endsWith(ext)) && + !ALWAYS_SKIP_SUFFIXES.some(suffix => name.endsWith(suffix)) + ); +} + +function walk(directory: string, ignoreSegments: Set): string[] { + const found: string[] = []; + + let entries; + try { + entries = readdirSync(directory, { withFileTypes: true }); + } catch { + return found; + } + + for (const entry of entries) { + if (ignoreSegments.has(entry.name)) continue; + const absolute = join(directory, entry.name); + + if (entry.isDirectory()) { + found.push(...walk(absolute, ignoreSegments)); + } else if (entry.isFile() && isSourceFile(entry.name)) { + found.push(absolute); + } + } + + return found; +} + +function toForwardSlashes(path: string): string { + return sep === '/' ? path : path.split(sep).join('/'); +} + +/** + * Line-by-line regex scan over the source tree. Returns one {@link ScanHit} + * per matching line. Textual (not AST-based) — false positives on string + * literals should be fixed by tightening the regex. + */ +export function scanSource(options: ScanOptions): ScanHit[] { + const { + roots = DEFAULT_ROOTS, + ignoreSegments = [], + pattern, + allowlist = [], + } = options; + + const ignoreSet = new Set([...ALWAYS_SKIP_SEGMENTS, ...ignoreSegments]); + const allowSet = new Set(allowlist); + const hits: ScanHit[] = []; + + const seen = new Set(); + for (const root of roots) { + const absoluteRoot = resolve(WORKSPACE_ROOT, root); + let stat; + try { + stat = statSync(absoluteRoot); + } catch { + continue; + } + if (!stat.isDirectory()) continue; + + for (const absoluteFile of walk(absoluteRoot, ignoreSet)) { + if (seen.has(absoluteFile)) continue; + seen.add(absoluteFile); + + const relativePath = toForwardSlashes( + relative(WORKSPACE_ROOT, absoluteFile), + ); + if (allowSet.has(relativePath)) continue; + + const contents = readFileSync(absoluteFile, 'utf8'); + const lines = contents.split('\n'); + + // Reuse the regex per file. Without the `g` flag, `.exec` ignores + // lastIndex, so recompiling per-line was wasted allocation. + const lineRegex = pattern.flags.includes('g') + ? new RegExp(pattern.source, pattern.flags.replace('g', '')) + : pattern; + + for (let index = 0; index < lines.length; index += 1) { + const lineText = lines[index]; + const match = lineRegex.exec(lineText); + if (match) { + hits.push({ + file: relativePath, + line: index + 1, + text: lineText.trim(), + match: match[0], + }); + } + } + } + } + + return hits; +} + +/** Format hits as a multi-line failure message: ` file:line — text`. */ +export function formatHits(hits: ScanHit[], header: string): string { + if (hits.length === 0) return header; + const lines = hits + .slice(0, 50) + .map(hit => ` ${hit.file}:${hit.line} — ${hit.text}`); + const overflow = + hits.length > 50 ? `\n ... and ${hits.length - 50} more` : ''; + return `${header}\n${lines.join('\n')}${overflow}`; +} + +/** Throw with a formatted message if `hits` is non-empty. */ +export function expectNoHits(hits: ScanHit[], header: string): void { + if (hits.length > 0) { + throw new Error(formatHits(hits, header)); + } +} diff --git a/superset-frontend/spec/helpers/withApplicationRoot.ts b/superset-frontend/spec/helpers/withApplicationRoot.ts new file mode 100644 index 000000000000..a84535723566 --- /dev/null +++ b/superset-frontend/spec/helpers/withApplicationRoot.ts @@ -0,0 +1,53 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * Run `callback` with `getBootstrapData().common.application_root` set to + * `applicationRoot`. Resets modules so any imports inside the callback see + * the configured value, then restores the prior DOM and module cache on exit. + * Pass `''` to simulate the default root-of-domain deployment. + */ +export async function withApplicationRoot( + applicationRoot: string, + callback: () => Promise | T, +): Promise { + const previousBody = document.body.innerHTML; + + try { + const bootstrapData = { common: { application_root: applicationRoot } }; + document.body.innerHTML = `
`; + jest.resetModules(); + await import('src/utils/getBootstrapData'); + return await callback(); + } finally { + document.body.innerHTML = previousBody; + jest.resetModules(); + } +} + +/** Run `body` once per scenario, each under a different application root. */ +export async function applicationRootScenarios( + scenarios: S[], + body: (scenario: S) => Promise | void, +): Promise { + for (const scenario of scenarios) { + // eslint-disable-next-line no-await-in-loop -- intentional: scenarios share document state. + await withApplicationRoot(scenario.root, () => body(scenario)); + } +} diff --git a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx index 04f403789475..b8a3de0668aa 100644 --- a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx @@ -43,7 +43,7 @@ import { import { fDuration, extendedDayjs } from '@superset-ui/core/utils/dates'; import { SqlLabRootState } from 'src/SqlLab/types'; import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes'; -import { makeUrl } from 'src/utils/pathUtils'; +import { openInNewTab } from 'src/utils/navigationUtils'; import ResultSet from '../ResultSet'; import HighlightedSql from '../HighlightedSql'; import { StaticPosition, StyledTooltip, ModalResultSetWrapper } from './styles'; @@ -79,8 +79,7 @@ interface QueryTableProps { } const openQuery = (id: number) => { - const url = makeUrl(`/sqllab?queryId=${id}`); - window.open(url); + openInNewTab(`/sqllab?queryId=${id}`); }; const QueryTable = ({ diff --git a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx index d7c42fc2b2c2..f3cdb9bb37ef 100644 --- a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx +++ b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx @@ -86,7 +86,7 @@ import { usePermissions } from 'src/hooks/usePermissions'; import { StreamingExportModal } from 'src/components/StreamingExportModal'; import { useStreamingExport } from 'src/components/StreamingExportModal/useStreamingExport'; import { useConfirmModal } from 'src/hooks/useConfirmModal'; -import { makeUrl } from 'src/utils/pathUtils'; +import { makeUrl } from 'src/utils/navigationUtils'; import ExploreCtasResultsButton from '../ExploreCtasResultsButton'; import ExploreResultsButton from '../ExploreResultsButton'; import HighlightedSql from '../HighlightedSql'; diff --git a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx b/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx index 9326b63b485a..100c3d2f9eb1 100644 --- a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx +++ b/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx @@ -50,7 +50,6 @@ import Table, { import { RootState } from 'src/dashboard/types'; import { usePermissions } from 'src/hooks/usePermissions'; import { useToasts } from 'src/components/MessageToasts/withToasts'; -import { ensureAppRoot } from 'src/utils/pathUtils'; import { safeStringify } from 'src/utils/safeStringify'; import HeaderWithRadioGroup from '@superset-ui/core/components/Table/header-renderers/HeaderWithRadioGroup'; import { useDatasetMetadataBar } from 'src/features/datasets/metadataBar/useDatasetMetadataBar'; @@ -249,7 +248,7 @@ export default function DrillDetailPane({ if (dashboardId) { payload.form_data = { dashboardId }; } - SupersetClient.postForm(ensureAppRoot('/api/v1/chart/data'), { + SupersetClient.postForm('/api/v1/chart/data', { form_data: safeStringify(payload), }).catch(error => { addDangerToast( diff --git a/superset-frontend/src/components/Chart/chartAction.ts b/superset-frontend/src/components/Chart/chartAction.ts index 40d8ed79b168..e5d9345b1cf6 100644 --- a/superset-frontend/src/components/Chart/chartAction.ts +++ b/superset-frontend/src/components/Chart/chartAction.ts @@ -48,7 +48,6 @@ import { Logger, LOG_ACTIONS_LOAD_CHART } from 'src/logger/LogUtils'; import { allowCrossDomain as domainShardingEnabled } from 'src/utils/hostNamesConfig'; import { updateDataMask } from 'src/dataMask/actions'; import { waitForAsyncData } from 'src/middleware/asyncEvent'; -import { ensureAppRoot } from 'src/utils/pathUtils'; import { safeStringify } from 'src/utils/safeStringify'; import { extendedDayjs } from '@superset-ui/core/utils/dates'; import type { Dispatch, Action, AnyAction } from 'redux'; @@ -934,7 +933,7 @@ export function redirectSQLLab( requestedQuery: payload, }); } else { - SupersetClient.postForm(ensureAppRoot(redirectUrl), { + SupersetClient.postForm(redirectUrl, { form_data: safeStringify(payload), }); } diff --git a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx index 58f26f2829c4..20ca8e2e889a 100644 --- a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx +++ b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx @@ -78,7 +78,7 @@ import { } from 'src/database/actions'; import Mousetrap from 'mousetrap'; import { clearDatasetCache } from 'src/utils/cachedSupersetGet'; -import { makeUrl } from 'src/utils/pathUtils'; +import { makeUrl } from 'src/utils/navigationUtils'; import { OwnerSelectLabel, OWNER_TEXT_LABEL_PROP, diff --git a/superset-frontend/src/components/FacePile/index.tsx b/superset-frontend/src/components/FacePile/index.tsx index 220209afc2da..5fefd307d25a 100644 --- a/superset-frontend/src/components/FacePile/index.tsx +++ b/superset-frontend/src/components/FacePile/index.tsx @@ -24,7 +24,7 @@ import { } from '@superset-ui/core'; import getOwnerName from 'src/utils/getOwnerName'; import { Avatar, AvatarGroup, Tooltip } from '@superset-ui/core/components'; -import { ensureAppRoot } from 'src/utils/pathUtils'; +import { ensureAppRoot } from 'src/utils/navigationUtils'; import { getRandomColor } from './utils'; import type { FacePileProps } from './types'; diff --git a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts index 05a7e5d5b789..0e5f732517c4 100644 --- a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts +++ b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts @@ -19,7 +19,7 @@ import { useState, useCallback, useRef, useEffect } from 'react'; import { SupersetClient } from '@superset-ui/core'; import { ExportStatus, StreamingProgress } from './StreamingExportModal'; -import { makeUrl } from 'src/utils/pathUtils'; +import { makeUrl } from 'src/utils/navigationUtils'; import { applicationRoot } from 'src/utils/getBootstrapData'; interface UseStreamingExportOptions { @@ -36,8 +36,8 @@ interface StreamingExportParams { * The API endpoint URL for the export request. * * URLs should be prefixed with the application root at the call site using - * `makeUrl()` from 'src/utils/pathUtils'. This ensures proper handling for - * subdirectory deployments (e.g., /superset/api/v1/...). + * `makeUrl()` from `src/utils/navigationUtils`. This ensures proper handling + * for subdirectory deployments (e.g., /superset/api/v1/...). * * A defensive guard (`ensureUrlPrefix`) will apply the prefix if missing, * but callers should not rely on this fallback behavior. diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.subdirectory.test.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.subdirectory.test.tsx new file mode 100644 index 000000000000..3528bb12a084 --- /dev/null +++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.subdirectory.test.tsx @@ -0,0 +1,153 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { render, screen, userEvent } from 'spec/helpers/testing-library'; +import { VizType } from '@superset-ui/core'; +import mockState from 'spec/fixtures/mockState'; +import SliceHeaderControls, { SliceHeaderControlsProps } from '.'; + +// Subdirectory-specific regressions live here so the existing 676-line +// SliceHeaderControls.test.tsx doesn't need to mock getBootstrapData. + +// Name must start with `mock` so Jest's hoisted jest.mock() factory may +// reference it. `default` returns a static shape (not mockApplicationRoot) +// because consumers like setupClient.ts call getBootstrapData() at import +// time — calling mockApplicationRoot inside `default` hits TDZ. +const mockApplicationRoot = jest.fn(() => ''); + +jest.mock('src/utils/getBootstrapData', () => ({ + __esModule: true, + default: () => ({ + common: { application_root: '', static_assets_prefix: '' }, + }), + applicationRoot: () => mockApplicationRoot(), + staticAssetsPrefix: () => '', +})); + +const SLICE_ID = 371; + +const buildProps = (): SliceHeaderControlsProps => + ({ + addDangerToast: jest.fn(), + addSuccessToast: jest.fn(), + exploreChart: jest.fn(), + exportCSV: jest.fn(), + exportFullCSV: jest.fn(), + exportXLSX: jest.fn(), + exportFullXLSX: jest.fn(), + exportPivotExcel: jest.fn(), + forceRefresh: jest.fn(), + handleToggleFullSize: jest.fn(), + toggleExpandSlice: jest.fn(), + logEvent: jest.fn(), + logExploreChart: jest.fn(), + slice: { + slice_id: SLICE_ID, + slice_url: '/explore/?form_data=%7B%22slice_id%22%3A%20371%7D', + slice_name: 'Subdirectory regression chart', + slice_description: '', + form_data: { + slice_id: SLICE_ID, + datasource: '58__table', + viz_type: VizType.Sunburst, + }, + viz_type: VizType.Sunburst, + datasource: '58__table', + description: '', + description_markeddown: '', + owners: [], + modified: '', + changed_on: 0, + }, + isCached: [false], + isExpanded: false, + cachedDttm: [''], + updatedDttm: 0, + supersetCanExplore: true, + supersetCanCSV: true, + componentId: 'CHART-subdir', + dashboardId: 26, + isFullSize: false, + chartStatus: 'rendered', + showControls: true, + supersetCanShare: true, + formData: { + slice_id: SLICE_ID, + datasource: '58__table', + viz_type: VizType.Sunburst, + }, + exploreUrl: '/explore/?dashboard_page_id=abc&slice_id=371', + defaultOpen: true, + }) as unknown as SliceHeaderControlsProps; + +const renderControls = (): void => { + render(, { + useRedux: true, + useRouter: true, + initialState: { + ...mockState, + user: { + ...mockState.user, + roles: { Admin: [['can_samples', 'Datasource']] }, + }, + }, + }); +}; + +describe('SliceHeaderControls — Cmd-click "Edit chart" under subdirectory deployment', () => { + let openSpy: jest.SpyInstance; + + beforeEach(() => { + mockApplicationRoot.mockReturnValue(''); + openSpy = jest.spyOn(window, 'open').mockImplementation(() => null); + }); + + afterEach(() => { + openSpy.mockRestore(); + }); + + test('opens the unprefixed exploreUrl when application root is empty', async () => { + mockApplicationRoot.mockReturnValue(''); + renderControls(); + + userEvent.click(screen.getByRole('button', { name: 'More Options' })); + const editChart = await screen.findByText('Edit chart'); + userEvent.click(editChart, { metaKey: true }); + + expect(openSpy).toHaveBeenCalledWith( + '/explore/?dashboard_page_id=abc&slice_id=371', + '_blank', + 'noopener noreferrer', + ); + }); + + test('opens the prefixed exploreUrl when deployed under a subdirectory', async () => { + mockApplicationRoot.mockReturnValue('/superset'); + renderControls(); + + userEvent.click(screen.getByRole('button', { name: 'More Options' })); + const editChart = await screen.findByText('Edit chart'); + userEvent.click(editChart, { metaKey: true }); + + 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 508b83c0bc16..644e7a17cecf 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/explore/components/controls/ViewQuery.test.tsx b/superset-frontend/src/explore/components/controls/ViewQuery.test.tsx index 5fd67dafa076..e1b5c50c24bc 100644 --- a/superset-frontend/src/explore/components/controls/ViewQuery.test.tsx +++ b/superset-frontend/src/explore/components/controls/ViewQuery.test.tsx @@ -185,6 +185,7 @@ test('opens SQL Lab in a new tab when View in SQL Lab button is clicked with met expect(window.open).toHaveBeenCalledWith( `/sqllab?datasourceKey=${datasource}&sql=${encodeURIComponent(sql)}`, '_blank', + 'noopener noreferrer', ); }); diff --git a/superset-frontend/src/explore/components/controls/ViewQuery.tsx b/superset-frontend/src/explore/components/controls/ViewQuery.tsx index 647b342c5c1f..3faa73de32b6 100644 --- a/superset-frontend/src/explore/components/controls/ViewQuery.tsx +++ b/superset-frontend/src/explore/components/controls/ViewQuery.tsx @@ -40,7 +40,7 @@ import { import { CopyToClipboard } from 'src/components'; import { RootState } from 'src/dashboard/types'; import { findPermission } from 'src/utils/findPermission'; -import { makeUrl } from 'src/utils/pathUtils'; +import { openInNewTab } from 'src/utils/navigationUtils'; import CodeSyntaxHighlighter, { SupportedLanguage, preloadLanguages, @@ -138,11 +138,8 @@ const ViewQuery: FC = props => { }; if (domEvent.metaKey || domEvent.ctrlKey) { domEvent.preventDefault(); - window.open( - makeUrl( - `/sqllab?datasourceKey=${datasource}&sql=${encodeURIComponent(currentSQL)}`, - ), - '_blank', + openInNewTab( + `/sqllab?datasourceKey=${datasource}&sql=${encodeURIComponent(currentSQL)}`, ); } else { history.push({ pathname: '/sqllab', state: { requestedQuery } }); diff --git a/superset-frontend/src/explore/exploreUtils/index.ts b/superset-frontend/src/explore/exploreUtils/index.ts index e0207f22dd7f..543f3f7b1fea 100644 --- a/superset-frontend/src/explore/exploreUtils/index.ts +++ b/superset-frontend/src/explore/exploreUtils/index.ts @@ -33,7 +33,7 @@ import { import { availableDomains } from 'src/utils/hostNamesConfig'; import { safeStringify } from 'src/utils/safeStringify'; import { optionLabel } from 'src/utils/common'; -import { ensureAppRoot } from 'src/utils/pathUtils'; +import { ensureAppRoot } from 'src/utils/navigationUtils'; import { URL_PARAMS } from 'src/constants'; import { DISABLE_INPUT_OPERATORS, diff --git a/superset-frontend/src/features/datasets/AddDataset/LeftPanel/index.tsx b/superset-frontend/src/features/datasets/AddDataset/LeftPanel/index.tsx index ee47bf18af5f..ef90f1815ebd 100644 --- a/superset-frontend/src/features/datasets/AddDataset/LeftPanel/index.tsx +++ b/superset-frontend/src/features/datasets/AddDataset/LeftPanel/index.tsx @@ -30,7 +30,7 @@ import { } from 'src/features/datasets/AddDataset/types'; import { Table } from 'src/hooks/apiResources'; import { Typography } from '@superset-ui/core/components/Typography'; -import { ensureAppRoot } from 'src/utils/pathUtils'; +import { ensureAppRoot } from 'src/utils/navigationUtils'; interface LeftPanelProps { setDataset: Dispatch>; diff --git a/superset-frontend/src/features/home/Menu.tsx b/superset-frontend/src/features/home/Menu.tsx index 06d606cc6a40..afc180acad29 100644 --- a/superset-frontend/src/features/home/Menu.tsx +++ b/superset-frontend/src/features/home/Menu.tsx @@ -20,7 +20,7 @@ import { useState, useEffect } from 'react'; import { styled, css, useTheme } from '@apache-superset/core/theme'; import { t } from '@apache-superset/core/translation'; import { ensureStaticPrefix } from 'src/utils/assetUrl'; -import { ensureAppRoot } from 'src/utils/pathUtils'; +import { ensureAppRoot } from 'src/utils/navigationUtils'; import { getUrlParam } from 'src/utils/urlUtils'; import { MainNav, MenuItem } from '@superset-ui/core/components/Menu'; import { Tooltip, Grid, Row, Col, Image } from '@superset-ui/core/components'; diff --git a/superset-frontend/src/features/home/RightMenu.tsx b/superset-frontend/src/features/home/RightMenu.tsx index 6462c1c588b7..99b0bdfbb636 100644 --- a/superset-frontend/src/features/home/RightMenu.tsx +++ b/superset-frontend/src/features/home/RightMenu.tsx @@ -44,7 +44,7 @@ import { TelemetryPixel, } from '@superset-ui/core/components'; import type { ItemType, MenuItem } from '@superset-ui/core/components/Menu'; -import { ensureAppRoot } from 'src/utils/pathUtils'; +import { ensureAppRoot } from 'src/utils/navigationUtils'; import { isEmbedded } from 'src/dashboard/util/isEmbedded'; import { findPermission } from 'src/utils/findPermission'; import { isUserAdmin } from 'src/dashboard/util/permissionUtils'; diff --git a/superset-frontend/src/features/home/SavedQueries.tsx b/superset-frontend/src/features/home/SavedQueries.tsx index 5e977dd8c7c2..0b4e3c359e91 100644 --- a/superset-frontend/src/features/home/SavedQueries.tsx +++ b/superset-frontend/src/features/home/SavedQueries.tsx @@ -45,7 +45,6 @@ import { shortenSQL, } from 'src/views/CRUD/utils'; import { assetUrl } from 'src/utils/assetUrl'; -import { ensureAppRoot } from 'src/utils/pathUtils'; import { navigateTo } from 'src/utils/navigationUtils'; import SubMenu from './SubMenu'; import EmptyState from './EmptyState'; @@ -306,7 +305,7 @@ export const SavedQueries = ({ { - window.location.href = '/'; + redirect('/'); }, []); if (!targetUrl) { diff --git a/superset-frontend/src/pages/SavedQueryList/index.tsx b/superset-frontend/src/pages/SavedQueryList/index.tsx index 1010585eb37d..35c92172519c 100644 --- a/superset-frontend/src/pages/SavedQueryList/index.tsx +++ b/superset-frontend/src/pages/SavedQueryList/index.tsx @@ -65,7 +65,7 @@ import copyTextToClipboard from 'src/utils/copy'; import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; import SavedQueryPreviewModal from 'src/features/queries/SavedQueryPreviewModal'; import { findPermission } from 'src/utils/findPermission'; -import { makeUrl } from 'src/utils/pathUtils'; +import { getShareableUrl, openInNewTab } from 'src/utils/navigationUtils'; const PAGE_SIZE = 25; const PASSWORDS_NEEDED_MESSAGE = t( @@ -233,11 +233,8 @@ function SavedQueryList({ // Action methods const openInSqlLab = (id: number, openInNewWindow: boolean) => { - copyTextToClipboard(() => - Promise.resolve( - `${window.location.origin}${makeUrl(`/sqllab?savedQueryId=${id}`)}`, - ), - ) + const path = `/sqllab?savedQueryId=${id}`; + copyTextToClipboard(() => Promise.resolve(getShareableUrl(path))) .then(() => { addSuccessToast(t('Link Copied!')); }) @@ -245,11 +242,11 @@ function SavedQueryList({ addDangerToast(t('Sorry, your browser does not support copying.')); }); if (openInNewWindow) { - window.open(makeUrl(`/sqllab?savedQueryId=${id}`)); + openInNewTab(path); } else { // React Router's basename already includes the application root; passing // a relative path ensures correct navigation under subdirectory deployments. - history.push(`/sqllab?savedQueryId=${id}`); + history.push(path); } }; diff --git a/superset-frontend/src/preamble.ts b/superset-frontend/src/preamble.ts index 57874fb537ef..e777a3e05914 100644 --- a/superset-frontend/src/preamble.ts +++ b/superset-frontend/src/preamble.ts @@ -26,7 +26,7 @@ import setupFormatters from './setup/setupFormatters'; import setupDashboardComponents from './setup/setupDashboardComponents'; import { User } from './types/bootstrapTypes'; import getBootstrapData, { applicationRoot } from './utils/getBootstrapData'; -import { makeUrl } from './utils/pathUtils'; +import { makeUrl } from './utils/navigationUtils'; import './hooks/useLocale'; // Import dayjs plugin types for global TypeScript support diff --git a/superset-frontend/src/utils/navigationUtils.AppLink.test.tsx b/superset-frontend/src/utils/navigationUtils.AppLink.test.tsx new file mode 100644 index 000000000000..8644efe01d7e --- /dev/null +++ b/superset-frontend/src/utils/navigationUtils.AppLink.test.tsx @@ -0,0 +1,69 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { render } from 'spec/helpers/testing-library'; +import { AppLink } from 'src/utils/navigationUtils'; + +// AppLink renders a real React element via React Testing Library, which is +// incompatible with the withApplicationRoot fixture's `jest.resetModules()` +// (it corrupts the testing-library module graph). Mock applicationRoot at +// file scope and vary per test instead. Variable must start with `mock` to +// satisfy Jest's hoisted-factory out-of-scope check. +const mockApplicationRoot = jest.fn(() => ''); + +jest.mock('src/utils/getBootstrapData', () => ({ + __esModule: true, + default: () => ({ + common: { application_root: '', static_assets_prefix: '' }, + }), + applicationRoot: () => mockApplicationRoot(), + staticAssetsPrefix: () => '', +})); + +beforeEach(() => { + mockApplicationRoot.mockReturnValue(''); +}); + +test('renders an anchor with prefixed href under subdirectory deployment', () => { + mockApplicationRoot.mockReturnValue('/superset'); + const { container } = render(go); + const anchor = container.querySelector('a'); + expect(anchor).not.toBeNull(); + expect(anchor?.getAttribute('href')).toBe('/superset/foo'); +}); + +test('passes through other anchor props', () => { + const { container } = render( + + go + , + ); + const anchor = container.querySelector('a'); + expect(anchor?.getAttribute('target')).toBe('_blank'); + expect(anchor?.getAttribute('rel')).toBe('noreferrer'); +}); + +test('passes absolute URLs through without prefixing', () => { + mockApplicationRoot.mockReturnValue('/superset'); + const { container } = render( + x, + ); + expect(container.querySelector('a')?.getAttribute('href')).toBe( + 'https://external.example.com', + ); +}); diff --git a/superset-frontend/src/utils/navigationUtils.invariants.test.ts b/superset-frontend/src/utils/navigationUtils.invariants.test.ts new file mode 100644 index 000000000000..d4946aef4d43 --- /dev/null +++ b/superset-frontend/src/utils/navigationUtils.invariants.test.ts @@ -0,0 +1,38 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { expectNoHits, scanSource } from 'spec/helpers/sourceTreeScanner'; + +test('no file outside navigationUtils.ts imports from pathUtils', () => { + // pathUtils.ts is the implementation module; navigationUtils.ts re-exports + // its helpers as the single sanctioned entry point for the rest of the + // codebase. Callers should `import { ensureAppRoot } from + // 'src/utils/navigationUtils'` (or use the focused helpers there). + const hits = scanSource({ + pattern: /from\s+['"](?:src\/utils\/pathUtils|\.\.?\/[\w./]*pathUtils)['"]/, + allowlist: ['src/utils/navigationUtils.ts'], + }); + + expectNoHits( + hits, + 'Found direct imports from src/utils/pathUtils. Import from ' + + 'src/utils/navigationUtils instead — it re-exports ensureAppRoot ' + + 'and makeUrl, and exposes focused helpers (openInNewTab, redirect, ' + + 'getShareableUrl, AppLink) for most call sites.', + ); +}); diff --git a/superset-frontend/src/utils/navigationUtils.test.ts b/superset-frontend/src/utils/navigationUtils.test.ts new file mode 100644 index 000000000000..aae326dded05 --- /dev/null +++ b/superset-frontend/src/utils/navigationUtils.test.ts @@ -0,0 +1,173 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { withApplicationRoot } from 'spec/helpers/withApplicationRoot'; + +// Surface any future hang as a Jest timeout instead of stalling CI. +jest.setTimeout(20000); + +describe('openInNewTab', () => { + let openSpy: jest.SpyInstance; + + beforeEach(() => { + openSpy = jest.spyOn(window, 'open').mockImplementation(() => null); + }); + + afterEach(() => { + openSpy.mockRestore(); + }); + + test('passes router-relative path through unchanged when application root is empty', async () => { + await withApplicationRoot('', async () => { + const { openInNewTab } = await import('src/utils/navigationUtils'); + openInNewTab('/sqllab?new=true'); + expect(openSpy).toHaveBeenCalledWith( + '/sqllab?new=true', + '_blank', + 'noopener noreferrer', + ); + }); + }); + + test('prefixes router-relative path with application root under subdirectory deployment', async () => { + await withApplicationRoot('/superset/', async () => { + const { openInNewTab } = await import('src/utils/navigationUtils'); + openInNewTab('/sqllab?new=true'); + expect(openSpy).toHaveBeenCalledWith( + '/superset/sqllab?new=true', + '_blank', + 'noopener noreferrer', + ); + }); + }); + + test('prefixes correctly for nested subdirectory roots', async () => { + await withApplicationRoot('/a/b/c/', async () => { + const { openInNewTab } = await import('src/utils/navigationUtils'); + openInNewTab('/dashboard/list'); + expect(openSpy).toHaveBeenCalledWith( + '/a/b/c/dashboard/list', + '_blank', + 'noopener noreferrer', + ); + }); + }); + + test('passes absolute URLs through unchanged regardless of application root', async () => { + await withApplicationRoot('/superset/', async () => { + const { openInNewTab } = await import('src/utils/navigationUtils'); + openInNewTab('https://external.example.com/docs'); + expect(openSpy).toHaveBeenCalledWith( + 'https://external.example.com/docs', + '_blank', + 'noopener noreferrer', + ); + }); + }); + + test('passes mailto: URLs through unchanged', async () => { + await withApplicationRoot('/superset/', async () => { + const { openInNewTab } = await import('src/utils/navigationUtils'); + openInNewTab('mailto:owner@example.com'); + expect(openSpy).toHaveBeenCalledWith( + 'mailto:owner@example.com', + '_blank', + 'noopener noreferrer', + ); + }); + }); + + test('uses noopener noreferrer for security on every call', async () => { + await withApplicationRoot('/superset/', async () => { + const { openInNewTab } = await import('src/utils/navigationUtils'); + openInNewTab('/sqllab'); + expect(openSpy).toHaveBeenCalledTimes(1); + const features = openSpy.mock.calls[0][2] as string; + expect(features).toContain('noopener'); + expect(features).toContain('noreferrer'); + }); + }); +}); + +describe('redirect', () => { + let originalLocation: Location; + + beforeEach(() => { + originalLocation = window.location; + delete (window as unknown as { location?: Location }).location; + (window as unknown as { location: { href: string } }).location = { + href: '', + } as Location; + }); + + afterEach(() => { + (window as unknown as { location: Location }).location = originalLocation; + }); + + test('sets window.location.href to the unprefixed path under empty root', async () => { + await withApplicationRoot('', async () => { + const { redirect } = await import('src/utils/navigationUtils'); + redirect('/'); + expect(window.location.href).toBe('/'); + }); + }); + + test('prefixes the path under a subdirectory deployment', async () => { + await withApplicationRoot('/superset/', async () => { + const { redirect } = await import('src/utils/navigationUtils'); + redirect('/'); + expect(window.location.href).toBe('/superset/'); + }); + }); + + test('passes absolute URLs through unchanged', async () => { + await withApplicationRoot('/superset/', async () => { + const { redirect } = await import('src/utils/navigationUtils'); + redirect('https://external.example.com/foo'); + expect(window.location.href).toBe('https://external.example.com/foo'); + }); + }); +}); + +describe('getShareableUrl', () => { + test('returns origin + unprefixed path under empty root', async () => { + await withApplicationRoot('', async () => { + const { getShareableUrl } = await import('src/utils/navigationUtils'); + expect(getShareableUrl('/sqllab?id=1')).toBe( + `${window.location.origin}/sqllab?id=1`, + ); + }); + }); + + test('returns origin + prefixed path under subdirectory deployment', async () => { + await withApplicationRoot('/superset/', async () => { + const { getShareableUrl } = await import('src/utils/navigationUtils'); + expect(getShareableUrl('/sqllab?id=1')).toBe( + `${window.location.origin}/superset/sqllab?id=1`, + ); + }); + }); +}); + +// AppLink renders a real React element, so its tests can't use +// withApplicationRoot — `jest.resetModules()` corrupts @testing-library/react +// when its dist files are re-imported across the reset. Mock applicationRoot +// at module scope and vary it per test instead. +// +// Note: the mock factory is hoisted, so `mockApplicationRoot` must be +// `mock`-prefixed to satisfy Jest's out-of-scope-variable check. diff --git a/superset-frontend/src/utils/navigationUtils.ts b/superset-frontend/src/utils/navigationUtils.ts index 11606aa7e3d9..448ddedf08cf 100644 --- a/superset-frontend/src/utils/navigationUtils.ts +++ b/superset-frontend/src/utils/navigationUtils.ts @@ -16,12 +16,27 @@ * specific language governing permissions and limitations * under the License. */ -import { ensureAppRoot } from './pathUtils'; +import { + createElement, + type AnchorHTMLAttributes, + type ReactElement, +} from 'react'; +import { ensureAppRoot, makeUrl } from './pathUtils'; -export const navigateTo = ( +// Re-export so callers that legitimately need a raw prefixed path (native +// fetch, navigator.sendBeacon, image src, third-party `href` props) have a +// single sanctioned import location. The static-invariant scan disallows +// importing from `pathUtils` directly outside this module. +export { ensureAppRoot, makeUrl }; + +// `navigateTo` and `navigateWithState` are declared first so the focused +// helpers below can call them without tripping oxlint's no-use-before-define +// (which does not honour function-declaration hoisting). + +export function navigateTo( url: string, options?: { newWindow?: boolean; assign?: boolean }, -) => { +): void { if (options?.newWindow) { window.open(ensureAppRoot(url), '_blank', 'noopener noreferrer'); } else if (options?.assign) { @@ -29,16 +44,72 @@ export const navigateTo = ( } else { window.location.href = ensureAppRoot(url); } -}; +} -export const navigateWithState = ( +export function navigateWithState( url: string, state: Record, options?: { replace?: boolean }, -) => { +): void { if (options?.replace) { window.history.replaceState(state, '', ensureAppRoot(url)); } else { window.history.pushState(state, '', ensureAppRoot(url)); } -}; +} + +const NEW_TAB_FEATURES = 'noopener noreferrer'; + +// Allow-list of safe URL shapes for navigation: relative paths, protocol- +// relative URLs, and a small set of known-safe schemes. `ensureAppRoot` +// already neutralises `javascript:` / `data:` by prefixing them as relative +// paths, but checking here gives CodeQL a locally-visible sanitiser on the +// sinks below. +const SAFE_NAVIGATION_URL_RE = /^(?:\/(?!\/)|\/\/|https?:|ftp:|mailto:|tel:)/i; + +function assertSafeNavigationUrl(url: string): string { + if (!SAFE_NAVIGATION_URL_RE.test(url)) { + throw new Error( + 'navigationUtils refused unsafe URL: only relative paths and ' + + 'http(s):, ftp:, mailto:, tel: schemes are allowed.', + ); + } + return url; +} + +/** Open a router-relative path in a new browser tab. */ +export function openInNewTab(path: string): void { + window.open( + assertSafeNavigationUrl(ensureAppRoot(path)), + '_blank', + NEW_TAB_FEATURES, + ); +} + +/** + * Full-page redirect to a router-relative path. Use only when the destination + * is outside the React Router tree or a hard reload is required. + */ +export function redirect(path: string): void { + navigateTo(path); +} + +/** Build a `${origin}${appRoot}${path}` URL for clipboard / share targets. */ +export function getShareableUrl(path: string): string { + const safePath = assertSafeNavigationUrl(ensureAppRoot(path)); + return `${window.location.origin}${safePath}`; +} + +/** + * Anchor element that prefixes its href with the application root. Use + * instead of `` whenever the href is computed at runtime. + */ +export function AppLink( + props: AnchorHTMLAttributes & { href: string }, +): ReactElement { + const { href, ...rest } = props; + return createElement('a', { + ...rest, + href: assertSafeNavigationUrl(ensureAppRoot(href)), + }); +} diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index fa69b5840707..f250676d4bb5 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -42,7 +42,7 @@ import type { } from 'src/components'; import Chart, { Slice } from 'src/types/Chart'; import copyTextToClipboard from 'src/utils/copy'; -import { ensureAppRoot } from 'src/utils/pathUtils'; +import { getShareableUrl } from 'src/utils/navigationUtils'; import SupersetText from 'src/utils/textUtils'; import { DatabaseObject } from 'src/features/databases/types'; import { @@ -747,9 +747,7 @@ export const copyQueryLink = ( addSuccessToast: (arg0: string) => void, ) => { copyTextToClipboard(() => - Promise.resolve( - `${window.location.origin}${ensureAppRoot(`/sqllab?savedQueryId=${id}`)}`, - ), + Promise.resolve(getShareableUrl(`/sqllab?savedQueryId=${id}`)), ) .then(() => { addSuccessToast(t('Link Copied!'));