From b88f93bf630beb541b52d440590a9c1381a03064 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 30 Apr 2021 10:45:23 -0300 Subject: [PATCH] Revert "removed sanitizer logic for a separete PR" This reverts commit 927d50535310884c741393dc7f602919ab17e718. --- src/evaluator/value/sanitize.ts | 2 +- .../__tests__/wrapperAdapter.spec.ts | 50 ++++++++++++ src/storages/pluggable/wrapperAdapter.ts | 80 ++++++++++++++----- 3 files changed, 113 insertions(+), 19 deletions(-) diff --git a/src/evaluator/value/sanitize.ts b/src/evaluator/value/sanitize.ts index 991ed8e1..b42ff769 100644 --- a/src/evaluator/value/sanitize.ts +++ b/src/evaluator/value/sanitize.ts @@ -28,7 +28,7 @@ function sanitizeArray(val: any): string[] | undefined { return arr.length ? arr : undefined; } -function sanitizeBoolean(val: any): boolean | undefined { +export function sanitizeBoolean(val: any): boolean | undefined { if (val === true || val === false) return val; if (typeof val === 'string') { diff --git a/src/storages/pluggable/__tests__/wrapperAdapter.spec.ts b/src/storages/pluggable/__tests__/wrapperAdapter.spec.ts index d8f62ecc..49ad448d 100644 --- a/src/storages/pluggable/__tests__/wrapperAdapter.spec.ts +++ b/src/storages/pluggable/__tests__/wrapperAdapter.spec.ts @@ -38,6 +38,38 @@ export const wrapperWithIssues = { itemContains: throwsException, }; +export const wrapperWithValuesToSanitize = { + get: () => Promise.resolve(10), + set: () => Promise.resolve('tRue'), + del: () => Promise.resolve(), // no result + getKeysByPrefix: () => Promise.resolve(['1', null, false, true, '2', null]), + incr: () => Promise.resolve('1'), + decr: () => Promise.resolve('0'), + getMany: () => Promise.resolve(['1', null, false, true, '2', null]), + connect: () => Promise.resolve(1), + getAndSet: () => Promise.resolve(true), + getByPrefix: () => Promise.resolve(['1', null, false, true, '2', null]), + popItems: () => Promise.resolve('invalid array'), + getItemsCount: () => Promise.resolve('10'), + itemContains: () => Promise.resolve('true'), +}; + +const SANITIZED_RESULTS = { + get: '10', + set: true, + del: false, + getKeysByPrefix: ['1', '2'], + incr: false, + decr: false, + getMany: ['1', null, '2', null], + connect: false, + getAndSet: 'true', + getByPrefix: ['1', '2'], + popItems: [], + getItemsCount: 10, + itemContains: true, +}; + const VALID_METHOD_CALLS = { 'get': ['some_key'], 'set': ['some_key', 'some_value'], @@ -104,4 +136,22 @@ describe('Wrapper Adapter', () => { expect(loggerMock.error).toBeCalledTimes(methods.length); }); + test('sanitize wrapper call results', async () => { + const instance = wrapperAdapter(loggerMock, wrapperWithValuesToSanitize); + const methods = Object.keys(SANITIZED_RESULTS); + + for (let i = 0; i < methods.length; i++) { + try { + const method = methods[i]; + const result = await instance[method](...VALID_METHOD_CALLS[method]); + + expect(result).toEqual(SANITIZED_RESULTS[method]); // result should be sanitized + } catch (e) { + expect(true).toBe(methods[i]); // promise shouldn't be rejected + } + } + + expect(loggerMock.warn).toBeCalledTimes(methods.length); // one warning for each wrapper operation call which result had to be sanitized + }); + }); diff --git a/src/storages/pluggable/wrapperAdapter.ts b/src/storages/pluggable/wrapperAdapter.ts index 06bcab0f..e668c335 100644 --- a/src/storages/pluggable/wrapperAdapter.ts +++ b/src/storages/pluggable/wrapperAdapter.ts @@ -1,24 +1,60 @@ +import { toString, isString, toNumber } from '../../utils/lang'; +import { sanitizeBoolean as sBoolean } from '../../evaluator/value/sanitize'; import { ILogger } from '../../logger/types'; import { SplitError } from '../../utils/lang/errors'; import { ICustomStorageWrapper } from '../types'; import { LOG_PREFIX } from './constants'; -const METHODS_TO_PROMISE_WRAP: string[] = [ - 'get', - 'set', - 'getAndSet', - 'del', - 'getKeysByPrefix', - 'getByPrefix', - 'incr', - 'decr', - 'getMany', - 'pushItems', - 'popItems', - 'getItemsCount', - 'itemContains', - 'connect', - 'close' +// Sanitizers return the given value if it is of the expected type, or a new sanitized one if invalid. +// @TODO review or remove sanitizers. Could be expensive and error-prone for producer methods + +function sanitizeBoolean(val: any): boolean { + return sBoolean(val) || false; +} +sanitizeBoolean.type = 'boolean'; + +function sanitizeNumber(val: any): number { + return toNumber(val); +} +sanitizeNumber.type = 'number'; + +function sanitizeArrayOfStrings(val: any): string[] { + if (!Array.isArray(val)) return []; // if not an array, returns a new empty one + if (val.every(isString)) return val; // if all items are valid, return the given array + return val.filter(isString); // otherwise, return a new array filtering the invalid items +} +sanitizeArrayOfStrings.type = 'Array'; + +function sanitizeNullableString(val: any): string | null { + if (val === null) return val; + return toString(val); // if not null, sanitize value as an string +} +sanitizeNullableString.type = 'string | null'; + +function sanitizeArrayOfNullableStrings(val: any): (string | null)[] { + const isStringOrNull = (v: any) => v === null || isString(v); + if (!Array.isArray(val)) return []; + if (val.every(isStringOrNull)) return val; + return val.filter(isStringOrNull); // otherwise, return a new array with items sanitized +} +sanitizeArrayOfNullableStrings.type = 'Array'; + +const METHODS_TO_PROMISE_WRAP: [string, undefined | { (val: any): any, type: string }][] = [ + ['get', sanitizeNullableString], + ['set', sanitizeBoolean], + ['getAndSet', sanitizeNullableString], + ['del', sanitizeBoolean], + ['getKeysByPrefix', sanitizeArrayOfStrings], + ['getByPrefix', sanitizeArrayOfStrings], + ['incr', sanitizeBoolean], + ['decr', sanitizeBoolean], + ['getMany', sanitizeArrayOfNullableStrings], + ['pushItems', undefined], + ['popItems', sanitizeArrayOfStrings], + ['getItemsCount', sanitizeNumber], + ['itemContains', sanitizeBoolean], + ['connect', sanitizeBoolean], + ['close', undefined], ]; /** @@ -33,7 +69,7 @@ export function wrapperAdapter(log: ILogger, wrapper: ICustomStorageWrapper): IC const wrapperAdapter: Record = {}; - METHODS_TO_PROMISE_WRAP.forEach((method) => { + METHODS_TO_PROMISE_WRAP.forEach(([method, sanitizer]) => { // Logs error and wraps it into an SplitError object (required to handle user callback errors in SDK readiness events) function handleError(e: any) { @@ -44,7 +80,15 @@ export function wrapperAdapter(log: ILogger, wrapper: ICustomStorageWrapper): IC wrapperAdapter[method] = function () { try { // @ts-ignore - return wrapper[method].apply(wrapper, arguments).then(value => value).catch(handleError); + return wrapper[method].apply(wrapper, arguments).then((value => { + if (!sanitizer) return value; + const sanitizedValue = sanitizer(value); + + // if value had to be sanitized, log a warning + if (sanitizedValue !== value) log.warn(`${LOG_PREFIX} Attempted to sanitize return value [${value}] of wrapper '${method}' operation which should be of type [${sanitizer.type}]. Sanitized and processed value => [${sanitizedValue}]`); + + return sanitizedValue; + })).catch(handleError); } catch (e) { return handleError(e); }