From ce1703a84d2712a7df8f93a3f3ecbb2c04b72e57 Mon Sep 17 00:00:00 2001 From: Riley Dulin Date: Mon, 2 Mar 2020 17:25:24 -0800 Subject: [PATCH] Change stringifySafe to have limits on larger objects Summary: In heap snapshots, it was found that really large (20 MB) strings representing network data were being logged as part of `Systrace.beginEvent` strings from `MessageQueue` in DEV mode. To combat this, use `JSON.stringify` with limits to keep the depth, strings, arrays, and objects in check. Changelog: [Internal] Change `stringifySafe` to have max limits on string size Reviewed By: yungsters Differential Revision: D20016501 fbshipit-source-id: e123016557bc154e4210e0b4df44360570da8016 --- Libraries/BatchedBridge/MessageQueue.js | 2 +- Libraries/Core/ExceptionsManager.js | 2 +- Libraries/StyleSheet/processTransform.js | 2 +- .../Utilities/__tests__/stringifySafe-test.js | 44 +++++- Libraries/Utilities/stringifySafe.js | 129 ++++++++++++++---- 5 files changed, 142 insertions(+), 37 deletions(-) diff --git a/Libraries/BatchedBridge/MessageQueue.js b/Libraries/BatchedBridge/MessageQueue.js index d3802e59ab5..3756b083fbd 100644 --- a/Libraries/BatchedBridge/MessageQueue.js +++ b/Libraries/BatchedBridge/MessageQueue.js @@ -15,7 +15,7 @@ const Systrace = require('../Performance/Systrace'); const deepFreezeAndThrowOnMutationInDev = require('../Utilities/deepFreezeAndThrowOnMutationInDev'); const invariant = require('invariant'); -const stringifySafe = require('../Utilities/stringifySafe'); +const stringifySafe = require('../Utilities/stringifySafe').default; const warnOnce = require('../Utilities/warnOnce'); export type SpyData = { diff --git a/Libraries/Core/ExceptionsManager.js b/Libraries/Core/ExceptionsManager.js index 3b6a5894848..fd6ce28ad6e 100644 --- a/Libraries/Core/ExceptionsManager.js +++ b/Libraries/Core/ExceptionsManager.js @@ -220,7 +220,7 @@ function reactConsoleErrorHandler() { /*reportToConsole*/ false, ); } else { - const stringifySafe = require('../Utilities/stringifySafe'); + const stringifySafe = require('../Utilities/stringifySafe').default; const str = Array.prototype.map .call(arguments, value => typeof value === 'string' ? value : stringifySafe(value), diff --git a/Libraries/StyleSheet/processTransform.js b/Libraries/StyleSheet/processTransform.js index 051c363f3a0..79ed51d9f89 100644 --- a/Libraries/StyleSheet/processTransform.js +++ b/Libraries/StyleSheet/processTransform.js @@ -14,7 +14,7 @@ const MatrixMath = require('../Utilities/MatrixMath'); const Platform = require('../Utilities/Platform'); const invariant = require('invariant'); -const stringifySafe = require('../Utilities/stringifySafe'); +const stringifySafe = require('../Utilities/stringifySafe').default; /** * Generate a transform matrix based on the provided transforms, and use that diff --git a/Libraries/Utilities/__tests__/stringifySafe-test.js b/Libraries/Utilities/__tests__/stringifySafe-test.js index c259caf20a7..9ab85e566cb 100644 --- a/Libraries/Utilities/__tests__/stringifySafe-test.js +++ b/Libraries/Utilities/__tests__/stringifySafe-test.js @@ -5,14 +5,15 @@ * LICENSE file in the root directory of this source tree. * * @format + * @flow strict-local * @emails oncall+react_native */ 'use strict'; -describe('stringifySafe', () => { - const stringifySafe = require('../stringifySafe'); +import stringifySafe, {createStringifySafeWithLimits} from '../stringifySafe'; +describe('stringifySafe', () => { it('stringifySafe stringifies undefined values', () => { expect(stringifySafe(undefined)).toEqual('undefined'); }); @@ -41,9 +42,8 @@ describe('stringifySafe', () => { }); it('stringifySafe stringifies circular objects without toString', () => { - const arg = {}; - arg.arg = arg; - arg.toString = undefined; + const arg = {x: {}, toString: undefined}; + arg.x = arg; const result = stringifySafe(arg); expect(result).toEqual('["object" failed to stringify]'); }); @@ -53,4 +53,38 @@ describe('stringifySafe', () => { const result = stringifySafe(error); expect(result).toEqual('Error: error'); }); + + it('stringifySafe truncates long strings', () => { + const stringify = createStringifySafeWithLimits({maxStringLimit: 3}); + expect(stringify('abcdefghijklmnopqrstuvwxyz')).toEqual( + '"abc...(truncated)..."', + ); + expect(stringify({a: 'abcdefghijklmnopqrstuvwxyz'})).toEqual( + '{"a":"abc...(truncated)..."}', + ); + }); + + it('stringifySafe truncates large arrays', () => { + const stringify = createStringifySafeWithLimits({maxArrayLimit: 3}); + expect(stringify([1, 2, 3, 4, 5])).toEqual( + '[1,2,3,"... extra 2 values truncated ..."]', + ); + expect(stringify({a: [1, 2, 3, 4, 5]})).toEqual( + '{"a":[1,2,3,"... extra 2 values truncated ..."]}', + ); + }); + + it('stringifySafe truncates large objects', () => { + const stringify = createStringifySafeWithLimits({maxObjectKeysLimit: 3}); + expect(stringify({a: 1, b: 2, c: 3, d: 4, e: 5})).toEqual( + '{"a":1,"b":2,"c":3,"...(truncated keys)...":2}', + ); + }); + + it('stringifySafe truncates deep objects', () => { + const stringify = createStringifySafeWithLimits({maxDepth: 3}); + expect(stringify({a: {a: {a: {x: 0, y: 1, z: 2}}}})).toEqual( + '{"a":{"a":{"a":"{ ... object with 3 keys ... }"}}}', + ); + }); }); diff --git a/Libraries/Utilities/stringifySafe.js b/Libraries/Utilities/stringifySafe.js index d562a911e97..effeedcd299 100644 --- a/Libraries/Utilities/stringifySafe.js +++ b/Libraries/Utilities/stringifySafe.js @@ -5,46 +5,117 @@ * LICENSE file in the root directory of this source tree. * * @format - * @flow + * @flow strict-local */ 'use strict'; +import invariant from 'invariant'; + /** * Tries to stringify with JSON.stringify and toString, but catches exceptions * (e.g. from circular objects) and always returns a string and never throws. */ -function stringifySafe(arg: any): string { - let ret; - const type = typeof arg; - if (arg === undefined) { - ret = 'undefined'; - } else if (arg === null) { - ret = 'null'; - } else if (type === 'string') { - ret = '"' + arg + '"'; - } else if (type === 'function') { - try { - ret = arg.toString(); - } catch (e) { - ret = '[function unknown]'; +export function createStringifySafeWithLimits(limits: {| + maxDepth?: number, + maxStringLimit?: number, + maxArrayLimit?: number, + maxObjectKeysLimit?: number, +|}): mixed => string { + const { + maxDepth = Number.POSITIVE_INFINITY, + maxStringLimit = Number.POSITIVE_INFINITY, + maxArrayLimit = Number.POSITIVE_INFINITY, + maxObjectKeysLimit = Number.POSITIVE_INFINITY, + } = limits; + const stack = []; + function replacer(key: string, value: mixed): mixed { + while (stack.length && this !== stack[0]) { + stack.shift(); } - } else if (arg instanceof Error) { - ret = arg.name + ': ' + arg.message; - } else { - // Perform a try catch, just in case the object has a circular - // reference or stringify throws for some other reason. - try { - ret = JSON.stringify(arg); - } catch (e) { - if (typeof arg.toString === 'function') { - try { - ret = arg.toString(); - } catch (E) {} + + if (typeof value === 'string') { + const truncatedString = '...(truncated)...'; + if (value.length > maxStringLimit + truncatedString.length) { + return value.substring(0, maxStringLimit) + truncatedString; + } + return value; + } + if (typeof value !== 'object' || value === null) { + return value; + } + + let retval = value; + if (Array.isArray(value)) { + if (stack.length >= maxDepth) { + retval = `[ ... array with ${value.length} values ... ]`; + } else if (value.length > maxArrayLimit) { + retval = value + .slice(0, maxArrayLimit) + .concat([ + `... extra ${value.length - maxArrayLimit} values truncated ...`, + ]); + } + } else { + // Add refinement after Array.isArray call. + invariant(typeof value === 'object', 'This was already found earlier'); + let keys = Object.keys(value); + if (stack.length >= maxDepth) { + retval = `{ ... object with ${keys.length} keys ... }`; + } else if (keys.length > maxObjectKeysLimit) { + // Return a sample of the keys. + retval = {}; + for (let k of keys.slice(0, maxObjectKeysLimit)) { + retval[k] = value[k]; + } + const truncatedKey = '...(truncated keys)...'; + retval[truncatedKey] = keys.length - maxObjectKeysLimit; } } + stack.unshift(retval); + return retval; } - return ret || '["' + type + '" failed to stringify]'; + + return function stringifySafe(arg: mixed): string { + if (arg === undefined) { + return 'undefined'; + } else if (arg === null) { + return 'null'; + } else if (typeof arg === 'function') { + try { + return arg.toString(); + } catch (e) { + return '[function unknown]'; + } + } else if (arg instanceof Error) { + return arg.name + ': ' + arg.message; + } else { + // Perform a try catch, just in case the object has a circular + // reference or stringify throws for some other reason. + try { + const ret = JSON.stringify(arg, replacer); + if (ret === undefined) { + return '["' + typeof arg + '" failed to stringify]'; + } + return ret; + } catch (e) { + if (typeof arg.toString === 'function') { + try { + // $FlowFixMe: toString shouldn't take any arguments in general. + return arg.toString(); + } catch (E) {} + } + } + } + return '["' + typeof arg + '" failed to stringify]'; + }; } -module.exports = stringifySafe; +const stringifySafe: mixed => string = createStringifySafeWithLimits({ + maxDepth: 10, + maxStringLimit: 100, + maxArrayLimit: 50, + maxObjectKeysLimit: 50, +}); + +export default stringifySafe;