From 5939a9f43ba5e62a7121509b4b9fc1a47111201c Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 8 Apr 2021 00:38:30 +0200 Subject: [PATCH 1/5] Fall back to raw string when parsing object arg param, and add support for fractional numbers --- lib/client-api/src/args.ts | 3 +++ lib/core-client/src/preview/parseArgsParam.test.ts | 8 ++++++++ lib/core-client/src/preview/parseArgsParam.ts | 11 +++++++++-- lib/router/src/utils.ts | 11 +++++++++-- 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/lib/client-api/src/args.ts b/lib/client-api/src/args.ts index f2816057fead..f65506cc4219 100644 --- a/lib/client-api/src/args.ts +++ b/lib/client-api/src/args.ts @@ -7,6 +7,8 @@ type ValueType = { name: string; value?: ObjectValueType | ValueType }; type ObjectValueType = Record; const INCOMPATIBLE = Symbol('incompatible'); +const NUMBER_REGEXP = /^-?[0-9]+(\.[0-9]+)?$/; + const map = (arg: unknown, type: ValueType): any => { if (arg === undefined || arg === null || !type) return arg; switch (type.name) { @@ -26,6 +28,7 @@ const map = (arg: unknown, type: ValueType): any => { return acc; }, new Array(arg.length)); case 'object': + if (typeof arg === 'string') return NUMBER_REGEXP.test(arg) ? Number(arg) : arg; if (!type.value || typeof arg !== 'object') return INCOMPATIBLE; return Object.entries(arg).reduce((acc, [key, val]) => { const mapped = map(val, (type.value as ObjectValueType)[key]); diff --git a/lib/core-client/src/preview/parseArgsParam.test.ts b/lib/core-client/src/preview/parseArgsParam.test.ts index 4208791b7138..9186c0217fbe 100644 --- a/lib/core-client/src/preview/parseArgsParam.test.ts +++ b/lib/core-client/src/preview/parseArgsParam.test.ts @@ -225,6 +225,14 @@ describe('parseArgsParam', () => { expect(parseArgsParam('key:1')).toStrictEqual({ key: '1' }); }); + it('allows valid fractional numbers', () => { + expect(parseArgsParam('key:1.2')).toStrictEqual({ key: '1.2' }); + expect(parseArgsParam('key:-1.2')).toStrictEqual({ key: '-1.2' }); + expect(parseArgsParam('key:1.')).toStrictEqual({}); + expect(parseArgsParam('key:.2')).toStrictEqual({}); + expect(parseArgsParam('key:1.2.3')).toStrictEqual({}); + }); + it('also applies to nested object and array values', () => { expect(parseArgsParam('obj.key:a!b')).toStrictEqual({}); expect(parseArgsParam('obj[key]:a!b')).toStrictEqual({}); diff --git a/lib/core-client/src/preview/parseArgsParam.ts b/lib/core-client/src/preview/parseArgsParam.ts index b0e79f9aa054..1bdebabeb9f2 100644 --- a/lib/core-client/src/preview/parseArgsParam.ts +++ b/lib/core-client/src/preview/parseArgsParam.ts @@ -6,6 +6,7 @@ import isPlainObject from 'lodash/isPlainObject'; // Keep this in sync with validateArgs in router/src/utils.ts const VALIDATION_REGEXP = /^[a-zA-Z0-9 _-]*$/; +const NUMBER_REGEXP = /^-?[0-9]+(\.[0-9]+)?$/; const HEX_REGEXP = /^#([a-f0-9]{3,4}|[a-f0-9]{6}|[a-f0-9]{8})$/i; const COLOR_REGEXP = /^(rgba?|hsla?)\(([0-9]{1,3}),\s?([0-9]{1,3})%?,\s?([0-9]{1,3})%?,?\s?([0-9](\.[0-9]{1,2})?)?\)$/i; const validateArgs = (key = '', value: unknown): boolean => { @@ -14,8 +15,14 @@ const validateArgs = (key = '', value: unknown): boolean => { if (value === null || value === undefined) return true; // encoded as `!null` or `!undefined` if (value instanceof Date) return true; // encoded as modified ISO string if (typeof value === 'number' || typeof value === 'boolean') return true; - if (typeof value === 'string') - return VALIDATION_REGEXP.test(value) || HEX_REGEXP.test(value) || COLOR_REGEXP.test(value); + if (typeof value === 'string') { + return ( + VALIDATION_REGEXP.test(value) || + NUMBER_REGEXP.test(value) || + HEX_REGEXP.test(value) || + COLOR_REGEXP.test(value) + ); + } if (Array.isArray(value)) return value.every((v) => validateArgs(key, v)); if (isPlainObject(value)) return Object.entries(value).every(([k, v]) => validateArgs(k, v)); return false; diff --git a/lib/router/src/utils.ts b/lib/router/src/utils.ts index 20743e00e3a4..0831485f8e04 100644 --- a/lib/router/src/utils.ts +++ b/lib/router/src/utils.ts @@ -63,6 +63,7 @@ export const deepDiff = (value: any, update: any): any => { // Keep this in sync with validateArgs in core-client/src/preview/parseArgsParam.ts const VALIDATION_REGEXP = /^[a-zA-Z0-9 _-]*$/; +const NUMBER_REGEXP = /^-?[0-9]+(\.[0-9]+)?$/; const HEX_REGEXP = /^#([a-f0-9]{3,4}|[a-f0-9]{6}|[a-f0-9]{8})$/i; const COLOR_REGEXP = /^(rgba?|hsla?)\(([0-9]{1,3}),\s?([0-9]{1,3})%?,\s?([0-9]{1,3})%?,?\s?([0-9](\.[0-9]{1,2})?)?\)$/i; const validateArgs = (key = '', value: unknown): boolean => { @@ -71,8 +72,14 @@ const validateArgs = (key = '', value: unknown): boolean => { if (value === null || value === undefined) return true; // encoded as `!null` or `!undefined` if (value instanceof Date) return true; // encoded as modified ISO string if (typeof value === 'number' || typeof value === 'boolean') return true; - if (typeof value === 'string') - return VALIDATION_REGEXP.test(value) || HEX_REGEXP.test(value) || COLOR_REGEXP.test(value); + if (typeof value === 'string') { + return ( + VALIDATION_REGEXP.test(value) || + NUMBER_REGEXP.test(value) || + HEX_REGEXP.test(value) || + COLOR_REGEXP.test(value) + ); + } if (Array.isArray(value)) return value.every((v) => validateArgs(key, v)); if (isPlainObject(value)) return Object.entries(value).every(([k, v]) => validateArgs(k, v)); return false; From 7fef44f716d41497dfc8973ee005d0d1add3ad8a Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 8 Apr 2021 00:49:38 +0200 Subject: [PATCH 2/5] Add tests for raw object values --- lib/client-api/src/args.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/client-api/src/args.test.ts b/lib/client-api/src/args.test.ts index 559a8783d5a0..7663a5103b59 100644 --- a/lib/client-api/src/args.test.ts +++ b/lib/client-api/src/args.test.ts @@ -53,6 +53,7 @@ describe('mapArgsToTypes', () => { it('maps numbers', () => { expect(mapArgsToTypes({ a: '42' }, { a: { type: numberType } })).toStrictEqual({ a: 42 }); + expect(mapArgsToTypes({ a: '4.2' }, { a: { type: numberType } })).toStrictEqual({ a: 4.2 }); expect(mapArgsToTypes({ a: 'a' }, { a: { type: numberType } })).toStrictEqual({ a: NaN }); }); @@ -86,6 +87,18 @@ describe('mapArgsToTypes', () => { }); }); + it('passes raw string for object type', () => { + expect(mapArgsToTypes({ a: 'string' }, { a: { type: boolObjectType } })).toStrictEqual({ + a: 'string', + }); + }); + + it('parses number for object type', () => { + expect(mapArgsToTypes({ a: '1.2' }, { a: { type: boolObjectType } })).toStrictEqual({ + a: 1.2, + }); + }); + it('deeply maps objects', () => { expect( mapArgsToTypes( From 0eee876c1dc69b4e2caf2b91abf16e42a5a3c467 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 8 Apr 2021 01:00:37 +0200 Subject: [PATCH 3/5] Actually parse the number --- .../src/preview/parseArgsParam.test.ts | 56 +++++++++---------- lib/core-client/src/preview/parseArgsParam.ts | 1 + 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/lib/core-client/src/preview/parseArgsParam.test.ts b/lib/core-client/src/preview/parseArgsParam.test.ts index 9186c0217fbe..2fd4bfe24aa8 100644 --- a/lib/core-client/src/preview/parseArgsParam.test.ts +++ b/lib/core-client/src/preview/parseArgsParam.test.ts @@ -71,60 +71,60 @@ describe('parseArgsParam', () => { }); it('parses multiple values', () => { - const args = parseArgsParam('one:1;two:2;three:3'); - expect(args).toStrictEqual({ one: '1', two: '2', three: '3' }); + const args = parseArgsParam('one:A;two:B;three:C'); + expect(args).toStrictEqual({ one: 'A', two: 'B', three: 'C' }); }); it('parses arrays', () => { - const args = parseArgsParam('arr[]:1;arr[]:2;arr[]:3'); - expect(args).toStrictEqual({ arr: ['1', '2', '3'] }); + const args = parseArgsParam('arr[]:A;arr[]:B;arr[]:C'); + expect(args).toStrictEqual({ arr: ['A', 'B', 'C'] }); }); it('parses arrays with indices', () => { - const args = parseArgsParam('arr[0]:1;arr[1]:2;arr[2]:3'); - expect(args).toStrictEqual({ arr: ['1', '2', '3'] }); + const args = parseArgsParam('arr[0]:A;arr[1]:B;arr[2]:C'); + expect(args).toStrictEqual({ arr: ['A', 'B', 'C'] }); }); it('parses sparse arrays', () => { - const args = parseArgsParam('arr[0]:1;arr[2]:3'); + const args = parseArgsParam('arr[0]:A;arr[2]:C'); // eslint-disable-next-line no-sparse-arrays - expect(args).toStrictEqual({ arr: ['1', , '3'] }); + expect(args).toStrictEqual({ arr: ['A', , 'C'] }); }); it('parses repeated values as arrays', () => { - const args = parseArgsParam('arr:1;arr:2;arr:3'); - expect(args).toStrictEqual({ arr: ['1', '2', '3'] }); + const args = parseArgsParam('arr:A;arr:B;arr:C'); + expect(args).toStrictEqual({ arr: ['A', 'B', 'C'] }); }); it('parses simple objects', () => { - const args = parseArgsParam('obj.one:1;obj.two:2'); - expect(args).toStrictEqual({ obj: { one: '1', two: '2' } }); + const args = parseArgsParam('obj.one:A;obj.two:B'); + expect(args).toStrictEqual({ obj: { one: 'A', two: 'B' } }); }); it('parses nested objects', () => { - const args = parseArgsParam('obj.foo.one:1;obj.foo.two:2;obj.bar.one:1'); - expect(args).toStrictEqual({ obj: { foo: { one: '1', two: '2' }, bar: { one: '1' } } }); + const args = parseArgsParam('obj.foo.one:A;obj.foo.two:B;obj.bar.one:A'); + expect(args).toStrictEqual({ obj: { foo: { one: 'A', two: 'B' }, bar: { one: 'A' } } }); }); it('parses arrays in objects', () => { - expect(parseArgsParam('obj.foo[]:1;obj.foo[]:2')).toStrictEqual({ obj: { foo: ['1', '2'] } }); - expect(parseArgsParam('obj.foo[0]:1;obj.foo[1]:2')).toStrictEqual({ obj: { foo: ['1', '2'] } }); + expect(parseArgsParam('obj.foo[]:A;obj.foo[]:B')).toStrictEqual({ obj: { foo: ['A', 'B'] } }); + expect(parseArgsParam('obj.foo[0]:A;obj.foo[1]:B')).toStrictEqual({ obj: { foo: ['A', 'B'] } }); // eslint-disable-next-line no-sparse-arrays - expect(parseArgsParam('obj.foo[1]:2')).toStrictEqual({ obj: { foo: [, '2'] } }); - expect(parseArgsParam('obj.foo:1;obj.foo:2')).toStrictEqual({ obj: { foo: ['1', '2'] } }); + expect(parseArgsParam('obj.foo[1]:B')).toStrictEqual({ obj: { foo: [, 'B'] } }); + expect(parseArgsParam('obj.foo:A;obj.foo:B')).toStrictEqual({ obj: { foo: ['A', 'B'] } }); }); it('parses single object in array', () => { - const args = parseArgsParam('arr[].one:1;arr[].two:2'); - expect(args).toStrictEqual({ arr: [{ one: '1', two: '2' }] }); + const args = parseArgsParam('arr[].one:A;arr[].two:B'); + expect(args).toStrictEqual({ arr: [{ one: 'A', two: 'B' }] }); }); it('parses multiple objects in array', () => { - expect(parseArgsParam('arr[0].key:1;arr[1].key:2')).toStrictEqual({ - arr: [{ key: '1' }, { key: '2' }], + expect(parseArgsParam('arr[0].key:A;arr[1].key:B')).toStrictEqual({ + arr: [{ key: 'A' }, { key: 'B' }], }); - expect(parseArgsParam('arr[0][key]:1;arr[1][key]:2')).toStrictEqual({ - arr: [{ key: '1' }, { key: '2' }], + expect(parseArgsParam('arr[0][key]:A;arr[1][key]:B')).toStrictEqual({ + arr: [{ key: 'A' }, { key: 'B' }], }); }); @@ -222,12 +222,12 @@ describe('parseArgsParam', () => { expect(parseArgsParam('key:_val_')).toStrictEqual({ key: '_val_' }); expect(parseArgsParam('key:-val-')).toStrictEqual({ key: '-val-' }); expect(parseArgsParam('key:VAL123')).toStrictEqual({ key: 'VAL123' }); - expect(parseArgsParam('key:1')).toStrictEqual({ key: '1' }); }); - it('allows valid fractional numbers', () => { - expect(parseArgsParam('key:1.2')).toStrictEqual({ key: '1.2' }); - expect(parseArgsParam('key:-1.2')).toStrictEqual({ key: '-1.2' }); + it('allows and parses valid (fractional) numbers', () => { + expect(parseArgsParam('key:1')).toStrictEqual({ key: 1 }); + expect(parseArgsParam('key:1.2')).toStrictEqual({ key: 1.2 }); + expect(parseArgsParam('key:-1.2')).toStrictEqual({ key: -1.2 }); expect(parseArgsParam('key:1.')).toStrictEqual({}); expect(parseArgsParam('key:.2')).toStrictEqual({}); expect(parseArgsParam('key:1.2.3')).toStrictEqual({}); diff --git a/lib/core-client/src/preview/parseArgsParam.ts b/lib/core-client/src/preview/parseArgsParam.ts index 1bdebabeb9f2..78b46618015f 100644 --- a/lib/core-client/src/preview/parseArgsParam.ts +++ b/lib/core-client/src/preview/parseArgsParam.ts @@ -55,6 +55,7 @@ const QS_OPTIONS = { : `${color[1]}(${color[2]}, ${color[3]}%, ${color[4]}%)`; } } + if (type === 'value' && NUMBER_REGEXP.test(str)) return Number(str); return defaultDecoder(str, defaultDecoder, charset); }, }; From dc83904751519d6634ef4a8599a7f472d176992c Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 8 Apr 2021 01:02:38 +0200 Subject: [PATCH 4/5] We already parsed the number --- lib/client-api/src/args.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/client-api/src/args.ts b/lib/client-api/src/args.ts index f65506cc4219..f09e3bf91910 100644 --- a/lib/client-api/src/args.ts +++ b/lib/client-api/src/args.ts @@ -7,8 +7,6 @@ type ValueType = { name: string; value?: ObjectValueType | ValueType }; type ObjectValueType = Record; const INCOMPATIBLE = Symbol('incompatible'); -const NUMBER_REGEXP = /^-?[0-9]+(\.[0-9]+)?$/; - const map = (arg: unknown, type: ValueType): any => { if (arg === undefined || arg === null || !type) return arg; switch (type.name) { @@ -28,7 +26,7 @@ const map = (arg: unknown, type: ValueType): any => { return acc; }, new Array(arg.length)); case 'object': - if (typeof arg === 'string') return NUMBER_REGEXP.test(arg) ? Number(arg) : arg; + if (typeof arg === 'string' || typeof arg === 'number') return arg; if (!type.value || typeof arg !== 'object') return INCOMPATIBLE; return Object.entries(arg).reduce((acc, [key, val]) => { const mapped = map(val, (type.value as ObjectValueType)[key]); From 67a1dd08b10ff32acb8e90c5df71573a1944c8e4 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Thu, 8 Apr 2021 01:05:46 +0200 Subject: [PATCH 5/5] Update test --- lib/client-api/src/args.test.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/client-api/src/args.test.ts b/lib/client-api/src/args.test.ts index 7663a5103b59..b26d877943b2 100644 --- a/lib/client-api/src/args.test.ts +++ b/lib/client-api/src/args.test.ts @@ -87,16 +87,12 @@ describe('mapArgsToTypes', () => { }); }); - it('passes raw string for object type', () => { - expect(mapArgsToTypes({ a: 'string' }, { a: { type: boolObjectType } })).toStrictEqual({ - a: 'string', - }); + it('passes string for object type', () => { + expect(mapArgsToTypes({ a: 'A' }, { a: { type: boolObjectType } })).toStrictEqual({ a: 'A' }); }); - it('parses number for object type', () => { - expect(mapArgsToTypes({ a: '1.2' }, { a: { type: boolObjectType } })).toStrictEqual({ - a: 1.2, - }); + it('passes number for object type', () => { + expect(mapArgsToTypes({ a: 1.2 }, { a: { type: boolObjectType } })).toStrictEqual({ a: 1.2 }); }); it('deeply maps objects', () => {