This is an automated email from the ASF dual-hosted git repository. jli pushed a commit to branch fix-app-prefix-export in repository https://gitbox.apache.org/repos/asf/superset.git
commit a1b86aa87b0940ae1efd67d99b8d55b7f8584320 Author: Joe Li <[email protected]> AuthorDate: Fri Dec 19 00:00:39 2025 -0800 feat(streaming-export): add URL prefix guard to prevent regression Adds ensureUrlPrefix guard in useStreamingExport that automatically applies makeUrl to relative URLs missing the app root prefix. This prevents future regressions where callers forget to prefix URLs for subdirectory deployments. The guard: - Applies prefix to unprefixed relative URLs when app root is configured - Skips already-prefixed URLs to prevent double-prefixing - Leaves URLs unchanged when no app root is configured 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> --- .../useStreamingExport.test.ts | 119 +++++++++++++++++++++ .../StreamingExportModal/useStreamingExport.ts | 29 ++++- 2 files changed, 147 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts index 7fe03cd621..5297576e9b 100644 --- a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts +++ b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts @@ -29,6 +29,15 @@ jest.mock('@superset-ui/core', () => ({ }, })); +// Mock pathUtils and getBootstrapData for URL prefix guard tests +jest.mock('src/utils/pathUtils', () => ({ + makeUrl: jest.fn((path: string) => path), +})); + +jest.mock('src/utils/getBootstrapData', () => ({ + applicationRoot: jest.fn(() => ''), +})); + global.URL.createObjectURL = jest.fn(() => 'blob:mock-url'); global.URL.revokeObjectURL = jest.fn(); @@ -207,3 +216,113 @@ test('sets ERROR status and calls onError when fetch rejects', async () => { expect(onError).toHaveBeenCalledTimes(1); expect(onError).toHaveBeenCalledWith(errorMessage); }); + +// URL prefix guard tests - prevent regression of missing app root prefix +describe('URL prefix guard', () => { + const { applicationRoot } = jest.requireMock('src/utils/getBootstrapData'); + const { makeUrl } = jest.requireMock('src/utils/pathUtils'); + + const createMockFetch = () => + jest.fn().mockResolvedValue({ + ok: true, + headers: new Headers({ + 'Content-Disposition': 'attachment; filename="export.csv"', + }), + body: { + getReader: () => ({ + read: jest.fn().mockResolvedValue({ done: true, value: undefined }), + }), + }, + }); + + beforeEach(() => { + applicationRoot.mockReturnValue(''); + makeUrl.mockImplementation((path: string) => path); + }); + + test('applies prefix to unprefixed relative URL when app root is configured', async () => { + const appRoot = '/superset'; + applicationRoot.mockReturnValue(appRoot); + makeUrl.mockImplementation((path: string) => `${appRoot}${path}`); + + const mockFetch = createMockFetch(); + global.fetch = mockFetch; + + const { result } = renderHook(() => useStreamingExport()); + + act(() => { + result.current.startExport({ + url: '/api/v1/sqllab/export_streaming/', + payload: { client_id: 'test-id' }, + exportType: 'csv', + }); + }); + + await waitFor(() => { + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + // Guard should have applied the prefix + expect(mockFetch).toHaveBeenCalledWith( + '/superset/api/v1/sqllab/export_streaming/', + expect.any(Object), + ); + }); + + test('does not double-prefix URL that already has app root', async () => { + const appRoot = '/superset'; + applicationRoot.mockReturnValue(appRoot); + makeUrl.mockImplementation((path: string) => `${appRoot}${path}`); + + const mockFetch = createMockFetch(); + global.fetch = mockFetch; + + const { result } = renderHook(() => useStreamingExport()); + + // URL already has prefix (caller did the right thing) + act(() => { + result.current.startExport({ + url: '/superset/api/v1/sqllab/export_streaming/', + payload: { client_id: 'test-id' }, + exportType: 'csv', + }); + }); + + await waitFor(() => { + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + // Should NOT double-prefix + expect(mockFetch).toHaveBeenCalledWith( + '/superset/api/v1/sqllab/export_streaming/', + expect.any(Object), + ); + }); + + test('leaves URL unchanged when no app root is configured', async () => { + applicationRoot.mockReturnValue(''); + + const mockFetch = createMockFetch(); + global.fetch = mockFetch; + + const { result } = renderHook(() => useStreamingExport()); + + act(() => { + result.current.startExport({ + url: '/api/v1/sqllab/export_streaming/', + payload: { client_id: 'test-id' }, + exportType: 'csv', + }); + }); + + await waitFor(() => { + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + // No prefix should be applied + expect(mockFetch).toHaveBeenCalledWith( + '/api/v1/sqllab/export_streaming/', + expect.any(Object), + ); + }); +}); diff --git a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts index fd0bf34cb2..945827d371 100644 --- a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts +++ b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts @@ -19,6 +19,8 @@ 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 { applicationRoot } from 'src/utils/getBootstrapData'; interface UseStreamingExportOptions { onComplete?: (downloadUrl: string, filename: string) => void; @@ -39,6 +41,29 @@ interface StreamingExportParams { const NEWLINE_BYTE = 10; // '\n' character code +/** + * Ensures URL has the application root prefix for subdirectory deployments. + * Applies makeUrl to relative paths that don't already include the app root. + * This guards against callers forgetting to prefix URLs when using native fetch. + */ +const ensureUrlPrefix = (url: string): string => { + const appRoot = applicationRoot(); + // Only process relative URLs (starting with /) + if (!url.startsWith('/')) { + return url; + } + // If no app root configured, return as-is + if (!appRoot) { + return url; + } + // If URL already has the app root prefix, return as-is + if (url.startsWith(appRoot)) { + return url; + } + // Apply prefix via makeUrl + return makeUrl(url); +}; + const createFetchRequest = async ( _url: string, payload: StreamingExportPayload, @@ -157,7 +182,9 @@ export const useStreamingExport = (options: UseStreamingExportOptions = {}) => { expectedRows, abortControllerRef.current.signal, ); - const response = await fetch(url, fetchOptions); + // Guard: ensure URL has app root prefix for subdirectory deployments + const prefixedUrl = ensureUrlPrefix(url); + const response = await fetch(prefixedUrl, fetchOptions); if (!response.ok) { throw new Error(
