Skip to content

Commit

Permalink
feat(utils): Limit normalize maximum properties/elements (#4689)
Browse files Browse the repository at this point in the history
This limits the number of properties/elements serialized for an object/array, to protect against huge objects being serialized if users inadvertently log them. To control this, a new `normalizeMaxBreadth` option has been added, which defaults to 1000.

Documented in getsentry/sentry-docs#4844.
  • Loading branch information
timfish authored Mar 16, 2022
1 parent cc44957 commit 72aed62
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 23 deletions.
14 changes: 7 additions & 7 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
* @returns A new event with more information.
*/
protected _prepareEvent(event: Event, scope?: Scope, hint?: EventHint): PromiseLike<Event | null> {
const { normalizeDepth = 3 } = 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()),
Expand Down Expand Up @@ -376,7 +376,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
evt.sdkProcessingMetadata = { ...evt.sdkProcessingMetadata, normalizeDepth: normalize(normalizeDepth) };
}
if (typeof normalizeDepth === 'number' && normalizeDepth > 0) {
return this._normalizeEvent(evt, normalizeDepth);
return this._normalizeEvent(evt, normalizeDepth, normalizeMaxBreadth);
}
return evt;
});
Expand All @@ -392,7 +392,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
* @param event Event
* @returns Normalized event
*/
protected _normalizeEvent(event: Event | null, depth: number): Event | null {
protected _normalizeEvent(event: Event | null, depth: number, maxBreadth: number): Event | null {
if (!event) {
return null;
}
Expand All @@ -403,18 +403,18 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
breadcrumbs: event.breadcrumbs.map(b => ({
...b,
...(b.data && {
data: normalize(b.data, depth),
data: normalize(b.data, depth, maxBreadth),
}),
})),
}),
...(event.user && {
user: normalize(event.user, depth),
user: normalize(event.user, depth, maxBreadth),
}),
...(event.contexts && {
contexts: normalize(event.contexts, depth),
contexts: normalize(event.contexts, depth, maxBreadth),
}),
...(event.extra && {
extra: normalize(event.extra, depth),
extra: normalize(event.extra, depth, maxBreadth),
}),
};
// event.contexts.trace stores information about a Transaction. Similarly,
Expand Down
11 changes: 11 additions & 0 deletions packages/types/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,17 @@ export interface Options {
*/
normalizeDepth?: number;

/**
* 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`
* - `contexts`
* - `extra`
* Defaults to `1000`
*/
normalizeMaxBreadth?: 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
Expand Down
53 changes: 37 additions & 16 deletions packages/utils/src/object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,29 +300,35 @@ function makeSerializable<T>(value: T, key?: any): T | string {
return value;
}

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
*/
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
export function walk(key: string, value: any, depth: number = +Infinity, memo: MemoFunc = memoBuilder()): any {
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);
}

/* 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 */

// `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
Expand All @@ -344,15 +350,24 @@ export function walk(key: string, value: any, depth: number = +Infinity, memo: M
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: any = source[innerKey];
acc[innerKey] = walk(innerKey, innerValue, depth - 1, memo);
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
Expand All @@ -363,22 +378,28 @@ export function walk(key: string, value: any, depth: number = +Infinity, memo: M
}

/**
* normalize()
* Recursively normalizes the given object.
*
* - Creates a copy to prevent original input mutation
* - Skip non-enumerablers
* - Calls `toJSON` if implemented
* - Skips non-enumerable properties
* - When stringifying, 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
* - 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 normalization.
*/
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
export function normalize(input: any, depth?: number): any {
export function normalize(input: unknown, depth: number = +Infinity, maxProperties: number = +Infinity): any {
try {
// since we're at the outermost level, there is no key
return walk('', input, depth);
return walk('', input as UnknownMaybeWithToJson, depth, maxProperties);
} catch (_oO) {
return '**non-serializable**';
}
Expand Down
45 changes: 45 additions & 0 deletions packages/utils/test/object.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down

0 comments on commit 72aed62

Please sign in to comment.