From 96b9ce580d23367bd490da678fa48db7dcf17ab9 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 7 Mar 2022 15:11:53 +0000 Subject: [PATCH 1/4] Max properties --- packages/core/src/baseclient.ts | 14 ++-- packages/types/src/options.ts | 11 +++ packages/utils/src/object.ts | 128 ++++++++++++++--------------- packages/utils/test/object.test.ts | 45 ++++++++++ 4 files changed, 127 insertions(+), 71 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index b831284d221f..4e426c4b6dbc 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -346,7 +346,7 @@ export abstract class BaseClient implement * @returns A new event with more information. */ protected _prepareEvent(event: Event, scope?: Scope, hint?: EventHint): PromiseLike { - const { normalizeDepth = 3 } = this.getOptions(); + const { normalizeDepth = 3, normalizeMaxProperties = 1_000 } = this.getOptions(); const prepared: Event = { ...event, event_id: event.event_id || (hint && hint.event_id ? hint.event_id : uuid4()), @@ -380,7 +380,7 @@ export abstract class BaseClient implement evt.sdkProcessingMetadata = { ...evt.sdkProcessingMetadata, normalizeDepth: normalize(normalizeDepth) }; } if (typeof normalizeDepth === 'number' && normalizeDepth > 0) { - return this._normalizeEvent(evt, normalizeDepth); + return this._normalizeEvent(evt, normalizeDepth, normalizeMaxProperties); } return evt; }); @@ -396,7 +396,7 @@ export abstract class BaseClient implement * @param event Event * @returns Normalized event */ - protected _normalizeEvent(event: Event | null, depth: number): Event | null { + protected _normalizeEvent(event: Event | null, depth: number, maxProperties: number): Event | null { if (!event) { return null; } @@ -407,18 +407,18 @@ export abstract class BaseClient implement breadcrumbs: event.breadcrumbs.map(b => ({ ...b, ...(b.data && { - data: normalize(b.data, depth), + data: normalize(b.data, depth, maxProperties), }), })), }), ...(event.user && { - user: normalize(event.user, depth), + user: normalize(event.user, depth, maxProperties), }), ...(event.contexts && { - contexts: normalize(event.contexts, depth), + contexts: normalize(event.contexts, depth, maxProperties), }), ...(event.extra && { - extra: normalize(event.extra, depth), + extra: normalize(event.extra, depth, maxProperties), }), }; // event.contexts.trace stores information about a Transaction. Similarly, diff --git a/packages/types/src/options.ts b/packages/types/src/options.ts index 483af0f94903..74c2fbb47844 100644 --- a/packages/types/src/options.ts +++ b/packages/types/src/options.ts @@ -101,6 +101,17 @@ export interface Options { */ normalizeDepth?: number; + /** + * Maximum number of properties or elements that the normalization algorithm will output. + * Used when normalizing an event before sending, on all of the listed attributes: + * - `breadcrumbs.data` + * - `user` + * - `contexts` + * - `extra` + * Defaults to `1000` + */ + normalizeMaxProperties?: number; + /** * Controls how many milliseconds to wait before shutting down. The default is * SDK-specific but typically around 2 seconds. Setting this too low can cause diff --git a/packages/utils/src/object.ts b/packages/utils/src/object.ts index 568d1b34f99b..ad8b9a53492b 100644 --- a/packages/utils/src/object.ts +++ b/packages/utils/src/object.ts @@ -4,7 +4,7 @@ import { ExtendedError, WrappedFunction } from '@sentry/types'; import { htmlTreeAsString } from './browser'; import { isElement, isError, isEvent, isInstanceOf, isPlainObject, isPrimitive, isSyntheticEvent } from './is'; -import { memoBuilder, MemoFunc } from './memo'; +import { memoBuilder } from './memo'; import { getFunctionName } from './stacktrace'; import { truncate } from './string'; @@ -300,85 +300,85 @@ function makeSerializable(value: T, key?: any): T | string { return value; } +type UnknownMaybeToJson = unknown & { toJSON?: () => string }; + /** - * Walks an object to perform a normalization on it + * normalize() * - * @param key of object that's walked in current iteration - * @param value object to be walked - * @param depth Optional number indicating how deep should walking be performed - * @param memo Optional Memo class handling decycling + * - Creates a copy to prevent original input mutation + * - Skip non-enumerable + * - Calls `toJSON` if implemented + * - Removes circular references + * - Translates non-serializable values (undefined/NaN/Functions) to serializable format + * - Translates known global objects/Classes to a string representations + * - Takes care of Error objects serialization + * - Optionally limit depth of final output + * - Optionally limit max number of properties/elements for each object/array */ -// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types -export function walk(key: string, value: any, depth: number = +Infinity, memo: MemoFunc = memoBuilder()): any { - const [memoize, unmemoize] = memo; +export function normalize(input: unknown, depth: number = +Infinity, maxProperties: number = +Infinity): any { + const [memoize, unmemoize] = memoBuilder(); - // If we reach the maximum depth, serialize whatever is left - if (depth === 0) { - return serializeValue(value); - } + function walk(key: string, value: UnknownMaybeToJson, depth: number = +Infinity): unknown { + // If we reach the maximum depth, serialize whatever is left + if (depth === 0) { + return serializeValue(value); + } - /* eslint-disable @typescript-eslint/no-unsafe-member-access */ - // If value implements `toJSON` method, call it and return early - if (value !== null && value !== undefined && typeof value.toJSON === 'function') { - return value.toJSON(); - } - /* eslint-enable @typescript-eslint/no-unsafe-member-access */ + // If value implements `toJSON` method, call it and return early + if (value !== null && value !== undefined && typeof value.toJSON === 'function') { + return value.toJSON(); + } - // `makeSerializable` provides a string representation of certain non-serializable values. For all others, it's a - // pass-through. If what comes back is a primitive (either because it's been stringified or because it was primitive - // all along), we're done. - const serializable = makeSerializable(value, key); - if (isPrimitive(serializable)) { - return serializable; - } + // `makeSerializable` provides a string representation of certain non-serializable values. For all others, it's a + // pass-through. If what comes back is a primitive (either because it's been stringified or because it was primitive + // all along), we're done. + const serializable = makeSerializable(value, key); + if (isPrimitive(serializable)) { + return serializable; + } - // Create source that we will use for the next iteration. It will either be an objectified error object (`Error` type - // with extracted key:value pairs) or the input itself. - const source = getWalkSource(value); + // Create source that we will use for the next iteration. It will either be an objectified error object (`Error` type + // with extracted key:value pairs) or the input itself. + const source = getWalkSource(value); - // Create an accumulator that will act as a parent for all future itterations of that branch - const acc: { [key: string]: any } = Array.isArray(value) ? [] : {}; + // Create an accumulator that will act as a parent for all future itterations of that branch + const acc: { [key: string]: any } = Array.isArray(value) ? [] : {}; - // If we already walked that branch, bail out, as it's circular reference - if (memoize(value)) { - return '[Circular ~]'; - } + // If we already walked that branch, bail out, as it's circular reference + if (memoize(value)) { + return '[Circular ~]'; + } - // Walk all keys of the source - for (const innerKey in source) { - // Avoid iterating over fields in the prototype if they've somehow been exposed to enumeration. - if (!Object.prototype.hasOwnProperty.call(source, innerKey)) { - continue; + let propertyCount = 0; + // Walk all keys of the source + for (const innerKey in source) { + // Avoid iterating over fields in the prototype if they've somehow been exposed to enumeration. + if (!Object.prototype.hasOwnProperty.call(source, innerKey)) { + continue; + } + + if (propertyCount >= maxProperties) { + acc[innerKey] = '[MaxProperties ~]'; + break; + } + + propertyCount += 1; + + // Recursively walk through all the child nodes + const innerValue: UnknownMaybeToJson = source[innerKey]; + acc[innerKey] = walk(innerKey, innerValue, depth - 1); } - // Recursively walk through all the child nodes - const innerValue: any = source[innerKey]; - acc[innerKey] = walk(innerKey, innerValue, depth - 1, memo); - } - // Once walked through all the branches, remove the parent from memo storage - unmemoize(value); + // Once walked through all the branches, remove the parent from memo storage + unmemoize(value); - // Return accumulated values - return acc; -} + // Return accumulated values + return acc; + } -/** - * normalize() - * - * - Creates a copy to prevent original input mutation - * - Skip non-enumerablers - * - Calls `toJSON` if implemented - * - Removes circular references - * - Translates non-serializeable values (undefined/NaN/Functions) to serializable format - * - Translates known global objects/Classes to a string representations - * - Takes care of Error objects serialization - * - Optionally limit depth of final output - */ -// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types -export function normalize(input: any, depth?: number): any { try { // since we're at the outermost level, there is no key - return walk('', input, depth); + return walk('', input as UnknownMaybeToJson, depth); } catch (_oO) { return '**non-serializable**'; } diff --git a/packages/utils/test/object.test.ts b/packages/utils/test/object.test.ts index dffa23e22408..b0b103683978 100644 --- a/packages/utils/test/object.test.ts +++ b/packages/utils/test/object.test.ts @@ -533,6 +533,51 @@ describe('normalize()', () => { }); }); + describe('can limit max properties', () => { + test('object', () => { + const obj = { + nope: 'here', + foo: { + one: 1, + two: 2, + three: 3, + four: 4, + five: 5, + six: 6, + seven: 7, + }, + after: 'more', + }; + + expect(normalize(obj, 10, 5)).toEqual({ + nope: 'here', + foo: { + one: 1, + two: 2, + three: 3, + four: 4, + five: 5, + six: '[MaxProperties ~]', + }, + after: 'more', + }); + }); + + test('array', () => { + const obj = { + nope: 'here', + foo: new Array(100).fill('s'), + after: 'more', + }; + + expect(normalize(obj, 10, 5)).toEqual({ + nope: 'here', + foo: ['s', 's', 's', 's', 's', '[MaxProperties ~]'], + after: 'more', + }); + }); + }); + test('normalizes value on every iteration of decycle and takes care of things like Reacts SyntheticEvents', () => { const obj = { foo: { From 4a1b499dad95802e92b98f8ca71dcbcdc0584f4c Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 7 Mar 2022 19:45:06 +0000 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Katie Byers --- packages/types/src/options.ts | 2 +- packages/utils/src/object.ts | 24 +++++++++++++++--------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/types/src/options.ts b/packages/types/src/options.ts index 74c2fbb47844..2e5a5d7a0aa1 100644 --- a/packages/types/src/options.ts +++ b/packages/types/src/options.ts @@ -102,7 +102,7 @@ export interface Options { normalizeDepth?: number; /** - * Maximum number of properties or elements that the normalization algorithm will output. + * Maximum number of properties or elements that the normalization algorithm will output in any single array or object included in the normalized event. * Used when normalizing an event before sending, on all of the listed attributes: * - `breadcrumbs.data` * - `user` diff --git a/packages/utils/src/object.ts b/packages/utils/src/object.ts index ad8b9a53492b..3c33691806fd 100644 --- a/packages/utils/src/object.ts +++ b/packages/utils/src/object.ts @@ -300,20 +300,26 @@ function makeSerializable(value: T, key?: any): T | string { return value; } -type UnknownMaybeToJson = unknown & { toJSON?: () => string }; +type UnknownMaybeWithToJson = unknown & { toJSON?: () => string }; /** - * normalize() + * Recursively normalizes the given object. * * - Creates a copy to prevent original input mutation - * - Skip non-enumerable - * - Calls `toJSON` if implemented + * - Skips non-enumerable properties + * - When stringifying, calls `toJSON` if implemented * - Removes circular references - * - Translates non-serializable values (undefined/NaN/Functions) to serializable format - * - Translates known global objects/Classes to a string representations - * - Takes care of Error objects serialization - * - Optionally limit depth of final output - * - Optionally limit max number of properties/elements for each object/array + * - Translates non-serializable values (`undefined`/`NaN`/functions) to serializable format + * - Translates known global objects/classes to a string representations + * - Takes care of `Error` object serialization + * - Optionally limits depth of final output + * - Optionally limits number of properties/elements included in any single object/array + * + * @param input The object to be normalized. + * @param depth The max depth to which to normalize the object. (Anything deeper stringified whole.) + * @param maxProperties The max number of elements or properties to be included in any single array or + * object in the normallized output.. + * @returns A normalized version of the object, or `"**non-serializable**"` if any errors are thrown during normaliztion. */ export function normalize(input: unknown, depth: number = +Infinity, maxProperties: number = +Infinity): any { const [memoize, unmemoize] = memoBuilder(); From 48fc389e3a7631efc48513003177b59cef97f0db Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 7 Mar 2022 19:54:19 +0000 Subject: [PATCH 3/4] get `walk` out of there! --- packages/utils/src/object.ts | 143 +++++++++++++++++++---------------- 1 file changed, 79 insertions(+), 64 deletions(-) diff --git a/packages/utils/src/object.ts b/packages/utils/src/object.ts index 3c33691806fd..4c9d9de2da88 100644 --- a/packages/utils/src/object.ts +++ b/packages/utils/src/object.ts @@ -4,7 +4,7 @@ import { ExtendedError, WrappedFunction } from '@sentry/types'; import { htmlTreeAsString } from './browser'; import { isElement, isError, isEvent, isInstanceOf, isPlainObject, isPrimitive, isSyntheticEvent } from './is'; -import { memoBuilder } from './memo'; +import { memoBuilder, MemoFunc } from './memo'; import { getFunctionName } from './stacktrace'; import { truncate } from './string'; @@ -302,6 +302,81 @@ function makeSerializable(value: T, key?: any): T | string { type UnknownMaybeWithToJson = unknown & { toJSON?: () => string }; +/** + * Walks an object to perform a normalization on it + * + * @param key of object that's walked in current iteration + * @param value object to be walked + * @param depth Optional number indicating how deep should walking be performed + * @param maxProperties Optional maximum number of properties/elements included in any single object/array + * @param memo Optional Memo class handling decycling + */ +export function walk( + key: string, + value: UnknownMaybeWithToJson, + depth: number = +Infinity, + maxProperties: number = +Infinity, + memo: MemoFunc = memoBuilder(), +): unknown { + const [memoize, unmemoize] = memo; + + // If we reach the maximum depth, serialize whatever is left + if (depth === 0) { + return serializeValue(value); + } + + // If value implements `toJSON` method, call it and return early + if (value !== null && value !== undefined && typeof value.toJSON === 'function') { + return value.toJSON(); + } + + // `makeSerializable` provides a string representation of certain non-serializable values. For all others, it's a + // pass-through. If what comes back is a primitive (either because it's been stringified or because it was primitive + // all along), we're done. + const serializable = makeSerializable(value, key); + if (isPrimitive(serializable)) { + return serializable; + } + + // Create source that we will use for the next iteration. It will either be an objectified error object (`Error` type + // with extracted key:value pairs) or the input itself. + const source = getWalkSource(value); + + // Create an accumulator that will act as a parent for all future itterations of that branch + const acc: { [key: string]: any } = Array.isArray(value) ? [] : {}; + + // If we already walked that branch, bail out, as it's circular reference + if (memoize(value)) { + return '[Circular ~]'; + } + + let propertyCount = 0; + // Walk all keys of the source + for (const innerKey in source) { + // Avoid iterating over fields in the prototype if they've somehow been exposed to enumeration. + if (!Object.prototype.hasOwnProperty.call(source, innerKey)) { + continue; + } + + if (propertyCount >= maxProperties) { + acc[innerKey] = '[MaxProperties ~]'; + break; + } + + propertyCount += 1; + + // Recursively walk through all the child nodes + const innerValue: UnknownMaybeWithToJson = source[innerKey]; + acc[innerKey] = walk(innerKey, innerValue, depth - 1, maxProperties, memo); + } + + // Once walked through all the branches, remove the parent from memo storage + unmemoize(value); + + // Return accumulated values + return acc; +} + /** * Recursively normalizes the given object. * @@ -317,74 +392,14 @@ type UnknownMaybeWithToJson = unknown & { toJSON?: () => string }; * * @param input The object to be normalized. * @param depth The max depth to which to normalize the object. (Anything deeper stringified whole.) - * @param maxProperties The max number of elements or properties to be included in any single array or + * @param maxProperties The max number of elements or properties to be included in any single array or * object in the normallized output.. - * @returns A normalized version of the object, or `"**non-serializable**"` if any errors are thrown during normaliztion. + * @returns A normalized version of the object, or `"**non-serializable**"` if any errors are thrown during normalization. */ export function normalize(input: unknown, depth: number = +Infinity, maxProperties: number = +Infinity): any { - const [memoize, unmemoize] = memoBuilder(); - - function walk(key: string, value: UnknownMaybeToJson, depth: number = +Infinity): unknown { - // If we reach the maximum depth, serialize whatever is left - if (depth === 0) { - return serializeValue(value); - } - - // If value implements `toJSON` method, call it and return early - if (value !== null && value !== undefined && typeof value.toJSON === 'function') { - return value.toJSON(); - } - - // `makeSerializable` provides a string representation of certain non-serializable values. For all others, it's a - // pass-through. If what comes back is a primitive (either because it's been stringified or because it was primitive - // all along), we're done. - const serializable = makeSerializable(value, key); - if (isPrimitive(serializable)) { - return serializable; - } - - // Create source that we will use for the next iteration. It will either be an objectified error object (`Error` type - // with extracted key:value pairs) or the input itself. - const source = getWalkSource(value); - - // Create an accumulator that will act as a parent for all future itterations of that branch - const acc: { [key: string]: any } = Array.isArray(value) ? [] : {}; - - // If we already walked that branch, bail out, as it's circular reference - if (memoize(value)) { - return '[Circular ~]'; - } - - let propertyCount = 0; - // Walk all keys of the source - for (const innerKey in source) { - // Avoid iterating over fields in the prototype if they've somehow been exposed to enumeration. - if (!Object.prototype.hasOwnProperty.call(source, innerKey)) { - continue; - } - - if (propertyCount >= maxProperties) { - acc[innerKey] = '[MaxProperties ~]'; - break; - } - - propertyCount += 1; - - // Recursively walk through all the child nodes - const innerValue: UnknownMaybeToJson = source[innerKey]; - acc[innerKey] = walk(innerKey, innerValue, depth - 1); - } - - // Once walked through all the branches, remove the parent from memo storage - unmemoize(value); - - // Return accumulated values - return acc; - } - try { // since we're at the outermost level, there is no key - return walk('', input as UnknownMaybeToJson, depth); + return walk('', input as UnknownMaybeWithToJson, depth, maxProperties); } catch (_oO) { return '**non-serializable**'; } From e7583984eb7f2baa2f557175c9976f54d458d16a Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 15 Mar 2022 09:53:33 +0000 Subject: [PATCH 4/4] Rename to normalizeMaxBreadth --- packages/core/src/baseclient.ts | 14 +++++++------- packages/types/src/options.ts | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 0c96b60a79a9..a8d444c36c46 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -342,7 +342,7 @@ export abstract class BaseClient implement * @returns A new event with more information. */ protected _prepareEvent(event: Event, scope?: Scope, hint?: EventHint): PromiseLike { - const { normalizeDepth = 3, normalizeMaxProperties = 1_000 } = this.getOptions(); + const { normalizeDepth = 3, normalizeMaxBreadth = 1_000 } = this.getOptions(); const prepared: Event = { ...event, event_id: event.event_id || (hint && hint.event_id ? hint.event_id : uuid4()), @@ -376,7 +376,7 @@ export abstract class BaseClient implement evt.sdkProcessingMetadata = { ...evt.sdkProcessingMetadata, normalizeDepth: normalize(normalizeDepth) }; } if (typeof normalizeDepth === 'number' && normalizeDepth > 0) { - return this._normalizeEvent(evt, normalizeDepth, normalizeMaxProperties); + return this._normalizeEvent(evt, normalizeDepth, normalizeMaxBreadth); } return evt; }); @@ -392,7 +392,7 @@ export abstract class BaseClient implement * @param event Event * @returns Normalized event */ - protected _normalizeEvent(event: Event | null, depth: number, maxProperties: number): Event | null { + protected _normalizeEvent(event: Event | null, depth: number, maxBreadth: number): Event | null { if (!event) { return null; } @@ -403,18 +403,18 @@ export abstract class BaseClient implement breadcrumbs: event.breadcrumbs.map(b => ({ ...b, ...(b.data && { - data: normalize(b.data, depth, maxProperties), + data: normalize(b.data, depth, maxBreadth), }), })), }), ...(event.user && { - user: normalize(event.user, depth, maxProperties), + user: normalize(event.user, depth, maxBreadth), }), ...(event.contexts && { - contexts: normalize(event.contexts, depth, maxProperties), + contexts: normalize(event.contexts, depth, maxBreadth), }), ...(event.extra && { - extra: normalize(event.extra, depth, maxProperties), + extra: normalize(event.extra, depth, maxBreadth), }), }; // event.contexts.trace stores information about a Transaction. Similarly, diff --git a/packages/types/src/options.ts b/packages/types/src/options.ts index 2e5a5d7a0aa1..3fefd41ca137 100644 --- a/packages/types/src/options.ts +++ b/packages/types/src/options.ts @@ -110,7 +110,7 @@ export interface Options { * - `extra` * Defaults to `1000` */ - normalizeMaxProperties?: number; + normalizeMaxBreadth?: number; /** * Controls how many milliseconds to wait before shutting down. The default is