From 095821264e71aee87dff542bff7cc995a4bb446b Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Fri, 28 Aug 2026 06:40:25 +0000 Subject: [PATCH] fix(explorer): guard saved-view URL params against non-JSON values (#12706) #### Description - Opening an explorer with a `viewName`/`viewKey` in the URL that isn't valid JSON crashed the whole page. - The hook ran `JSON.parse` on the raw param during render, so a bare saved-view name threw error and broke page. - Fix: wrap `JSON.parse` in try/catch and fall back to the raw string. - Renamed the hook to `useGetSavedViewParams` and moved it under `hooks/saveViews`; it only reads `viewName`/`viewKey` so the old query-builder name/location was misleading. Now returns `{ viewName, viewKey }` directly. - Behavior preserved for all consumers; added tests for the non-JSON case. Closes https://github.com/SigNoz/engineering-pod/issues/5638 Screenshots/Recording Before: Test url: Just remove quotes from viewKey or viewName: [url](https://app.us.staging.signoz.cloud/logs/logs-explorer?relativeTime=1month&compositeQuery=%257B%2522queryType%2522%253A%2522builder%2522%252C%2522builder%2522%253A%257B%2522queryData%2522%253A%255B%257B%2522dataSource%2522%253A%2522logs%2522%252C%2522queryName%2522%253A%2522A%2522%252C%2522aggregateOperator%2522%253A%2522count%2522%252C%2522aggregateAttribute%2522%253A%257B%2522id%2522%253A%2522----%2522%252C%2522dataType%2522%253A%2522%2522%252C%2522key%2522%253A%2522%2522%252C%2522type%2522%253A%2522%2522%257D%252C%2522timeAggregation%2522%253A%2522rate%2522%252C%2522spaceAggregation%2522%253A%2522sum%2522%252C%2522filter%2522%253A%257B%2522expression%2522%253A%2522%2522%257D%252C%2522aggregations%2522%253A%255B%257B%2522expression%2522%253A%2522count%28%29%2522%257D%255D%252C%2522functions%2522%253Anull%252C%2522filters%2522%253A%257B%2522items%2522%253A%255B%257B%2522id%2522%253A%2522228b8a2f-d6ba-4704-9104-936e91a2c119%2522%252C%2522key%2522%253A%257B%2522id%2522%253A%2522code.function--string--tag%2522%252C%2522dataType%2522%253A%2522string%2522%252C%2522key%2522%253A%2522code.function%2522%252C%2522type%2522%253A%2522tag%2522%257D%252C%2522op%2522%253A%2522%253D%2522%252C%2522value%2522%253A%2522render_test%2522%257D%255D%252C%2522op%2522%253A%2522AND%2522%257D%252C%2522expression%2522%253A%2522A%2522%252C%2522disabled%2522%253Afalse%252C%2522stepInterval%2522%253A0%252C%2522having%2522%253A%257B%2522expression%2522%253A%2522%2522%257D%252C%2522limit%2522%253Anull%252C%2522orderBy%2522%253A%255B%255D%252C%2522groupBy%2522%253A%255B%255D%252C%2522legend%2522%253A%2522%2522%252C%2522reduceTo%2522%253A%2522avg%2522%252C%2522source%2522%253A%2522%2522%252C%2522name%2522%253A%2522A%2522%252C%2522signal%2522%253A%2522logs%2522%252C%2522order%2522%253Anull%252C%2522selectFields%2522%253Anull%252C%2522secondaryAggregations%2522%253Anull%257D%255D%252C%2522queryFormulas%2522%253A%255B%255D%252C%2522queryTraceOperator%2522%253A%255B%255D%257D%252C%2522promql%2522%253A%255B%257B%2522name%2522%253A%2522A%2522%252C%2522query%2522%253A%2522%2522%252C%2522legend%2522%253A%2522%2522%252C%2522disabled%2522%253Afalse%257D%255D%252C%2522clickhouse_sql%2522%253A%255B%257B%2522name%2522%253A%2522A%2522%252C%2522legend%2522%253A%2522%2522%252C%2522disabled%2522%253Afalse%252C%2522query%2522%253A%2522%2522%257D%255D%252C%2522id%2522%253A%2522bfb926e4-7b98-4cf4-bd4d-adcbd18a1da2%2522%252C%2522unit%2522%253A%2522%2522%257D&options=%7B%22selectColumns%22%3A%5B%7B%22name%22%3A%22timestamp%22%2C%22signal%22%3A%22logs%22%2C%22fieldContext%22%3A%22log%22%2C%22fieldDataType%22%3A%22%22%7D%2C%7B%22name%22%3A%22lkadsjfl%22%2C%22signal%22%3A%22%22%2C%22fieldContext%22%3A%22%22%2C%22fieldDataType%22%3A%22%22%7D%2C%7B%22name%22%3A%22body%22%2C%22signal%22%3A%22logs%22%2C%22fieldContext%22%3A%22log%22%2C%22fieldDataType%22%3A%22%22%7D%2C%7B%22name%22%3A%22test%22%2C%22signal%22%3A%22%22%2C%22fieldContext%22%3A%22%22%2C%22fieldDataType%22%3A%22%22%7D%2C%7B%22name%22%3A%22severity_text%22%2C%22description%22%3A%22Log+level.+Learn+more+%5Bhere%5D%28https%3A%2F%2Fopentelemetry.io%2Fdocs%2Fspecs%2Fotel%2Flogs%2Fdata-model%2F%23field-severitytext%29%22%2C%22signal%22%3A%22logs%22%2C%22fieldContext%22%3A%22log%22%2C%22fieldDataType%22%3A%22string%22%7D%5D%2C%22format%22%3A%22list%22%2C%22maxLines%22%3A1%2C%22fontSize%22%3A%22small%22%7D&panelTypes=%22list%22&viewName=%22test+manul+key%22&viewKey=068e4a96-5225-4abe-8f9b-a5009f26d4ce) Breaks page image #### Additional Information Sentry: https://signoz-io.sentry.io/issues/7520808361 --- .../components/ExplorerCard/ExplorerCard.tsx | 7 +-- .../src/components/ExplorerCard/constants.ts | 4 -- .../ExplorerOptions/ExplorerOptions.tsx | 5 +- .../LogDetailedView/BodyTitleRenderer.tsx | 5 +- .../TableView/TableViewActions.tsx | 5 +- .../__test__/TableViewActions.test.tsx | 14 +++-- .../hooks/useLogAttributeActions.tsx | 5 +- .../queryBuilder/useGetSearchQueryParam.ts | 15 ----- .../__tests__/useGetSavedViewParams.test.ts | 60 +++++++++++++++++++ .../hooks/saveViews/useGetSavedViewParams.ts | 33 ++++++++++ .../src/hooks/useHandleExplorerTabChange.ts | 6 +- 11 files changed, 114 insertions(+), 45 deletions(-) delete mode 100644 frontend/src/hooks/queryBuilder/useGetSearchQueryParam.ts create mode 100644 frontend/src/hooks/saveViews/__tests__/useGetSavedViewParams.test.ts create mode 100644 frontend/src/hooks/saveViews/useGetSavedViewParams.ts diff --git a/frontend/src/components/ExplorerCard/ExplorerCard.tsx b/frontend/src/components/ExplorerCard/ExplorerCard.tsx index 32ab5b9297d..e04af8fedaf 100644 --- a/frontend/src/components/ExplorerCard/ExplorerCard.tsx +++ b/frontend/src/components/ExplorerCard/ExplorerCard.tsx @@ -7,9 +7,8 @@ import axios from 'axios'; import TextToolTip from 'components/TextToolTip'; import { SOMETHING_WENT_WRONG } from 'constants/api'; import { LOCALSTORAGE } from 'constants/localStorage'; -import { QueryParams } from 'constants/query'; import { useOptionsMenu } from 'container/OptionsMenu'; -import { useGetSearchQueryParam } from 'hooks/queryBuilder/useGetSearchQueryParam'; +import { useGetSavedViewParams } from 'hooks/saveViews/useGetSavedViewParams'; import { useQueryBuilder } from 'hooks/queryBuilder/useQueryBuilder'; import { useDeleteView } from 'hooks/saveViews/useDeleteView'; import { useGetAllViews } from 'hooks/saveViews/useGetAllViews'; @@ -69,9 +68,7 @@ function ExplorerCard({ setIsOpen(newOpen); }; - const viewName = useGetSearchQueryParam(QueryParams.viewName) || ''; - - const viewKey = useGetSearchQueryParam(QueryParams.viewKey) || ''; + const { viewName, viewKey } = useGetSavedViewParams(); const { options } = useOptionsMenu({ storageKey: diff --git a/frontend/src/components/ExplorerCard/constants.ts b/frontend/src/components/ExplorerCard/constants.ts index 3a08ad5edb7..8cc5887b41b 100644 --- a/frontend/src/components/ExplorerCard/constants.ts +++ b/frontend/src/components/ExplorerCard/constants.ts @@ -1,5 +1,3 @@ -import { QueryParams } from 'constants/query'; - export const ExploreHeaderToolTip = { url: 'https://signoz.io/docs/querying/overview/?utm_source=product&utm_medium=new-query-builder', text: 'More details on how to use query builder', @@ -9,5 +7,3 @@ export const SaveButtonText = { SAVE_AS_NEW_VIEW: 'Save as new view', SAVE_VIEW: 'Save view', }; - -export type QuerySearchParamNames = QueryParams.viewName | QueryParams.viewKey; diff --git a/frontend/src/container/ExplorerOptions/ExplorerOptions.tsx b/frontend/src/container/ExplorerOptions/ExplorerOptions.tsx index 4d548a8209d..712ff7c8e3b 100644 --- a/frontend/src/container/ExplorerOptions/ExplorerOptions.tsx +++ b/frontend/src/container/ExplorerOptions/ExplorerOptions.tsx @@ -54,7 +54,7 @@ import { } from 'container/OptionsMenu/constants'; import { OptionsQuery } from 'container/OptionsMenu/types'; import { ExportDashboard } from 'hooks/dashboard/useExportDashboards'; -import { useGetSearchQueryParam } from 'hooks/queryBuilder/useGetSearchQueryParam'; +import { useGetSavedViewParams } from 'hooks/saveViews/useGetSavedViewParams'; import { useQueryBuilder } from 'hooks/queryBuilder/useQueryBuilder'; import { useGetAllViews } from 'hooks/saveViews/useGetAllViews'; import { useSaveView } from 'hooks/saveViews/useSaveView'; @@ -287,8 +287,7 @@ function ExplorerOptions({ const compositeQuery = mapCompositeQueryFromQuery(currentQuery, panelType); - const viewName = useGetSearchQueryParam(QueryParams.viewName) || ''; - const viewKey = useGetSearchQueryParam(QueryParams.viewKey) || ''; + const { viewName, viewKey } = useGetSavedViewParams(); const extraData = viewsData?.data?.data?.find( (view) => view.id === viewKey, diff --git a/frontend/src/container/LogDetailedView/BodyTitleRenderer.tsx b/frontend/src/container/LogDetailedView/BodyTitleRenderer.tsx index 39be93760d8..b34279abe37 100644 --- a/frontend/src/container/LogDetailedView/BodyTitleRenderer.tsx +++ b/frontend/src/container/LogDetailedView/BodyTitleRenderer.tsx @@ -15,9 +15,8 @@ import { QUERY_BUILDER_FUNCTIONS, } from 'constants/antlrQueryConstants'; import { FeatureKeys } from 'constants/features'; -import { QueryParams } from 'constants/query'; import { useActiveLog } from 'hooks/logs/useActiveLog'; -import { useGetSearchQueryParam } from 'hooks/queryBuilder/useGetSearchQueryParam'; +import { useGetSavedViewParams } from 'hooks/saveViews/useGetSavedViewParams'; import { useQueryBuilder } from 'hooks/queryBuilder/useQueryBuilder'; import { ICurrentQueryData } from 'hooks/useHandleExplorerTabChange'; import { useNotifications } from 'hooks/useNotifications'; @@ -50,7 +49,7 @@ function BodyTitleRenderer({ const { featureFlags } = useAppContext(); const [, setCopy] = useCopyToClipboard(); const { notifications } = useNotifications(); - const viewName = useGetSearchQueryParam(QueryParams.viewName) || ''; + const { viewName } = useGetSavedViewParams(); const cleanedNodeKey = removeObjectFromString(nodeKey); const isBodyJsonQueryEnabled = diff --git a/frontend/src/container/LogDetailedView/TableView/TableViewActions.tsx b/frontend/src/container/LogDetailedView/TableView/TableViewActions.tsx index 138f7134fb0..42234a7440a 100644 --- a/frontend/src/container/LogDetailedView/TableView/TableViewActions.tsx +++ b/frontend/src/container/LogDetailedView/TableView/TableViewActions.tsx @@ -7,13 +7,12 @@ import GroupByIcon from 'assets/CustomIcons/GroupByIcon'; import cx from 'classnames'; import CopyClipboardHOC from 'components/Logs/CopyClipboardHOC'; import { DATE_TIME_FORMATS } from 'constants/dateTimeFormats'; -import { QueryParams } from 'constants/query'; import { OPERATORS } from 'constants/queryBuilder'; import ROUTES from 'constants/routes'; import { ChangeViewFunctionType } from 'container/ExplorerOptions/types'; import { RESTRICTED_SELECTED_FIELDS } from 'container/LogsFilters/config'; import { MetricsType } from 'container/MetricsApplication/constant'; -import { useGetSearchQueryParam } from 'hooks/queryBuilder/useGetSearchQueryParam'; +import { useGetSavedViewParams } from 'hooks/saveViews/useGetSavedViewParams'; import { useQueryBuilder } from 'hooks/queryBuilder/useQueryBuilder'; import { ICurrentQueryData } from 'hooks/useHandleExplorerTabChange'; import { @@ -141,7 +140,7 @@ export default function TableViewActions( const { pathname } = useLocation(); const { stagedQuery, updateQueriesData } = useQueryBuilder(); - const viewName = useGetSearchQueryParam(QueryParams.viewName) || ''; + const { viewName } = useGetSavedViewParams(); const { dataType, logType: fieldType } = getFieldAttributes(record.field); // there is no option for where clause in old logs explorer and live logs page or infra monitoring diff --git a/frontend/src/container/LogDetailedView/TableView/__test__/TableViewActions.test.tsx b/frontend/src/container/LogDetailedView/TableView/__test__/TableViewActions.test.tsx index 1602b47e4cc..536613b94bd 100644 --- a/frontend/src/container/LogDetailedView/TableView/__test__/TableViewActions.test.tsx +++ b/frontend/src/container/LogDetailedView/TableView/__test__/TableViewActions.test.tsx @@ -1,6 +1,6 @@ import { fireEvent, render, screen } from '@testing-library/react'; import { RESTRICTED_SELECTED_FIELDS } from 'container/LogsFilters/config'; -import { useGetSearchQueryParam } from 'hooks/queryBuilder/useGetSearchQueryParam'; +import { useGetSavedViewParams } from 'hooks/saveViews/useGetSavedViewParams'; import { useQueryBuilder } from 'hooks/queryBuilder/useQueryBuilder'; import { ExplorerViews } from 'pages/LogsExplorer/utils'; @@ -88,7 +88,7 @@ jest.mock('react-router-dom', () => ({ })); jest.mock('hooks/queryBuilder/useQueryBuilder'); -jest.mock('hooks/queryBuilder/useGetSearchQueryParam'); +jest.mock('hooks/saveViews/useGetSavedViewParams'); describe('TableViewActions', () => { const TEST_VALUE = 'test value'; @@ -140,8 +140,10 @@ describe('TableViewActions', () => { }), } as any); - // Default mock for useGetSearchQueryParam - jest.mocked(useGetSearchQueryParam).mockReturnValue(null); + // Default mock for useGetSavedViewParams + jest + .mocked(useGetSavedViewParams) + .mockReturnValue({ viewName: '', viewKey: '' }); }); it('should render without crashing', () => { @@ -249,7 +251,9 @@ describe('TableViewActions', () => { updateQueriesData: mockUpdateQueriesData, } as any); - jest.mocked(useGetSearchQueryParam).mockReturnValue(null); + jest + .mocked(useGetSavedViewParams) + .mockReturnValue({ viewName: '', viewKey: '' }); render( flag.name === FeatureKeys.USE_JSON_BODY) diff --git a/frontend/src/hooks/queryBuilder/useGetSearchQueryParam.ts b/frontend/src/hooks/queryBuilder/useGetSearchQueryParam.ts deleted file mode 100644 index 254f24c4d79..00000000000 --- a/frontend/src/hooks/queryBuilder/useGetSearchQueryParam.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { useMemo } from 'react'; -import { QuerySearchParamNames } from 'components/ExplorerCard/constants'; -import useUrlQuery from 'hooks/useUrlQuery'; - -export const useGetSearchQueryParam = ( - searchParams: QuerySearchParamNames, -): string | null => { - const urlQuery = useUrlQuery(); - - return useMemo(() => { - const searchQuery = urlQuery.get(searchParams); - - return searchQuery ? JSON.parse(searchQuery) : null; - }, [urlQuery, searchParams]); -}; diff --git a/frontend/src/hooks/saveViews/__tests__/useGetSavedViewParams.test.ts b/frontend/src/hooks/saveViews/__tests__/useGetSavedViewParams.test.ts new file mode 100644 index 00000000000..1cb251bf3e5 --- /dev/null +++ b/frontend/src/hooks/saveViews/__tests__/useGetSavedViewParams.test.ts @@ -0,0 +1,60 @@ +import { renderHook } from '@testing-library/react'; +import useUrlQuery from 'hooks/useUrlQuery'; + +import { useGetSavedViewParams } from '../useGetSavedViewParams'; + +jest.mock('hooks/useUrlQuery'); + +const mockedUseUrlQuery = useUrlQuery as jest.Mock; + +const setSearch = (search: string): void => { + mockedUseUrlQuery.mockReturnValue(new URLSearchParams(search)); +}; + +describe('useGetSavedViewParams', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('returns empty strings when no params are present', () => { + setSearch(''); + + const { result } = renderHook(() => useGetSavedViewParams()); + + expect(result.current).toStrictEqual({ viewName: '', viewKey: '' }); + }); + + it('parses JSON-stringified values', () => { + setSearch( + `viewName=${encodeURIComponent( + JSON.stringify('Hindsight'), + )}&viewKey=${encodeURIComponent(JSON.stringify('abc-123'))}`, + ); + + const { result } = renderHook(() => useGetSavedViewParams()); + + expect(result.current).toStrictEqual({ + viewName: 'Hindsight', + viewKey: 'abc-123', + }); + }); + + it('falls back to the raw string when a value is not valid JSON', () => { + setSearch('viewName=Hindsight&viewKey=some-uuid-value'); + + const { result } = renderHook(() => useGetSavedViewParams()); + + expect(result.current).toStrictEqual({ + viewName: 'Hindsight', + viewKey: 'some-uuid-value', + }); + }); + + it('does not throw and keeps the raw string for non-string JSON', () => { + setSearch('viewName=123'); + + const { result } = renderHook(() => useGetSavedViewParams()); + + expect(result.current).toStrictEqual({ viewName: '123', viewKey: '' }); + }); +}); diff --git a/frontend/src/hooks/saveViews/useGetSavedViewParams.ts b/frontend/src/hooks/saveViews/useGetSavedViewParams.ts new file mode 100644 index 00000000000..dcba66991c0 --- /dev/null +++ b/frontend/src/hooks/saveViews/useGetSavedViewParams.ts @@ -0,0 +1,33 @@ +import { useMemo } from 'react'; +import { QueryParams } from 'constants/query'; +import useUrlQuery from 'hooks/useUrlQuery'; + +interface SavedViewParams { + viewName: string; + viewKey: string; +} + +const parseViewParam = (value: string | null): string => { + if (!value) { + return ''; + } + + try { + const parsed = JSON.parse(value); + return typeof parsed === 'string' ? parsed : value; + } catch { + return value; + } +}; + +export const useGetSavedViewParams = (): SavedViewParams => { + const urlQuery = useUrlQuery(); + + return useMemo( + () => ({ + viewName: parseViewParam(urlQuery.get(QueryParams.viewName)), + viewKey: parseViewParam(urlQuery.get(QueryParams.viewKey)), + }), + [urlQuery], + ); +}; diff --git a/frontend/src/hooks/useHandleExplorerTabChange.ts b/frontend/src/hooks/useHandleExplorerTabChange.ts index 0ae0ded66f2..88940358d61 100644 --- a/frontend/src/hooks/useHandleExplorerTabChange.ts +++ b/frontend/src/hooks/useHandleExplorerTabChange.ts @@ -6,7 +6,7 @@ import { SIGNOZ_VALUE } from 'container/QueryBuilder/filters/OrderByFilter/const import { Query } from 'types/api/queryBuilder/queryBuilderData'; import { DataSource } from 'types/common/queryBuilder'; -import { useGetSearchQueryParam } from './queryBuilder/useGetSearchQueryParam'; +import { useGetSavedViewParams } from './saveViews/useGetSavedViewParams'; import { useQueryBuilder } from './queryBuilder/useQueryBuilder'; export interface ICurrentQueryData { @@ -31,9 +31,7 @@ export const useHandleExplorerTabChange = (): { updateQueriesData, } = useQueryBuilder(); - const viewName = useGetSearchQueryParam(QueryParams.viewName) || ''; - - const viewKey = useGetSearchQueryParam(QueryParams.viewKey) || ''; + const { viewName, viewKey } = useGetSavedViewParams(); const getUpdateQuery = useCallback( (newPanelType: PANEL_TYPES): Query => {