From b81488ff32e0be5dbfd39e5c7fcb883313a2166a Mon Sep 17 00:00:00 2001 From: Walt Jones Date: Fri, 25 Feb 2022 11:20:32 -0800 Subject: [PATCH] Handle circular references in custom data (#990) * fix: handle circular references in custom data * test: use deepStrictEqual to compare objects for equality --- src/utility.js | 41 ++++++++++++++++++++++++++--- test/browser.core.test.js | 52 +++++++++++++++++++++++++++++++++++++ test/server.rollbar.test.js | 12 ++++----- 3 files changed, 95 insertions(+), 10 deletions(-) diff --git a/src/utility.js b/src/utility.js index b871d6834..f1e2acbad 100644 --- a/src/utility.js +++ b/src/utility.js @@ -398,6 +398,37 @@ function wrapCallback(logger, f) { }; } +function nonCircularClone(obj) { + var seen = [obj]; + + function clone(obj, seen) { + var value, name, newSeen, result = {}; + + try { + for (name in obj) { + value = obj[name]; + + if (value && (isType(value, 'object') || isType(value, 'array'))) { + if (seen.includes(value)) { + result[name] = 'Removed circular reference: ' + typeName(value); + } else { + newSeen = seen.slice(); + newSeen.push(value); + result[name] = clone(value, newSeen); + } + continue; + } + + result[name] = value; + } + } catch (e) { + result = 'Failed cloning custom data: ' + e.message; + } + return result; + } + return clone(obj, seen); +} + function createItem(args, logger, notifier, requestKeys, lambdaContext) { var message, err, custom, callback, request; var arg; @@ -455,10 +486,12 @@ function createItem(args, logger, notifier, requestKeys, lambdaContext) { } } + // if custom is an array this turns it into an object with integer keys + if (custom) custom = nonCircularClone(custom); + if (extraArgs.length > 0) { - // if custom is an array this turns it into an object with integer keys - custom = merge(custom); - custom.extraArgs = extraArgs; + if (!custom) custom = nonCircularClone({}); + custom.extraArgs = nonCircularClone(extraArgs); } var item = { @@ -503,7 +536,7 @@ function addErrorContext(item, errors) { try { for (var i = 0; i < errors.length; ++i) { if (errors[i].hasOwnProperty('rollbarContext')) { - custom = merge(custom, errors[i].rollbarContext); + custom = merge(custom, nonCircularClone(errors[i].rollbarContext)); contextAdded = true; } } diff --git a/test/browser.core.test.js b/test/browser.core.test.js index af4deb34c..7b27b1bde 100644 --- a/test/browser.core.test.js +++ b/test/browser.core.test.js @@ -503,6 +503,58 @@ describe('log', function() { done(); }) + it('should remove circular references in custom data', function(done) { + var server = window.server; + stubResponse(server); + server.requests.length = 0; + + var options = { + accessToken: 'POST_CLIENT_ITEM_TOKEN', + addErrorContext: true + }; + var rollbar = window.rollbar = new Rollbar(options); + + var err = new Error('test error'); + var contextData = { extra: 'baz' } + contextData.data = contextData; + var context = { err: 'test', contextData: contextData }; + err.rollbarContext = context; + + var array = ['one', 'two']; + array.push(array); + var custom = { foo: 'bar', array: array }; + var notCircular = { key: 'value' }; + custom.notCircular1 = notCircular; + custom.notCircular2 = notCircular; + custom.self = custom; + rollbar.error(err, custom); + + server.respond(); + + var body = JSON.parse(server.requests[0].requestBody); + + expect(body.data.body.trace.exception.message).to.eql('test error'); + expect(body.data.custom.foo).to.eql('bar'); + expect(body.data.custom.err).to.eql('test'); + + // Duplicate objects are allowed when there is no circular reference. + expect(body.data.custom.notCircular1).to.eql(notCircular); + expect(body.data.custom.notCircular2).to.eql(notCircular); + + expect(body.data.custom.self).to.eql( + 'Removed circular reference: object' + ); + expect(body.data.custom.array).to.eql([ + 'one', 'two', 'Removed circular reference: array' + ]); + expect(body.data.custom.contextData).to.eql({ + extra: 'baz', + data: 'Removed circular reference: object' + }); + + done(); + }) + it('should send message when called with only null arguments', function(done) { var server = window.server; stubResponse(server); diff --git a/test/server.rollbar.test.js b/test/server.rollbar.test.js index a84cbe1d1..21683be03 100644 --- a/test/server.rollbar.test.js +++ b/test/server.rollbar.test.js @@ -278,7 +278,7 @@ vows.describe('rollbar') var item = r.client.logCalls[r.client.logCalls.length - 1].item; assert.equal(item.message, message); assert.equal(item.request, request); - assert.equal(item.custom, custom); + assert.deepStrictEqual(item.custom, custom); item.callback(); assert.isTrue(callbackCalled); } @@ -332,7 +332,7 @@ vows.describe('rollbar') var item = r.client.logCalls[r.client.logCalls.length - 1].item assert.equal(item.message, message) assert.equal(item.request, request) - assert.equal(item.custom, custom) + assert.deepStrictEqual(item.custom, custom) }, 'should work with request and custom and callback': function(r) { var message = 'hello' @@ -346,7 +346,7 @@ vows.describe('rollbar') var item = r.client.logCalls[r.client.logCalls.length - 1].item assert.equal(item.message, message) assert.equal(item.request, request) - assert.equal(item.custom, custom) + assert.deepStrictEqual(item.custom, custom) item.callback(); assert.isTrue(callbackCalled); } @@ -399,7 +399,7 @@ vows.describe('rollbar') var item = r.client.logCalls[r.client.logCalls.length - 1].item assert.equal(item.err, err) assert.equal(item.request, request) - assert.equal(item.custom, custom) + assert.deepStrictEqual(item.custom, custom) item.callback(); assert.isTrue(callbackCalled); } @@ -453,7 +453,7 @@ vows.describe('rollbar') var item = r.client.logCalls[r.client.logCalls.length - 1].item assert.equal(item.err, err) assert.equal(item.request, request) - assert.equal(item.custom, custom) + assert.deepStrictEqual(item.custom, custom) }, 'should work with request and custom and callback': function(r) { var err = new Error('hello!') @@ -467,7 +467,7 @@ vows.describe('rollbar') var item = r.client.logCalls[r.client.logCalls.length - 1].item assert.equal(item.err, err) assert.equal(item.request, request) - assert.equal(item.custom, custom) + assert.deepStrictEqual(item.custom, custom) item.callback(); assert.isTrue(callbackCalled); }