Skip to content

Commit

Permalink
fix(CRUD/listviews): Errors with rison and search strings using speci…
Browse files Browse the repository at this point in the history
…al 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 <corbin@Corbins-MacBook-Pro.local>
  • Loading branch information
corbinrobb and Corbin Robb authored Feb 15, 2022
1 parent 97a879e commit c8df849
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 9 deletions.
3 changes: 1 addition & 2 deletions superset-frontend/src/components/ListView/Filters/Search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<HTMLInputElement>) => {
Expand Down
11 changes: 9 additions & 2 deletions superset-frontend/src/components/ListView/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, any> = {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
const loadDashboardOptions = useMemo(
() =>
(input = '', page: number, pageSize: number) => {
const query = rison.encode({
const query = rison.encode_uri({
filter: input,
page,
page_size: pageSize,
Expand Down Expand Up @@ -749,7 +749,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
const loadChartOptions = useMemo(
() =>
(input = '', page: number, pageSize: number) => {
const query = rison.encode({
const query = rison.encode_uri({
filter: input,
page,
page_size: pageSize,
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/views/CRUD/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export function useListViewResource<D extends object = any>(
: value,
}));

const queryParams = rison.encode({
const queryParams = rison.encode_uri({
order_column: sortBy[0].id,
order_direction: sortBy[0].desc ? 'desc' : 'asc',
page: pageIndex,
Expand Down
15 changes: 15 additions & 0 deletions superset-frontend/src/views/CRUD/utils.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import rison from 'rison';
import {
isNeedsPassword,
isAlreadyExists,
Expand Down Expand Up @@ -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);
});
});
31 changes: 30 additions & 1 deletion superset-frontend/src/views/CRUD/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
(
Expand All @@ -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,
Expand Down

0 comments on commit c8df849

Please sign in to comment.