From 26093ac786259e777753792eb4eb90dcb45608b3 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 9 May 2022 07:30:06 +0000 Subject: [PATCH 1/3] ref(utils): Add property flag to skip normalization --- packages/utils/src/normalize.ts | 22 ++++++++---- packages/utils/test/normalize.test.ts | 49 +++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/packages/utils/src/normalize.ts b/packages/utils/src/normalize.ts index 51566a2125a7..b64ac69f46a1 100644 --- a/packages/utils/src/normalize.ts +++ b/packages/utils/src/normalize.ts @@ -33,7 +33,7 @@ type ObjOrArray = { [key: string]: T }; */ export function normalize(input: unknown, depth: number = +Infinity, maxProperties: number = +Infinity): any { try { - // since we're at the outermost level, there is no key + // since we're at the outermost level, we don't provide a key return visit('', input, depth, maxProperties); } catch (err) { return { ERROR: `**non-serializable** (${err})` }; @@ -98,17 +98,27 @@ function visit( return stringified; } - // We're also done if we've reached the max depth - if (depth === 0) { - // At this point we know `serialized` is a string of the form `"[object XXXX]"`. Clean it up so it's just `"[XXXX]"`. - return stringified.replace('object ', ''); - } + // From here on, we can assert that `value` is either an object or an array! // If we've already visited this branch, bail out, as it's circular reference. If not, note that we're seeing it now. if (memoize(value)) { return '[Circular ~]'; } + // Do not normalize objects that we know have already been normalized. This MUST be below the circ-ref check otherwise + // we might somehow accidentally produce circular references by skipping normalization. + // As a general rule, the "__sentry_skip_normalization__" property should only be used sparingly and only should only + // be set on objects that have already been normalized. + if ((value as ObjOrArray)['__sentry_skip_normalization__']) { + return value as ObjOrArray; + } + + // We're also done if we've reached the max depth + if (depth === 0) { + // At this point we know `serialized` is a string of the form `"[object XXXX]"`. Clean it up so it's just `"[XXXX]"`. + return stringified.replace('object ', ''); + } + // At this point we know we either have an object or an array, we haven't seen it before, and we're going to recurse // because we haven't yet reached the max depth. Create an accumulator to hold the results of visiting each // property/entry, and keep track of the number of items we add to it. diff --git a/packages/utils/test/normalize.test.ts b/packages/utils/test/normalize.test.ts index 756020b403de..939c7cd1b51f 100644 --- a/packages/utils/test/normalize.test.ts +++ b/packages/utils/test/normalize.test.ts @@ -4,6 +4,7 @@ import * as isModule from '../src/is'; import { normalize } from '../src/normalize'; +import { addNonEnumerableProperty } from '../src/object'; import * as stacktraceModule from '../src/stacktrace'; describe('normalize()', () => { @@ -504,4 +505,52 @@ describe('normalize()', () => { qux: '[Function: qux]', }); }); + + describe.only('skips normalizing objects marked with a non-enumerable property __sentry_skip_normalization__', () => { + test('by leaving non-serializable values intact', () => { + const someFun = () => undefined; + const alreadyNormalizedObj = { + nan: NaN, + fun: someFun, + }; + + addNonEnumerableProperty(alreadyNormalizedObj, '__sentry_skip_normalization__', true); + + const result = normalize(alreadyNormalizedObj); + expect(result).toEqual({ + nan: NaN, + fun: someFun, + }); + }); + + test('by ignoring normalization depth', () => { + const alreadyNormalizedObj = { + three: { + more: { + layers: '!', + }, + }, + }; + + addNonEnumerableProperty(alreadyNormalizedObj, '__sentry_skip_normalization__', true); + + const obj = { + foo: { + bar: { + baz: alreadyNormalizedObj, + boo: { + bam: { + pow: 'poof', + }, + }, + }, + }, + }; + + const result = normalize(obj, 4); + + expect(result?.foo?.bar?.baz?.three?.more?.layers).toBe('!'); + expect(result?.foo?.bar?.boo?.bam?.pow).not.toBe('poof'); + }); + }); }); From 77e3096bbfca3c1af732b8ed00f98fb6b71f2a40 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 9 May 2022 13:38:17 +0000 Subject: [PATCH 2/3] Restore world order --- packages/utils/src/normalize.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/utils/src/normalize.ts b/packages/utils/src/normalize.ts index b64ac69f46a1..5e97ad6e069b 100644 --- a/packages/utils/src/normalize.ts +++ b/packages/utils/src/normalize.ts @@ -98,17 +98,11 @@ function visit( return stringified; } - // From here on, we can assert that `value` is either an object or an array! + // From here on, we can assert that `value` is either an object or an array. - // If we've already visited this branch, bail out, as it's circular reference. If not, note that we're seeing it now. - if (memoize(value)) { - return '[Circular ~]'; - } - - // Do not normalize objects that we know have already been normalized. This MUST be below the circ-ref check otherwise - // we might somehow accidentally produce circular references by skipping normalization. - // As a general rule, the "__sentry_skip_normalization__" property should only be used sparingly and only should only - // be set on objects that have already been normalized. + // Do not normalize objects that we know have already been normalized. As a general rule, the + // "__sentry_skip_normalization__" property should only be used sparingly and only should only be set on objects that + // have already been normalized. if ((value as ObjOrArray)['__sentry_skip_normalization__']) { return value as ObjOrArray; } @@ -119,6 +113,11 @@ function visit( return stringified.replace('object ', ''); } + // If we've already visited this branch, bail out, as it's circular reference. If not, note that we're seeing it now. + if (memoize(value)) { + return '[Circular ~]'; + } + // At this point we know we either have an object or an array, we haven't seen it before, and we're going to recurse // because we haven't yet reached the max depth. Create an accumulator to hold the results of visiting each // property/entry, and keep track of the number of items we add to it. From fed0c117b57e5f8e1177ba0bb8c5e9087ce91f79 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 9 May 2022 13:39:12 +0000 Subject: [PATCH 3/3] Remove .only --- packages/utils/test/normalize.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils/test/normalize.test.ts b/packages/utils/test/normalize.test.ts index 939c7cd1b51f..e5d01de4e962 100644 --- a/packages/utils/test/normalize.test.ts +++ b/packages/utils/test/normalize.test.ts @@ -506,7 +506,7 @@ describe('normalize()', () => { }); }); - describe.only('skips normalizing objects marked with a non-enumerable property __sentry_skip_normalization__', () => { + describe('skips normalizing objects marked with a non-enumerable property __sentry_skip_normalization__', () => { test('by leaving non-serializable values intact', () => { const someFun = () => undefined; const alreadyNormalizedObj = {