Skip to content

Commit

Permalink
fix: address logging infinite recursion issue
Browse files Browse the repository at this point in the history
This cropped up as a bug recently where Content Discovery were trying to
log an entire Request object, which references itself. This resulted in
Node.js running out of memory because we were infinitely recursing over
the properties.

This is a bug that's always been present in n-mask-logger.

I've addressed the issue with my first legit use of `WeakSet` and I'm
quite pleased with myself about it 馃榿
  • Loading branch information
rowanmanning committed Jul 7, 2023
1 parent 2a316fd commit 1eed362
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
14 changes: 13 additions & 1 deletion packages/logger/lib/transforms/legacy-mask.js
Expand Up @@ -16,6 +16,8 @@
* A regular expression which applies masking to a string.
* @property {string} maskString
* The mask string to apply to discovered sensitive values.
* @property {WeakSet} references
* An internal store of references used to avoid masking the same object infinitely.
*/

/**
Expand Down Expand Up @@ -69,6 +71,15 @@ function mask(value, settings) {
if (typeof value === 'string') {
return maskString(value, settings);
}

// Handle circular references
if (value && typeof value === 'object') {
if (settings.references.has(value)) {
return value;
}
settings.references.add(value);
}

if (Array.isArray(value)) {
return value.map((item) => mask(item, settings));
}
Expand Down Expand Up @@ -177,7 +188,8 @@ function createLegacyMaskTransform({
return maskObject(logData, {
maskedFields,
maskRegExp,
maskString
maskString,
references: new WeakSet()
});
};
}
Expand Down
12 changes: 12 additions & 0 deletions packages/logger/test/unit/lib/transforms/legacy-mask.spec.js
Expand Up @@ -217,6 +217,18 @@ describe('@dotcom-reliability-kit/logger', () => {
expect(transform({ mock: 123 })).toEqual({ mock: 123 });
});
});

describe('when called with an object that references itself', () => {
it('does not recurse infinitely', () => {
const logData = {
email: 'mock'
};
logData.naughtyLittleSelfReference = logData;
const result = transform(logData);
expect(result.email).toEqual('*****');
expect(result.naughtyLittleSelfReference.email).toEqual('*****');
});
});
});

describe('when `options.denyList` is set', () => {
Expand Down

0 comments on commit 1eed362

Please sign in to comment.