From e84ed7e378948708917c9a4d0e2b5aa3c4ce5576 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 16 Sep 2022 12:09:27 +0300 Subject: [PATCH 01/12] fix(): extraParam should not be passed to options --- src/shapes/object.class.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/shapes/object.class.ts b/src/shapes/object.class.ts index 6978d5ab7ee..1fc52de562b 100644 --- a/src/shapes/object.class.ts +++ b/src/shapes/object.class.ts @@ -6,6 +6,7 @@ import { Point } from '../point.class'; import { capValue } from '../util/misc/capValue'; import { pick } from '../util/misc/pick'; import { runningAnimations } from '../util/animation_registry'; +import { enlivenObjectEnlivables } from '../util/misc/objectEnlive'; (function (global) { var fabric = global.fabric || (global.fabric = {}), @@ -2053,15 +2054,13 @@ import { runningAnimations } from '../util/animation_registry'; * @param {AbortSignal} [options.signal] handle aborting, see https://developer.mozilla.org/en-US/docs/Web/API/AbortController/signal * @returns {Promise} */ - fabric.Object._fromObject = function (klass, object, options) { - var serializedObject = clone(object, true); - return fabric.util - .enlivenObjectEnlivables(serializedObject, options) - .then(function (enlivedMap) { - var newObject = Object.assign(object, enlivedMap); - return options && options.extraParam - ? new klass(object[options.extraParam], newObject) - : new klass(newObject); + fabric.Object._fromObject = function (klass, object, { extraParam, ...options } = {}) { + return enlivenObjectEnlivables(clone(object, true), options) + .then((enlivedMap) => { + const { [extraParam]: arg0, ...rest } = { ...options, ...enlivedMap }; + return extraParam + ? new klass(arg0, rest) + : new klass(rest); }); }; From 54f6fe8a583cf43265a75862d07ac96963a25273 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 16 Sep 2022 12:26:46 +0300 Subject: [PATCH 02/12] prettify --- src/shapes/object.class.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/shapes/object.class.ts b/src/shapes/object.class.ts index 1fc52de562b..af7597a046b 100644 --- a/src/shapes/object.class.ts +++ b/src/shapes/object.class.ts @@ -2054,14 +2054,17 @@ import { enlivenObjectEnlivables } from '../util/misc/objectEnlive'; * @param {AbortSignal} [options.signal] handle aborting, see https://developer.mozilla.org/en-US/docs/Web/API/AbortController/signal * @returns {Promise} */ - fabric.Object._fromObject = function (klass, object, { extraParam, ...options } = {}) { - return enlivenObjectEnlivables(clone(object, true), options) - .then((enlivedMap) => { + fabric.Object._fromObject = function ( + klass, + object, + { extraParam, ...options } = {} + ) { + return enlivenObjectEnlivables(clone(object, true), options).then( + (enlivedMap) => { const { [extraParam]: arg0, ...rest } = { ...options, ...enlivedMap }; - return extraParam - ? new klass(arg0, rest) - : new klass(rest); - }); + return extraParam ? new klass(arg0, rest) : new klass(rest); + } + ); }; /** From 4fcfcab5e47d40a355e9daf8fb354630c5d0c002 Mon Sep 17 00:00:00 2001 From: Shachar <34343793+ShaMan123@users.noreply.github.com> Date: Fri, 16 Sep 2022 12:53:57 +0300 Subject: [PATCH 03/12] add comment Co-authored-by: Andrea Bogazzi --- src/shapes/object.class.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/shapes/object.class.ts b/src/shapes/object.class.ts index af7597a046b..d8098b9e1f8 100644 --- a/src/shapes/object.class.ts +++ b/src/shapes/object.class.ts @@ -2061,6 +2061,8 @@ import { enlivenObjectEnlivables } from '../util/misc/objectEnlive'; ) { return enlivenObjectEnlivables(clone(object, true), options).then( (enlivedMap) => { + // from the resulting enlived options, extract options.extraParam in arg0 + // to avoid accidental ovverrides later const { [extraParam]: arg0, ...rest } = { ...options, ...enlivedMap }; return extraParam ? new klass(arg0, rest) : new klass(rest); } From 8a10ff33dc27a8f7a73e2509c22396a571d6e7cb Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 16 Sep 2022 12:57:50 +0300 Subject: [PATCH 04/12] Update object.class.ts --- src/shapes/object.class.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shapes/object.class.ts b/src/shapes/object.class.ts index d8098b9e1f8..00dcb1b58f3 100644 --- a/src/shapes/object.class.ts +++ b/src/shapes/object.class.ts @@ -2061,8 +2061,8 @@ import { enlivenObjectEnlivables } from '../util/misc/objectEnlive'; ) { return enlivenObjectEnlivables(clone(object, true), options).then( (enlivedMap) => { - // from the resulting enlived options, extract options.extraParam in arg0 - // to avoid accidental ovverrides later + // from the resulting enlived options, extract options.extraParam to arg0 + // to avoid accidental overrides later const { [extraParam]: arg0, ...rest } = { ...options, ...enlivedMap }; return extraParam ? new klass(arg0, rest) : new klass(rest); } From db4512ad416d09f71a7aef0535b5fccf0881c087 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 16 Sep 2022 13:17:35 +0300 Subject: [PATCH 05/12] fix(): safegurad text intialization --- src/shapes/text.class.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/shapes/text.class.ts b/src/shapes/text.class.ts index e945dd180e1..48c23ff83e4 100644 --- a/src/shapes/text.class.ts +++ b/src/shapes/text.class.ts @@ -366,9 +366,8 @@ import { DEFAULT_SVG_FONT_SIZE } from '../constants'; */ initialize: function (text, options) { this.styles = options ? options.styles || {} : {}; - this.text = text; this.__skipDimension = true; - this.callSuper('initialize', options); + this.callSuper('initialize', { text, ...options }); if (this.path) { this.setPathInfo(); } From 50b096ff7a8ae478378fd9cb9a190e6323ddc456 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 16 Sep 2022 13:23:01 +0300 Subject: [PATCH 06/12] Update text.class.ts --- src/shapes/text.class.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/shapes/text.class.ts b/src/shapes/text.class.ts index 48c23ff83e4..19914c0fbe1 100644 --- a/src/shapes/text.class.ts +++ b/src/shapes/text.class.ts @@ -364,10 +364,9 @@ import { DEFAULT_SVG_FONT_SIZE } from '../constants'; * @param {Object} [options] Options object * @return {fabric.Text} thisArg */ - initialize: function (text, options) { - this.styles = options ? options.styles || {} : {}; + initialize: function (text, { styles = {}, text: _, ...options } = {}) { this.__skipDimension = true; - this.callSuper('initialize', { text, ...options }); + this.callSuper('initialize', { text, styles, ...options }); if (this.path) { this.setPathInfo(); } From d3c2ddf935c1fb04c3e950979757d14c4df1edc1 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 16 Sep 2022 13:23:39 +0300 Subject: [PATCH 07/12] Update text.js --- test/unit/text.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unit/text.js b/test/unit/text.js index 779621fd248..5af34bc88af 100644 --- a/test/unit/text.js +++ b/test/unit/text.js @@ -70,6 +70,8 @@ assert.equal(text.get('type'), 'text'); assert.equal(text.get('text'), 'x'); + + assert.ok(new fabric.Text('a', { text: 'b' }).text, 'a', 'text arg is safeguraded'); }); QUnit.test('toString', function(assert) { From a6a15b719f4274a381b09e35d80ae9bd19495728 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 16 Sep 2022 14:03:14 +0300 Subject: [PATCH 08/12] fix(): safegurad `extraParam` in intialization --- src/shapes/path.class.ts | 4 +--- src/shapes/polyline.class.ts | 3 +-- test/unit/line.js | 3 +++ test/unit/path.js | 3 ++- test/unit/polyline.js | 2 ++ test/unit/text.js | 2 +- 6 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/shapes/path.class.ts b/src/shapes/path.class.ts index e53ce716835..896618e5e45 100644 --- a/src/shapes/path.class.ts +++ b/src/shapes/path.class.ts @@ -45,9 +45,7 @@ import { config } from '../config'; * @param {Object} [options] Options object * @return {fabric.Path} thisArg */ - initialize: function (path, options) { - options = clone(options || {}); - delete options.path; + initialize: function (path, { path: _, ...options } = {}) { this.callSuper('initialize', options); this._setPath(path || [], options); }, diff --git a/src/shapes/polyline.class.ts b/src/shapes/polyline.class.ts index 8d751eff5c0..9836d40b550 100644 --- a/src/shapes/polyline.class.ts +++ b/src/shapes/polyline.class.ts @@ -65,8 +65,7 @@ import { makeBoundingBoxFromPoints } from '../util/misc/boundingBoxFromPoints'; * top: 100 * }); */ - initialize: function (points, options) { - options = options || {}; + initialize: function (points, { points: _, ...options } = {}) { this.points = points || []; this.callSuper('initialize', options); this._setPositionDimensions(options); diff --git a/test/unit/line.js b/test/unit/line.js index 2dfffa47e2a..977523eda02 100644 --- a/test/unit/line.js +++ b/test/unit/line.js @@ -60,6 +60,9 @@ assert.equal(lineWithoutPoints.get('y1'), 0); assert.equal(lineWithoutPoints.get('x2'), 0); assert.equal(lineWithoutPoints.get('y2'), 0); + + const safeguradedLine = new fabric.Line(undefined, { x1: 1 }); + assert.equal(safeguradedLine.get('x1'), 0, 'points should be safeguraded'); }); QUnit.test('complexity', function(assert) { diff --git a/test/unit/path.js b/test/unit/path.js index 3d479f71afb..224f1f2970b 100644 --- a/test/unit/path.js +++ b/test/unit/path.js @@ -95,10 +95,11 @@ QUnit.test('initialize', function(assert) { var done = assert.async(); - var path = new fabric.Path('M 100 100 L 200 100 L 170 200 z', { top: 0, strokeWidth: 0 }); + var path = new fabric.Path('M 100 100 L 200 100 L 170 200 z', { top: 0, strokeWidth: 0, path: 'M 0 0' }); assert.equal(path.left, 100); assert.equal(path.top, 0); + assert.deepEqual(path.path, fabric.util.parsePath('M 100 100 L 200 100 L 170 200 z'), 'path arg should be safeguraded'); done(); }); diff --git a/test/unit/polyline.js b/test/unit/polyline.js index 118fe2e56f1..3fd00603a10 100644 --- a/test/unit/polyline.js +++ b/test/unit/polyline.js @@ -62,6 +62,8 @@ assert.equal(polyline.type, 'polyline'); assert.deepEqual(polyline.get('points'), [{ x: 10, y: 12 }, { x: 20, y: 22 }]); + + assert.deepEqual(new fabric.Polyline(getPoints(), { points: [] }).points, getPoints(), 'points arg should be safeguraded'); }); QUnit.test('complexity', function(assert) { diff --git a/test/unit/text.js b/test/unit/text.js index 5af34bc88af..17377b0266b 100644 --- a/test/unit/text.js +++ b/test/unit/text.js @@ -71,7 +71,7 @@ assert.equal(text.get('type'), 'text'); assert.equal(text.get('text'), 'x'); - assert.ok(new fabric.Text('a', { text: 'b' }).text, 'a', 'text arg is safeguraded'); + assert.ok(new fabric.Text('a', { text: 'b' }).text, 'a', 'text arg should be safeguraded'); }); QUnit.test('toString', function(assert) { From 1fade42893c701e661434c56b6ec612db9196735 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 16 Sep 2022 14:06:14 +0300 Subject: [PATCH 09/12] Update polyline.class.ts --- src/shapes/polyline.class.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/shapes/polyline.class.ts b/src/shapes/polyline.class.ts index 9836d40b550..8752b5ef16f 100644 --- a/src/shapes/polyline.class.ts +++ b/src/shapes/polyline.class.ts @@ -66,8 +66,7 @@ import { makeBoundingBoxFromPoints } from '../util/misc/boundingBoxFromPoints'; * }); */ initialize: function (points, { points: _, ...options } = {}) { - this.points = points || []; - this.callSuper('initialize', options); + this.callSuper('initialize', { points: points || [], ...options }); this._setPositionDimensions(options); }, @@ -79,7 +78,6 @@ import { makeBoundingBoxFromPoints } from '../util/misc/boundingBoxFromPoints'; }, _setPositionDimensions: function (options) { - options || (options = {}); var calcDim = this._calcDimensions(options), correctLeftTop, correctSize = this.exactBoundingBox ? this.strokeWidth : 0; From 0754351c7b0746e43aba2fbc550782bbd9d7a555 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Fri, 16 Sep 2022 14:06:29 +0300 Subject: [PATCH 10/12] Update polyline.class.ts --- src/shapes/polyline.class.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shapes/polyline.class.ts b/src/shapes/polyline.class.ts index 8752b5ef16f..ac70f4988dd 100644 --- a/src/shapes/polyline.class.ts +++ b/src/shapes/polyline.class.ts @@ -77,7 +77,7 @@ import { makeBoundingBoxFromPoints } from '../util/misc/boundingBoxFromPoints'; return projectStrokeOnPoints(this.points, this, true); }, - _setPositionDimensions: function (options) { + _setPositionDimensions: function (options = {}) { var calcDim = this._calcDimensions(options), correctLeftTop, correctSize = this.exactBoundingBox ? this.strokeWidth : 0; From e48d73974d75ca5ffad9d1b467e6d26d2b362bf8 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Wed, 14 Dec 2022 08:41:35 +0200 Subject: [PATCH 11/12] Update polyline.class.ts --- src/shapes/polyline.class.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/shapes/polyline.class.ts b/src/shapes/polyline.class.ts index 29fc10ad934..75af09a2923 100644 --- a/src/shapes/polyline.class.ts +++ b/src/shapes/polyline.class.ts @@ -95,7 +95,10 @@ export class Polyline extends FabricObject { * top: 100 * }); */ - constructor(points: IPoint[] = [], { left, top, ...options }: any = {}) { + constructor( + points: IPoint[] = [], + { points: _, left, top, ...options }: any = {} + ) { super({ points, ...options }); this.initialized = true; this.setBoundingBox(true); From 47401efe3660ffee584119fd76ddc0a2b5877fc4 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Wed, 14 Dec 2022 08:46:44 +0200 Subject: [PATCH 12/12] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 707de7eddd7..19ea7c48cc7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [next] +- chore(): safeguard `extraParam` in initialization [#8296](https://github.com/fabricjs/fabric.js/pull/8296) - chore(TS): polish text [#8489](https://github.com/fabricjs/fabric.js/pull/8489) - chore(TS): fix import cycle, extract `groupSVGElements` [#8506](https://github.com/fabricjs/fabric.js/pull/8506) - chore(TS): permissive `Point` typings [#8434](https://github.com/fabricjs/fabric.js/pull/8434)