From fb711b4d4a3772228ff0575de0c402241426bc7d Mon Sep 17 00:00:00 2001 From: nya1 <16636743+nya1@users.noreply.github.com> Date: Tue, 14 Jun 2022 13:08:38 +0200 Subject: [PATCH] fix: when payload type is 'json' validate value on toggle variable validation (#1704) * fix: when payload type is 'json' validate value on toggle variable validation * test: add missing feature toggle creation with variant type json Ref https://github.com/Unleash/unleash/pull/1704#discussion_r896476042 * refractor: remove verbose comment on validateJsonString Ref https://github.com/Unleash/unleash/pull/1704#discussion_r896482210 * test: add missing feature toggle creation with variant type string Ref https://github.com/Unleash/unleash/pull/1704#discussion_r896476042 * refractor: move variant value joi validation Ref https://github.com/Unleash/unleash/pull/1704#discussion_r896478563 --- src/lib/schema/feature-schema.ts | 23 ++++++- src/lib/util/validateJsonString.test.ts | 22 +++++++ src/lib/util/validateJsonString.ts | 12 ++++ src/test/e2e/api/admin/feature.e2e.test.ts | 70 ++++++++++++++++++++++ 4 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 src/lib/util/validateJsonString.test.ts create mode 100644 src/lib/util/validateJsonString.ts diff --git a/src/lib/schema/feature-schema.ts b/src/lib/schema/feature-schema.ts index 59523da034..cc9cee637b 100644 --- a/src/lib/schema/feature-schema.ts +++ b/src/lib/schema/feature-schema.ts @@ -1,6 +1,7 @@ import joi from 'joi'; import { ALL_OPERATORS } from '../util/constants'; import { nameType } from '../routes/util'; +import { validateJsonString } from '../util/validateJsonString'; export const nameSchema = joi .object() @@ -27,6 +28,26 @@ export const strategiesSchema = joi.object().keys({ parameters: joi.object(), }); +const variantValueSchema = joi + .string() + .required() + // perform additional validation + // when provided 'type' is 'json' + .when('type', { + is: 'json', + then: joi.custom((val, helper) => { + const isValidJsonString = validateJsonString(val); + if (isValidJsonString === false) { + return helper.error('invalidJsonString'); + } + return val; + }), + }) + .messages({ + invalidJsonString: + "'value' must be a valid json string when 'type' is json", + }); + export const variantsSchema = joi.object().keys({ name: nameType, weight: joi.number().min(0).max(1000).required(), @@ -35,7 +56,7 @@ export const variantsSchema = joi.object().keys({ .object() .keys({ type: joi.string().required(), - value: joi.string().required(), + value: variantValueSchema, }) .optional(), stickiness: joi.string().default('default'), diff --git a/src/lib/util/validateJsonString.test.ts b/src/lib/util/validateJsonString.test.ts new file mode 100644 index 0000000000..e5271366e9 --- /dev/null +++ b/src/lib/util/validateJsonString.test.ts @@ -0,0 +1,22 @@ +import { validateJsonString } from './validateJsonString'; + +test('should return true for valid json string', () => { + const input = '{"test":1,"nested":[{"test1":{"testinner":true}}]}'; + expect(validateJsonString(input)).toBe(true); +}); + +test('should return false for invalid json string (missing starting {)', () => { + // missing starting { + const input = '"test":1,"nested":[{"test1":{"testinner":true}}]}'; + expect(validateJsonString(input)).toBe(false); +}); + +test('should return false for invalid json string (plain string)', () => { + const input = 'not a json'; + expect(validateJsonString(input)).toBe(false); +}); + +test('should return false for invalid json string (null as a string)', () => { + const input = 'null'; + expect(validateJsonString(input)).toBe(false); +}); diff --git a/src/lib/util/validateJsonString.ts b/src/lib/util/validateJsonString.ts new file mode 100644 index 0000000000..b679cdc689 --- /dev/null +++ b/src/lib/util/validateJsonString.ts @@ -0,0 +1,12 @@ +export const validateJsonString = (value: string): boolean => { + // from https://stackoverflow.com/a/20392392 + try { + const parsedStr = JSON.parse(value); + if (parsedStr && typeof parsedStr === 'object') { + return true; + } + } catch (err) {} + + // an error is considered a non valid json + return false; +}; diff --git a/src/test/e2e/api/admin/feature.e2e.test.ts b/src/test/e2e/api/admin/feature.e2e.test.ts index 8d27524605..7b5bf207d2 100644 --- a/src/test/e2e/api/admin/feature.e2e.test.ts +++ b/src/test/e2e/api/admin/feature.e2e.test.ts @@ -261,6 +261,76 @@ test('creates new feature toggle with createdBy unknown', async () => { }); }); +test('create new feature toggle with variant type json', async () => { + return app.request + .post('/api/admin/features') + .send({ + name: 'com.test.featureWithJson', + variants: [ + { + name: 'variantTestJson', + weight: 1, + payload: { + type: 'json', + value: '{"test": true, "user": [{"jsonValid": 1}]}', + }, + weightType: 'variable', + }, + ], + }) + .set('Content-Type', 'application/json') + .expect(201); +}); + +test('create new feature toggle with variant type string', async () => { + return app.request + .post('/api/admin/features') + .send({ + name: 'com.test.featureWithString', + variants: [ + { + name: 'variantTestString', + weight: 1, + payload: { + type: 'string', + value: 'my string # here', + }, + weightType: 'variable', + }, + ], + }) + .set('Content-Type', 'application/json') + .expect(201); +}); + +test('refuses to create a new feature toggle with variant when type is json but value provided is not a valid json', async () => { + return app.request + .post('/api/admin/features') + .send({ + name: 'com.test.featureInvalidValue', + variants: [ + { + name: 'variantTest', + weight: 1, + payload: { + type: 'json', + value: 'this should be a # valid json', // <--- testing value + }, + weightType: 'variable', + }, + ], + }) + .set('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body.isJoi).toBe(true); + expect(res.body.details[0].type).toBe('invalidJsonString'); + expect(res.body.details[0].message).toBe( + `'value' must be a valid json string when 'type' is json`, + ); + }); +}); + test('require new feature toggle to have a name', async () => { expect.assertions(0); return app.request