diff --git a/src/components/sharelink/OpenInSiftButton.test.tsx b/src/components/sharelink/OpenInSiftButton.test.tsx index e818b50..2812d37 100644 --- a/src/components/sharelink/OpenInSiftButton.test.tsx +++ b/src/components/sharelink/OpenInSiftButton.test.tsx @@ -64,7 +64,7 @@ describe('OpenInSiftButton', () => { const button = screen.getByRole('button', { name: 'Open in Sift' }); fireEvent.click(button); - expect(openSpy).toHaveBeenCalledWith('https://sift.example.com/explorer', '_blank'); + expect(openSpy).toHaveBeenCalledWith('https://sift.example.com/explorer', '_blank', 'noopener,noreferrer'); openSpy.mockRestore(); }); @@ -96,7 +96,7 @@ describe('OpenInSiftButton', () => { const button = screen.getByRole('button', { name: 'Open in Sift' }); fireEvent.click(button); - expect(openSpy).toHaveBeenCalledWith('https://sift.example.com/explorer', '_blank'); + expect(openSpy).toHaveBeenCalledWith('https://sift.example.com/explorer', '_blank', 'noopener,noreferrer'); openSpy.mockRestore(); }); @@ -148,7 +148,7 @@ describe('OpenInSiftButton', () => { const button = screen.getByRole('button', { name: 'Open in Sift' }); fireEvent.click(button); - expect(openSpy).toHaveBeenCalledWith('https://sift.example.com/explorer', '_blank'); + expect(openSpy).toHaveBeenCalledWith('https://sift.example.com/explorer', '_blank', 'noopener,noreferrer'); openSpy.mockRestore(); }); @@ -210,4 +210,56 @@ describe('OpenInSiftButton', () => { expect(screen.getByRole('menuitem', { name: /Open Link/i })).toBeDisabled(); }); }); + + it('handles errors from generateLinkFromQuery gracefully', async () => { + const items = { + channelIds: ['channel-1'], + assetIds: ['asset-1'], + runIds: ['run-1'], + calculatedChannels: [], + }; + + generateLinkFromQueryMock.mockImplementation(() => { + throw new Error('Test error'); + }); + + render(); + + expect(generateLinkFromQueryMock).toHaveBeenCalled(); + expect(errorSpy).toHaveBeenCalledWith('Failed to generate share link:', expect.any(Error)); + + const menuButton = screen.getByRole('button', { name: 'Open in Sift' }); + expect(menuButton).toHaveAttribute('aria-disabled', 'true'); + + fireEvent.contextMenu(menuButton); + + await waitFor(() => { + expect(screen.getByRole('menuitem', { name: /Open Link/i })).toBeDisabled(); + }); + }); + + it('handles null hostname from getFrontendHostnameDefaults', async () => { + const items = { + channelIds: ['channel-1'], + assetIds: ['asset-1'], + runIds: ['run-1'], + calculatedChannels: [], + }; + + getFrontendHostnameDefaultsMock.mockReturnValue(null); + + render(); + + expect(getFrontendHostnameDefaultsMock).toHaveBeenCalledWith('https://unknown-api.example.com'); + expect(generateLinkFromQueryMock).not.toHaveBeenCalled(); + + const menuButton = screen.getByRole('button', { name: 'Open in Sift' }); + expect(menuButton).toHaveAttribute('aria-disabled', 'true'); + + fireEvent.contextMenu(menuButton); + + await waitFor(() => { + expect(screen.getByRole('menuitem', { name: /Open Link/i })).toBeDisabled(); + }); + }); }); diff --git a/src/components/sharelink/OpenInSiftButton.tsx b/src/components/sharelink/OpenInSiftButton.tsx index 79d1ce9..f1c9026 100644 --- a/src/components/sharelink/OpenInSiftButton.tsx +++ b/src/components/sharelink/OpenInSiftButton.tsx @@ -16,7 +16,7 @@ interface SharelinkMenuItemProps { } function openLink(link: string) { - window.open(link, '_blank'); + window.open(link, '_blank', 'noopener,noreferrer'); } async function copyToClipboard(value: string) { @@ -63,10 +63,19 @@ export const OpenInSiftButton = ({ className, items, apiBaseUrl, frontendUrl, ti disabledReason: 'Configure the Sift API REST URL to enable share links', }; } - return { - shareLink: generateLinkFromQuery(hostname, items, timeRange), - disabledReason: undefined, - }; + + try { + return { + shareLink: generateLinkFromQuery(hostname, items, timeRange), + disabledReason: undefined, + }; + } catch (error) { + console.error('Failed to generate share link:', error); + return { + shareLink: null, + disabledReason: 'Failed to generate share link', + }; + } }, [apiBaseUrl, frontendUrl, items, timeRange]); return ( diff --git a/src/components/sharelink/generateLinkFromQuery.test.ts b/src/components/sharelink/generateLinkFromQuery.test.ts new file mode 100644 index 0000000..18cffaf --- /dev/null +++ b/src/components/sharelink/generateLinkFromQuery.test.ts @@ -0,0 +1,250 @@ +import { generateLinkFromQuery } from './generateLinkFromQuery'; +import type { SharelinkItems, SharelinkTimeRange } from '../../types'; + +describe('generateLinkFromQuery', () => { + const mockItems: SharelinkItems = { + channelIds: ['channel-1', 'channel-2'], + assetIds: ['asset-1'], + runIds: ['run-1'], + calculatedChannels: [], + }; + + describe('URL construction with hostname normalization', () => { + it('should handle hostname without protocol and prepend https://', () => { + const result = generateLinkFromQuery('app.siftstack.com', mockItems); + + expect(result).toMatch(/^https:\/\/app\.siftstack\.com\/explorer#/); + }); + + it('should handle hostname with https:// protocol', () => { + const result = generateLinkFromQuery('https://app.siftstack.com', mockItems); + + expect(result).toMatch(/^https:\/\/app\.siftstack\.com\/explorer#/); + }); + + it('should handle hostname with http:// protocol', () => { + const result = generateLinkFromQuery('http://localhost:3000', mockItems); + + expect(result).toMatch(/^http:\/\/localhost:3000\/explorer#/); + }); + + it('should handle hostname with port', () => { + const result = generateLinkFromQuery('localhost:8080', mockItems); + + expect(result).toMatch(/^https:\/\/localhost:8080\/explorer#/); + }); + + it('should extract origin from full URL with path', () => { + const result = generateLinkFromQuery('https://app.siftstack.com/some/path', mockItems); + + // Should use only the origin, not the path + expect(result).toMatch(/^https:\/\/app\.siftstack\.com\/explorer#/); + expect(result).not.toContain('/some/path'); + }); + + it('should handle URL with query parameters by using only origin', () => { + const result = generateLinkFromQuery('https://app.siftstack.com?foo=bar', mockItems); + + expect(result).toMatch(/^https:\/\/app\.siftstack\.com\/explorer#/); + expect(result).not.toContain('?foo=bar'); + }); + + it('should handle URL with trailing slash', () => { + const result = generateLinkFromQuery('https://app.siftstack.com/', mockItems); + + expect(result).toMatch(/^https:\/\/app\.siftstack\.com\/explorer#/); + }); + }); + + describe('hash parameters encoding', () => { + it('should include assets in hash', () => { + const items: SharelinkItems = { + channelIds: ['channel-1'], + assetIds: ['asset-1', 'asset-2'], + runIds: [], + calculatedChannels: [], + }; + + const result = generateLinkFromQuery('app.siftstack.com', items); + + expect(result).toContain('assets=asset-1%2Casset-2'); + }); + + it('should include runs in hash', () => { + const items: SharelinkItems = { + channelIds: ['channel-1'], + assetIds: [], + runIds: ['run-1', 'run-2'], + calculatedChannels: [], + }; + + const result = generateLinkFromQuery('app.siftstack.com', items); + + expect(result).toContain('runs=run-1%2Crun-2'); + }); + + it('should base64 encode legend configuration', () => { + const result = generateLinkFromQuery('app.siftstack.com', mockItems); + + // Should contain base64 encoded legend + expect(result).toContain('legend='); + // Base64 strings typically contain these characters + expect(result).toMatch(/legend=[A-Za-z0-9+/=%]+/); + }); + + it('should include time range in legend when provided', () => { + const timeRange: SharelinkTimeRange = { + from: '2024-01-01T00:00:00Z', + to: '2024-01-02T00:00:00Z', + }; + + const result = generateLinkFromQuery('app.siftstack.com', mockItems, timeRange); + + // The time range should be encoded in the legend parameter + expect(result).toContain('legend='); + }); + + it('should handle calculated channels', () => { + const itemsWithCalc: SharelinkItems = { + channelIds: ['channel-1'], + assetIds: ['asset-1'], + runIds: ['run-1'], + calculatedChannels: [ + { + name: 'Calculated Channel', + sourceChannels: ['channel-1', 'channel-2'], + expression: '$1 + $2', + expressionDataType: 'DOUBLE', + }, + ], + }; + + const result = generateLinkFromQuery('app.siftstack.com', itemsWithCalc); + + // Should still generate a valid URL with legend + expect(result).toMatch(/^https:\/\/app\.siftstack\.com\/explorer#/); + expect(result).toContain('legend='); + }); + + it('should handle multiple calculated channels', () => { + const itemsWithMultipleCalc: SharelinkItems = { + channelIds: ['channel-1'], + assetIds: ['asset-1'], + runIds: ['run-1'], + calculatedChannels: [ + { + name: 'Calc 1', + sourceChannels: ['channel-1'], + expression: '$1 * 2', + expressionDataType: 'DOUBLE', + }, + { + name: 'Calc 2', + sourceChannels: ['channel-1', 'channel-2'], + expression: '$1 + $2', + expressionDataType: 'DOUBLE', + }, + ], + }; + + const result = generateLinkFromQuery('app.siftstack.com', itemsWithMultipleCalc); + + expect(result).toMatch(/^https:\/\/app\.siftstack\.com\/explorer#/); + expect(result).toContain('legend='); + }); + }); + + describe('edge cases', () => { + it('should handle empty asset and run arrays', () => { + const items: SharelinkItems = { + channelIds: ['channel-1'], + assetIds: [], + runIds: [], + calculatedChannels: [], + }; + + const result = generateLinkFromQuery('app.siftstack.com', items); + + expect(result).toMatch(/^https:\/\/app\.siftstack\.com\/explorer#/); + // Should not include assets or runs parameters + expect(result).not.toContain('assets='); + expect(result).not.toContain('runs='); + }); + + it('should handle undefined asset and run arrays', () => { + const items: SharelinkItems = { + channelIds: ['channel-1'], + assetIds: undefined, + runIds: undefined, + calculatedChannels: [], + }; + + const result = generateLinkFromQuery('app.siftstack.com', items); + + expect(result).toMatch(/^https:\/\/app\.siftstack\.com\/explorer#/); + expect(result).not.toContain('assets='); + expect(result).not.toContain('runs='); + }); + + it('should handle special characters in channel IDs', () => { + const items: SharelinkItems = { + channelIds: ['channel-with-dash', 'channel_with_underscore'], + assetIds: ['asset-1'], + runIds: ['run-1'], + calculatedChannels: [], + }; + + const result = generateLinkFromQuery('app.siftstack.com', items); + + expect(result).toMatch(/^https:\/\/app\.siftstack\.com\/explorer#/); + expect(result).toContain('legend='); + }); + + it('should handle IPv4 addresses', () => { + const result = generateLinkFromQuery('192.168.1.1:8080', mockItems); + + expect(result).toMatch(/^https:\/\/192\.168\.1\.1:8080\/explorer#/); + }); + + it('should handle localhost variations', () => { + const result1 = generateLinkFromQuery('localhost', mockItems); + expect(result1).toMatch(/^https:\/\/localhost\/explorer#/); + + const result2 = generateLinkFromQuery('http://localhost:3000', mockItems); + expect(result2).toMatch(/^http:\/\/localhost:3000\/explorer#/); + }); + }); + + describe('URL structure validation', () => { + it('should generate valid URL structure', () => { + const result = generateLinkFromQuery('app.siftstack.com', mockItems); + + // Should be a valid URL + expect(() => new URL(result)).not.toThrow(); + + const url = new URL(result); + expect(url.protocol).toMatch(/^https?:$/); + expect(url.pathname).toBe('/explorer'); + expect(url.hash).toMatch(/^#.+/); + }); + + it('should properly encode hash parameters', () => { + const result = generateLinkFromQuery('app.siftstack.com', mockItems); + const url = new URL(result); + + // Hash should be parseable as URLSearchParams (without the # prefix) + const hashParams = new URLSearchParams(url.hash.slice(1)); + + expect(hashParams.has('legend')).toBe(true); + expect(hashParams.has('assets')).toBe(true); + expect(hashParams.has('runs')).toBe(true); + }); + + it('should not double-encode URL components', () => { + const result = generateLinkFromQuery('app.siftstack.com', mockItems); + + // Should not contain double-encoded characters like %25 (encoded %) + expect(result).not.toContain('%25'); + }); + }); +}); diff --git a/src/components/sharelink/generateLinkFromQuery.ts b/src/components/sharelink/generateLinkFromQuery.ts index 263a133..4559aaa 100644 --- a/src/components/sharelink/generateLinkFromQuery.ts +++ b/src/components/sharelink/generateLinkFromQuery.ts @@ -115,11 +115,8 @@ function normaliseBasePath(basePath: string): string { if (!basePath.startsWith('/')) { return `/${basePath}`; } - return basePath; -} - -function stripTrailingSlash(input: string): string { - return input.endsWith('/') ? input.slice(0, -1) : input; + // Remove trailing slash for consistency + return basePath.endsWith('/') ? basePath.slice(0, -1) : basePath; } function setIfPresent(hash: URLSearchParams, key: string, value: string | undefined | null) { @@ -181,20 +178,55 @@ function createExplorerLink(params: ExplorerLinkParams): string { const hashString = hashParams.toString(); - let url = basePath; - if (hashString) { - url += `#${hashString}`; - } - + // If origin is provided, use URL constructor for proper URL building if (params.origin) { - url = `${stripTrailingSlash(params.origin)}${url}`; + try { + const url = new URL(basePath, params.origin); + if (hashString) { + url.hash = hashString; + } + return url.toString(); + } catch { + // If URL construction fails, fall back to string concatenation + // This shouldn't happen if origin is properly validated + const fullUrl = `${params.origin}${basePath}`; + return hashString ? `${fullUrl}#${hashString}` : fullUrl; + } } - return url; + // Return relative URL + return hashString ? `${basePath}#${hashString}` : basePath; } export function generateLinkFromQuery(hostname: string, items: SharelinkItems, timeRange?: SharelinkTimeRange) { - const origin = hostname.startsWith('http://') || hostname.startsWith('https://') ? hostname : `https://${hostname}`; + // Guard against null/undefined hostname + if (!hostname) { + throw new Error('hostname is required'); + } + + // Normalize hostname to a valid origin URL + let origin: string; + try { + // If hostname is already a valid URL, use it directly + const testUrl = new URL(hostname); + // Check if origin is valid (not null or 'null') + if (testUrl.origin && testUrl.origin !== 'null') { + origin = testUrl.origin; + } else { + // URL parsed but origin is invalid, treat as hostname + throw new Error('Invalid origin'); + } + } catch { + // If not a valid URL, assume it's a hostname and prepend https:// + const withProtocol = `https://${hostname}`; + try { + const validatedUrl = new URL(withProtocol); + origin = validatedUrl.origin; + } catch { + // If still invalid, fall back to the string (shouldn't happen with valid hostnames) + origin = withProtocol; + } + } const channelIds = items.channelIds; const channelKeys = channelIds.map((_, index) => `channel-key-${index + 1}`); const legendChannels: LegendConfigPayload['channels'] = {}; diff --git a/src/components/sharelink/getFrontendHostnameDefaults.test.ts b/src/components/sharelink/getFrontendHostnameDefaults.test.ts new file mode 100644 index 0000000..6bc5262 --- /dev/null +++ b/src/components/sharelink/getFrontendHostnameDefaults.test.ts @@ -0,0 +1,176 @@ +import { getFrontendHostnameDefaults } from './getFrontendHostnameDefaults'; + +describe('getFrontendHostnameDefaults', () => { + describe('known API endpoints', () => { + it('should map api.siftstack.com to app.siftstack.com', () => { + expect(getFrontendHostnameDefaults('api.siftstack.com')).toBe('app.siftstack.com'); + }); + + it('should map api.siftstack.com with https:// to app.siftstack.com', () => { + expect(getFrontendHostnameDefaults('https://api.siftstack.com')).toBe('app.siftstack.com'); + }); + + it('should map api.siftstack.com with http:// to app.siftstack.com', () => { + expect(getFrontendHostnameDefaults('http://api.siftstack.com')).toBe('app.siftstack.com'); + }); + + it('should map gov.api.siftstack.com to gov.siftstack.com', () => { + expect(getFrontendHostnameDefaults('gov.api.siftstack.com')).toBe('gov.siftstack.com'); + }); + + it('should map gov.api.siftstack.com with https:// to gov.siftstack.com', () => { + expect(getFrontendHostnameDefaults('https://gov.api.siftstack.com')).toBe('gov.siftstack.com'); + }); + + it('should map localhost:8080 to http://localhost:3000', () => { + expect(getFrontendHostnameDefaults('localhost:8080')).toBe('http://localhost:3000'); + }); + + it('should map localhost:8080 with http:// to http://localhost:3000', () => { + expect(getFrontendHostnameDefaults('http://localhost:8080')).toBe('http://localhost:3000'); + }); + + it('should map host.docker.internal:8080 to http://localhost:3000', () => { + expect(getFrontendHostnameDefaults('host.docker.internal:8080')).toBe('http://localhost:3000'); + }); + + it('should map host.docker.internal:8080 with http:// to http://localhost:3000', () => { + expect(getFrontendHostnameDefaults('http://host.docker.internal:8080')).toBe('http://localhost:3000'); + }); + }); + + describe('URL parsing with protocol', () => { + it('should extract host from URL with https protocol', () => { + expect(getFrontendHostnameDefaults('https://api.siftstack.com')).toBe('app.siftstack.com'); + }); + + it('should extract host from URL with http protocol', () => { + expect(getFrontendHostnameDefaults('http://api.siftstack.com')).toBe('app.siftstack.com'); + }); + + it('should extract host from URL with path', () => { + expect(getFrontendHostnameDefaults('https://api.siftstack.com/some/path')).toBe('app.siftstack.com'); + }); + + it('should extract host from URL with query parameters', () => { + expect(getFrontendHostnameDefaults('https://api.siftstack.com?foo=bar')).toBe('app.siftstack.com'); + }); + + it('should extract host from URL with port', () => { + expect(getFrontendHostnameDefaults('https://api.siftstack.com:8443')).toBe(null); + }); + + it('should handle URL with trailing slash', () => { + expect(getFrontendHostnameDefaults('https://api.siftstack.com/')).toBe('app.siftstack.com'); + }); + }); + + describe('hostname without protocol', () => { + it('should handle plain hostname', () => { + expect(getFrontendHostnameDefaults('api.siftstack.com')).toBe('app.siftstack.com'); + }); + + it('should handle hostname with port', () => { + expect(getFrontendHostnameDefaults('localhost:8080')).toBe('http://localhost:3000'); + }); + + it('should handle hostname with subdomain', () => { + expect(getFrontendHostnameDefaults('gov.api.siftstack.com')).toBe('gov.siftstack.com'); + }); + }); + + describe('unknown endpoints', () => { + it('should return null for unknown hostname', () => { + expect(getFrontendHostnameDefaults('unknown.example.com')).toBe(null); + }); + + it('should return null for unknown hostname with https://', () => { + expect(getFrontendHostnameDefaults('https://unknown.example.com')).toBe(null); + }); + + it('should return null for unknown hostname with port', () => { + expect(getFrontendHostnameDefaults('unknown.example.com:8080')).toBe(null); + }); + + it('should return null for unknown localhost port', () => { + expect(getFrontendHostnameDefaults('localhost:9999')).toBe(null); + }); + }); + + describe('edge cases', () => { + it('should return null for empty string', () => { + expect(getFrontendHostnameDefaults('')).toBe(null); + }); + + it('should return null for whitespace only', () => { + expect(getFrontendHostnameDefaults(' ')).toBe(null); + }); + + it('should handle URL with extra whitespace', () => { + expect(getFrontendHostnameDefaults(' https://api.siftstack.com ')).toBe('app.siftstack.com'); + }); + + it('should handle hostname with extra whitespace', () => { + expect(getFrontendHostnameDefaults(' api.siftstack.com ')).toBe('app.siftstack.com'); + }); + + it('should handle IPv4 address', () => { + expect(getFrontendHostnameDefaults('192.168.1.1:8080')).toBe(null); + }); + + it('should handle IPv4 address with protocol', () => { + expect(getFrontendHostnameDefaults('http://192.168.1.1:8080')).toBe(null); + }); + }); + + describe('URL constructor behavior', () => { + it('should properly extract host from full URL', () => { + const result = getFrontendHostnameDefaults('https://api.siftstack.com:443/api/v1'); + // Port 443 is default for https, so it should be stripped by URL.host + expect(result).toBe('app.siftstack.com'); + }); + + it('should preserve non-default ports in host extraction', () => { + const result = getFrontendHostnameDefaults('http://localhost:8080/api'); + expect(result).toBe('http://localhost:3000'); + }); + + it('should handle URLs with fragments', () => { + const result = getFrontendHostnameDefaults('https://api.siftstack.com#section'); + expect(result).toBe('app.siftstack.com'); + }); + + it('should handle URLs with authentication', () => { + const result = getFrontendHostnameDefaults('https://user:pass@api.siftstack.com'); + expect(result).toBe('app.siftstack.com'); + }); + }); + + describe('protocol variations', () => { + it('should handle uppercase protocol', () => { + expect(getFrontendHostnameDefaults('HTTPS://api.siftstack.com')).toBe('app.siftstack.com'); + }); + + it('should handle mixed case protocol', () => { + expect(getFrontendHostnameDefaults('HtTpS://api.siftstack.com')).toBe('app.siftstack.com'); + }); + + it('should handle http protocol for known endpoint', () => { + expect(getFrontendHostnameDefaults('http://api.siftstack.com')).toBe('app.siftstack.com'); + }); + }); + + describe('special localhost cases', () => { + it('should map localhost:8080 regardless of protocol', () => { + expect(getFrontendHostnameDefaults('localhost:8080')).toBe('http://localhost:3000'); + expect(getFrontendHostnameDefaults('http://localhost:8080')).toBe('http://localhost:3000'); + expect(getFrontendHostnameDefaults('https://localhost:8080')).toBe('http://localhost:3000'); + }); + + it('should map host.docker.internal:8080 regardless of protocol', () => { + expect(getFrontendHostnameDefaults('host.docker.internal:8080')).toBe('http://localhost:3000'); + expect(getFrontendHostnameDefaults('http://host.docker.internal:8080')).toBe('http://localhost:3000'); + expect(getFrontendHostnameDefaults('https://host.docker.internal:8080')).toBe('http://localhost:3000'); + }); + }); +}); diff --git a/src/components/sharelink/getFrontendHostnameDefaults.ts b/src/components/sharelink/getFrontendHostnameDefaults.ts index 48a9286..ca713fe 100644 --- a/src/components/sharelink/getFrontendHostnameDefaults.ts +++ b/src/components/sharelink/getFrontendHostnameDefaults.ts @@ -7,7 +7,21 @@ export function getFrontendHostnameDefaults(apiBaseUrl: string): string | null { return null; } - const cleanUrl = apiBaseUrl.replace(/^https?:\/\//, '').trim(); + // Use URL constructor to properly parse the URL and extract the host + let cleanUrl: string; + try { + const url = new URL(apiBaseUrl); + // Check if we got a valid absolute URL (origin should not be null) + if (url.origin && url.origin !== 'null' && url.host) { + cleanUrl = url.host; // host includes port if present + } else { + // URL parsed as relative, treat as hostname + throw new Error('Relative URL'); + } + } catch { + // If not a valid URL, assume it's already a hostname and use as-is + cleanUrl = apiBaseUrl.trim(); + } switch (cleanUrl) { case 'api.siftstack.com': diff --git a/src/resources.hooks.ts b/src/resources.hooks.ts index 4517f86..09c555a 100644 --- a/src/resources.hooks.ts +++ b/src/resources.hooks.ts @@ -11,7 +11,7 @@ import { } from './types'; import { getTemplateSrv, getAppEvents, RefreshEvent } from '@grafana/runtime'; import { TypedVariableModel, BusEventWithPayload } from '@grafana/data'; -import { CELUtil, replaceTemplateVariablesInQuery } from './utils'; +import { CELUtil } from './utils'; import { debounce } from 'lodash'; import leven from 'leven';