From c8df84985cf4f16f66287c493ba616b7d47a7c63 Mon Sep 17 00:00:00 2001 From: Corbin Robb <31329271+corbinrobb@users.noreply.github.com> Date: Tue, 15 Feb 2022 15:08:36 -0700 Subject: [PATCH] fix(CRUD/listviews): Errors with rison and search strings using special characters (#18056) * fix errors with rison and useQueryParams * add test for encode/decode * add rison link and make test case more readable Co-authored-by: Corbin Robb --- .../components/ListView/Filters/Search.tsx | 3 +- .../src/components/ListView/utils.ts | 11 +++++-- .../DateFilterControl/DateFilterLabel.tsx | 2 +- .../src/views/CRUD/alert/AlertReportModal.tsx | 4 +-- superset-frontend/src/views/CRUD/hooks.ts | 2 +- .../src/views/CRUD/utils.test.tsx | 15 +++++++++ superset-frontend/src/views/CRUD/utils.tsx | 31 ++++++++++++++++++- 7 files changed, 59 insertions(+), 9 deletions(-) diff --git a/superset-frontend/src/components/ListView/Filters/Search.tsx b/superset-frontend/src/components/ListView/Filters/Search.tsx index 482fac17d4b7a..fd61cba5cc79f 100644 --- a/superset-frontend/src/components/ListView/Filters/Search.tsx +++ b/superset-frontend/src/components/ListView/Filters/Search.tsx @@ -51,8 +51,7 @@ export default function SearchFilter({ const [value, setValue] = useState(initialValue || ''); const handleSubmit = () => { if (value) { - // encode plus signs to prevent them from being converted into a space - onSubmit(value.trim().replace(/\+/g, '%2B')); + onSubmit(value.trim()); } }; const handleChange = (e: React.ChangeEvent) => { diff --git a/superset-frontend/src/components/ListView/utils.ts b/superset-frontend/src/components/ListView/utils.ts index 3ce370d054867..346bde0982fc3 100644 --- a/superset-frontend/src/components/ListView/utils.ts +++ b/superset-frontend/src/components/ListView/utils.ts @@ -46,10 +46,17 @@ import { } from './types'; // Define custom RisonParam for proper encoding/decoding; note that -// plus symbols should be encoded to avoid being converted into a space +// %, &, +, and # must be encoded to avoid breaking the url const RisonParam: QueryParamConfig = { encode: (data?: any | null) => - data === undefined ? undefined : rison.encode(data).replace(/\+/g, '%2B'), + data === undefined + ? undefined + : rison + .encode(data) + .replace(/%/g, '%25') + .replace(/&/g, '%26') + .replace(/\+/g, '%2B') + .replace(/#/g, '%23'), decode: (dataStr?: string | string[]) => dataStr === undefined || Array.isArray(dataStr) ? undefined diff --git a/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx b/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx index e5324a926bdd3..80f287295989f 100644 --- a/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx +++ b/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx @@ -75,7 +75,7 @@ const fetchTimeRange = async ( timeRange: string, endpoints?: TimeRangeEndpoints, ) => { - const query = rison.encode(timeRange); + const query = rison.encode_uri(timeRange); const endpoint = `/api/v1/time_range/?q=${query}`; try { const response = await SupersetClient.get({ endpoint }); diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx index 416f47a4ab9f6..2934c3cd80ad0 100644 --- a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx +++ b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx @@ -675,7 +675,7 @@ const AlertReportModal: FunctionComponent = ({ const loadDashboardOptions = useMemo( () => (input = '', page: number, pageSize: number) => { - const query = rison.encode({ + const query = rison.encode_uri({ filter: input, page, page_size: pageSize, @@ -749,7 +749,7 @@ const AlertReportModal: FunctionComponent = ({ const loadChartOptions = useMemo( () => (input = '', page: number, pageSize: number) => { - const query = rison.encode({ + const query = rison.encode_uri({ filter: input, page, page_size: pageSize, diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts index 2c9c2f9930e05..ae4b041f66878 100644 --- a/superset-frontend/src/views/CRUD/hooks.ts +++ b/superset-frontend/src/views/CRUD/hooks.ts @@ -147,7 +147,7 @@ export function useListViewResource( : value, })); - const queryParams = rison.encode({ + const queryParams = rison.encode_uri({ order_column: sortBy[0].id, order_direction: sortBy[0].desc ? 'desc' : 'asc', page: pageIndex, diff --git a/superset-frontend/src/views/CRUD/utils.test.tsx b/superset-frontend/src/views/CRUD/utils.test.tsx index 2b95319d85d5b..ce7139e82a99a 100644 --- a/superset-frontend/src/views/CRUD/utils.test.tsx +++ b/superset-frontend/src/views/CRUD/utils.test.tsx @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +import rison from 'rison'; import { isNeedsPassword, isAlreadyExists, @@ -171,3 +172,17 @@ test('does not ask for password when the import type is wrong', () => { }; expect(hasTerminalValidation(error.errors)).toBe(true); }); + +test('successfully modified rison to encode correctly', () => { + const problemCharacters = '& # ? ^ { } [ ] | " = + `'; + + problemCharacters.split(' ').forEach(char => { + const testObject = { test: char }; + + const actualEncoding = rison.encode(testObject); + const expectedEncoding = `(test:'${char}')`; // Ex: (test:'&') + + expect(actualEncoding).toEqual(expectedEncoding); + expect(rison.decode(actualEncoding)).toEqual(testObject); + }); +}); diff --git a/superset-frontend/src/views/CRUD/utils.tsx b/superset-frontend/src/views/CRUD/utils.tsx index 64a92e08c63de..174e1aa493108 100644 --- a/superset-frontend/src/views/CRUD/utils.tsx +++ b/superset-frontend/src/views/CRUD/utils.tsx @@ -33,6 +33,35 @@ import { FetchDataConfig } from 'src/components/ListView'; import SupersetText from 'src/utils/textUtils'; import { Dashboard, Filters } from './types'; +// Modifies the rison encoding slightly to match the backend's rison encoding/decoding. Applies globally. +// Code pulled from rison.js (https://github.com/Nanonid/rison), rison is licensed under the MIT license. +(() => { + const risonRef: { + not_idchar: string; + not_idstart: string; + id_ok: RegExp; + next_id: RegExp; + } = rison as any; + + const l = []; + for (let hi = 0; hi < 16; hi += 1) { + for (let lo = 0; lo < 16; lo += 1) { + if (hi + lo === 0) continue; + const c = String.fromCharCode(hi * 16 + lo); + if (!/\w|[-_./~]/.test(c)) + l.push(`\\u00${hi.toString(16)}${lo.toString(16)}`); + } + } + + risonRef.not_idchar = l.join(''); + risonRef.not_idstart = '-0123456789'; + + const idrx = `[^${risonRef.not_idstart}${risonRef.not_idchar}][^${risonRef.not_idchar}]*`; + + risonRef.id_ok = new RegExp(`^${idrx}$`); + risonRef.next_id = new RegExp(idrx, 'g'); +})(); + const createFetchResourceMethod = (method: string) => ( @@ -43,7 +72,7 @@ const createFetchResourceMethod = ) => async (filterValue = '', page: number, pageSize: number) => { const resourceEndpoint = `/api/v1/${resource}/${method}/${relation}`; - const queryParams = rison.encode({ + const queryParams = rison.encode_uri({ filter: filterValue, page, page_size: pageSize,