Clone property values on input and output

This allows programs to mutate them internally -- a questionable practice, but supported by previous versions.
mapbox · Dec 5, 2017 · b860950 · b860950
1 parent 84316a7
commit b860950
Show file tree

Hide file tree

Showing 2 changed files with 151 additions and 7 deletions.
diff --git a/src/style/properties.js b/src/style/properties.js
@@ -1,7 +1,7 @@
 // @flow
 
 const assert = require('assert');
-const {extend, easeCubicInOut} = require('../util/util');
+const {clone, extend, easeCubicInOut} = require('../util/util');
 const interpolate = require('../style-spec/util/interpolate');
 const {normalizePropertyExpression} = require('../style-spec/expression');
 const Color = require('../style-spec/util/color');
@@ -167,7 +167,7 @@ class Transitionable<Props: Object> {
     }
 
     getValue<S: string, T>(name: S): PropertyValueSpecification<T> | void {
-        return this._values[name].value.value;
+        return clone(this._values[name].value.value);
     }
 
     setValue<S: string, T>(name: S, value: PropertyValueSpecification<T> | void) {
@@ -176,18 +176,18 @@ class Transitionable<Props: Object> {
         }
         // Note that we do not _remove_ an own property in the case where a value is being reset
         // to the default: the transition might still be non-default.
-        this._values[name].value = new PropertyValue(this._values[name].property, value === null ? undefined : value);
+        this._values[name].value = new PropertyValue(this._values[name].property, value === null ? undefined : clone(value));
     }
 
     getTransition<S: string>(name: S): TransitionSpecification | void {
-        return this._values[name].transition;
+        return clone(this._values[name].transition);
     }
 
     setTransition<S: string>(name: S, value: TransitionSpecification | void) {
         if (!this._values.hasOwnProperty(name)) {
             this._values[name] = new TransitionablePropertyValue(this._values[name].property);
         }
-        this._values[name].transition = value || undefined;
+        this._values[name].transition = clone(value) || undefined;
     }
 
     serialize() {
@@ -358,11 +358,11 @@ class Layout<Props: Object> {
     }
 
     getValue<S: string>(name: S) {
-        return this._values[name].value;
+        return clone(this._values[name].value);
     }
 
     setValue<S: string>(name: S, value: *) {
-        this._values[name] = new PropertyValue(this._values[name].property, value === null ? undefined : value);
+        this._values[name] = new PropertyValue(this._values[name].property, value === null ? undefined : clone(value));
     }
 
     serialize() {

diff --git a/test/unit/style/style.test.js b/test/unit/style/style.test.js
@@ -1313,6 +1313,150 @@ test('Style#setPaintProperty', (t) => {
         });
     });
 
+    t.test('#5802 clones the input', (t) => {
+        const style = new Style(new StubMap());
+        style.loadJSON({
+            "version": 8,
+            "sources": {},
+            "layers": [
+                {
+                    "id": "background",
+                    "type": "background"
+                }
+            ]
+        });
+
+        style.on('style.load', () => {
+            const value = {stops: [[0, 'red'], [10, 'blue']]};
+            style.setPaintProperty('background', 'background-color', value);
+            t.notEqual(style.getPaintProperty('background', 'background-color'), value);
+            t.ok(style._changed);
+
+            style.update({});
+            t.notOk(style._changed);
+
+            value.stops[0][0] = 1;
+            style.setPaintProperty('background', 'background-color', value);
+            t.ok(style._changed);
+
+            t.end();
+        });
+    });
+
+    t.end();
+});
+
+test('Style#getPaintProperty', (t) => {
+    t.test('#5802 clones the output', (t) => {
+        const style = new Style(new StubMap());
+        style.loadJSON({
+            "version": 8,
+            "sources": {},
+            "layers": [
+                {
+                    "id": "background",
+                    "type": "background"
+                }
+            ]
+        });
+
+        style.on('style.load', () => {
+            style.setPaintProperty('background', 'background-color', {stops: [[0, 'red'], [10, 'blue']]});
+            style.update({});
+            t.notOk(style._changed);
+
+            const value = style.getPaintProperty('background', 'background-color');
+            value.stops[0][0] = 1;
+            style.setPaintProperty('background', 'background-color', value);
+            t.ok(style._changed);
+
+            t.end();
+        });
+    });
+
+    t.end();
+});
+
+test('Style#setLayoutProperty', (t) => {
+    t.test('#5802 clones the input', (t) => {
+        const style = new Style(new StubMap());
+        style.loadJSON({
+            "version": 8,
+            "sources": {
+                "geojson": {
+                    "type": "geojson",
+                    "data": {
+                        "type": "FeatureCollection",
+                        "features": []
+                    }
+                }
+            },
+            "layers": [
+                {
+                    "id": "line",
+                    "type": "line",
+                    "source": "geojson"
+                }
+            ]
+        });
+
+        style.on('style.load', () => {
+            const value = {stops: [[0, 'butt'], [10, 'round']]};
+            style.setLayoutProperty('line', 'line-cap', value);
+            t.notEqual(style.getLayoutProperty('line', 'line-cap'), value);
+            t.ok(style._changed);
+
+            style.update({});
+            t.notOk(style._changed);
+
+            value.stops[0][0] = 1;
+            style.setLayoutProperty('line', 'line-cap', value);
+            t.ok(style._changed);
+
+            t.end();
+        });
+    });
+
+    t.end();
+});
+
+test('Style#getLayoutProperty', (t) => {
+    t.test('#5802 clones the output', (t) => {
+        const style = new Style(new StubMap());
+        style.loadJSON({
+            "version": 8,
+            "sources": {
+                "geojson": {
+                    "type": "geojson",
+                    "data": {
+                        "type": "FeatureCollection",
+                        "features": []
+                    }
+                }
+            },
+            "layers": [
+                {
+                    "id": "line",
+                    "type": "line",
+                    "source": "geojson"
+                }
+            ]
+        });
+
+        style.on('style.load', () => {
+            style.setLayoutProperty('line', 'line-cap', {stops: [[0, 'butt'], [10, 'round']]});
+            style.update({});
+            t.notOk(style._changed);
+
+            const value = style.getLayoutProperty('line', 'line-cap');
+            value.stops[0][0] = 1;
+            style.setLayoutProperty('line', 'line-cap', value);
+            t.ok(style._changed);
+
+            t.end();
+        });
+    });
+
     t.end();
 });