From 55d6daf8ee5e9f0de21ed8f49d842b67737a9143 Mon Sep 17 00:00:00 2001 From: Lucian Buzzo Date: Sun, 11 Mar 2018 21:39:27 +0000 Subject: [PATCH] Do not mutate options object used in constructor Connects to #1322 change-type: patch --- src/Terminal.test.ts | 18 ++++-- src/Terminal.ts | 3 +- src/utils/Clone.test.ts | 137 ++++++++++++++++++++++++++++++++++++++++ src/utils/Clone.ts | 28 ++++++++ 4 files changed, 181 insertions(+), 5 deletions(-) create mode 100644 src/utils/Clone.test.ts create mode 100644 src/utils/Clone.ts diff --git a/src/Terminal.test.ts b/src/Terminal.test.ts index d47fbdaf..5a604379 100644 --- a/src/Terminal.test.ts +++ b/src/Terminal.test.ts @@ -20,12 +20,13 @@ class TestTerminal extends Terminal { describe('term.js addons', () => { let term: TestTerminal; + const termOptions = { + cols: INIT_COLS, + rows: INIT_ROWS + }; beforeEach(() => { - term = new TestTerminal({ - cols: INIT_COLS, - rows: INIT_ROWS - }); + term = new TestTerminal(termOptions); term.refresh = () => {}; (term).renderer = new MockRenderer(); term.viewport = new MockViewport(); @@ -43,6 +44,15 @@ describe('term.js addons', () => { }; }); + it('should not mutate the options parameter', () => { + term.setOption('cols', 1000); + + assert.deepEqual(termOptions, { + cols: INIT_COLS, + rows: INIT_ROWS + }); + }); + it('should apply addons with Terminal.applyAddon', () => { Terminal.applyAddon(attach); // Test that addon was applied successfully, adding attach to Terminal's diff --git a/src/Terminal.ts b/src/Terminal.ts index a5841c9d..d1a266b2 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -40,6 +40,7 @@ import { CharMeasure } from './utils/CharMeasure'; import * as Browser from './shared/utils/Browser'; import * as Strings from './Strings'; import { MouseHelper } from './utils/MouseHelper'; +import { clone } from './utils/Clone'; import { DEFAULT_BELL_SOUND, SoundManager } from './SoundManager'; import { DEFAULT_ANSI_COLORS } from './renderer/ColorManager'; import { MouseZoneManager } from './input/MouseZoneManager'; @@ -245,7 +246,7 @@ export class Terminal extends EventEmitter implements ITerminal, IInputHandlingT options: ITerminalOptions = {} ) { super(); - this.options = options; + this.options = clone(options); this._setup(); } diff --git a/src/utils/Clone.test.ts b/src/utils/Clone.test.ts new file mode 100644 index 00000000..f5708a9e --- /dev/null +++ b/src/utils/Clone.test.ts @@ -0,0 +1,137 @@ +/** + * Copyright (c) 2016 The xterm.js authors. All rights reserved. + * @license MIT + */ + +import { assert, expect } from 'chai'; +import { clone } from './Clone'; + +describe('clone', () => { + it('should clone simple objects', () => { + const test = { + a: 1, + b: 2 + }; + + assert.deepEqual(clone(test), { a: 1, b: 2 }); + }); + + it('should clone nested objects', () => { + const test = { + bar: { + a: 1, + b: 2, + c: { + foo: 'bar' + } + } + }; + + assert.deepEqual(clone(test), { + bar: { + a: 1, + b: 2, + c: { + foo: 'bar' + } + } + }); + }); + + it('should clone null values', () => { + const test = { + a: null + }; + + assert.deepEqual(clone(test), { a: null }); + }); + + it('should clone array values', () => { + const test = { + a: [1, 2, 3], + b: [1, null, 'test', { foo: 'bar' }] + }; + + assert.deepEqual(clone(test), { + a: [1, 2, 3], + b: [1, null, 'test', { foo: 'bar' }] + }); + }); + + it('should stop mutation from occuring on the original object', () => { + const test = { + a: 1, + b: 2, + c: { + foo: 'bar' + } + }; + + const cloned = clone(test); + + test.a = 5; + test.c.foo = 'barbaz'; + + assert.deepEqual(cloned, { + a: 1, + b: 2, + c: { + foo: 'bar' + } + }); + }); + + it('should clone to a maximum depth of 5 by default', () => { + const test = { + a: { + b: { + c: { + d: { + e: { + f: 'foo' + } + } + } + } + } + }; + + const cloned = clone(test); + + test.a.b.c.d.e.f = 'bar'; + + // The values at a greater depth then 5 should not be cloned + assert.equal(cloned.a.b.c.d.e.f, 'bar'); + }); + + it('should allow an optional maximum depth to be set', () => { + const test = { + a: { + b: { + c: 'foo' + } + } + }; + + const cloned = clone(test, 2); + + test.a.b.c = 'bar'; + + // The values at a greater depth then 2 should not be cloned + assert.equal(cloned.a.b.c, 'bar'); + }); + + it('should not throw when cloning a recursive reference', () => { + const test = { + a: { + b: { + c: {} + } + } + }; + + test.a.b.c = test; + + expect(() => clone(test)).to.not.throw(); + }); +}); diff --git a/src/utils/Clone.ts b/src/utils/Clone.ts new file mode 100644 index 00000000..b09c0258 --- /dev/null +++ b/src/utils/Clone.ts @@ -0,0 +1,28 @@ +/** + * Copyright (c) 2016 The xterm.js authors. All rights reserved. + * @license MIT + */ + +/* + * A simple utility for cloning values + */ +export const clone = (val: T, depth: number = 5): T => { + if (typeof val !== 'object') { + return val; + } + + // cloning null always returns null + if (val === null) { + return null; + } + + // If we're cloning an array, use an array as the base, otherwise use an object + const clonedObject: any = Array.isArray(val) ? [] : {}; + + for (const key in val) { + // Recursively clone eack item unless we're at the maximum depth + clonedObject[key] = depth <= 1 ? val[key] : clone(val[key], depth - 1); + } + + return clonedObject as T; +};