Skip to content

Commit d4bd583

Browse files
authored
Fix prototype keys causing crash in color parsing and invalid colors in color coercion expressions (#1036)
* Fix error thrown when validating certain color strings parseCssColor checks `namedColors[input]` to see if the given color string is a predetermined named color. If "__proto__" or any other key found in Object.prototype was used, the evaluated value was truthy, and the function attempted to destructure the returned value, which throws an error. This commit fixes that issue by using getOwn() to access only own properties of the namedColors object. * Fix color coercion expressions returning array of nulls Color coercion (Coercion class) uses EvaluationContext.parseColor to parse color strings. EvaluationContext has an internal cache for color strings, which it checks before parsing to skip parsing the same value multiple times. If a color coercion expression was passed "__proto__" or some other key present in Object.prototype, the parseColor function would skip parsing altogether as this._parseColorCache["__proto__"] evaluated to a truthy value, and thus parseColor thought the value was cached and returned the evaluated value directly. This caused the entire coercion expression to evaluate to the value of Object.prototype, which in the end resulted in the color being [null, null, null, null] somewhere in maplibre-gl-js. Fixed by creating the cache with a null prototype, instead of an empty object literal. * Update CHANGELOG.md * Group prototype key related fixes in CHANGELOG.md * Use Map instead of an object in EvaluationContext's cache
1 parent 91175b3 commit d4bd583

File tree

9 files changed

+75
-7
lines changed

9 files changed

+75
-7
lines changed

CHANGELOG.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55

66
### 🐞 Bug fixes
77
- Fix RuntimeError class, make it inherited from Error ([#983](https://github.com/maplibre/maplibre-style-spec/issues/983))
8-
- Fix `validate_object` crashing when object prototype keys used in style's objects ([#1028](https://github.com/maplibre/maplibre-style-spec/pull/1028))
9-
- Validate that `layers` array items are objects instead of throwing an error if not ([!1026](https://github.com/maplibre/maplibre-style-spec/pull/1026))
8+
- Validate that `layers` array items are objects instead of throwing an error if not ([#1026](https://github.com/maplibre/maplibre-style-spec/pull/1026))
9+
- Multiple fixes related to validating and parsing of strings containing object prototype keys
10+
- Fix `validate_object` crashing when object prototype keys used in style's objects ([#1028](https://github.com/maplibre/maplibre-style-spec/pull/1028))
11+
- Fix `validate_color` crashing when object prototype keys used as color strings ([#1036](https://github.com/maplibre/maplibre-style-spec/pull/1036))
12+
- Fix color coercion in expressions (e.g. `to_color`) producing invalid colors when object prototype keys used as color strings ([#1036](https://github.com/maplibre/maplibre-style-spec/pull/1036))
1013

1114
## 23.1.0
1215

src/expression/evaluation_context.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,14 @@ export class EvaluationContext {
1313
availableImages: Array<string>;
1414
canonical: ICanonicalTileID;
1515

16-
_parseColorCache: {[_: string]: Color};
16+
_parseColorCache: Map<string, Color>;
1717

1818
constructor() {
1919
this.globals = null;
2020
this.feature = null;
2121
this.featureState = null;
2222
this.formattedSection = null;
23-
this._parseColorCache = {};
23+
this._parseColorCache = new Map<string, Color>();
2424
this.availableImages = null;
2525
this.canonical = null;
2626
}
@@ -46,9 +46,10 @@ export class EvaluationContext {
4646
}
4747

4848
parseColor(input: string): Color {
49-
let cached = this._parseColorCache[input];
49+
let cached = this._parseColorCache.get(input);
5050
if (!cached) {
51-
cached = this._parseColorCache[input] = Color.parse(input);
51+
cached = Color.parse(input);
52+
this._parseColorCache.set(input, cached);
5253
}
5354
return cached;
5455
}

src/expression/types/parse_css_color.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ describe('parseCssColor', () => {
3131
expect(parse('aqua-marine')).toBeUndefined();
3232
expect(parse('aqua_marine')).toBeUndefined();
3333
expect(parse('aqua marine')).toBeUndefined();
34+
expect(parse('__proto__')).toBeUndefined();
35+
expect(parse('valueOf')).toBeUndefined();
3436
});
3537

3638
});

src/expression/types/parse_css_color.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import {getOwn} from '../../util/get_own';
12
import {HSLColor, hslToRgb, RGBColor} from './color_spaces';
23

34
/**
@@ -37,7 +38,7 @@ export function parseCssColor(input: string): RGBColor | undefined {
3738
}
3839

3940
// 'white', 'black', 'blue'
40-
const namedColorsMatch = namedColors[input];
41+
const namedColorsMatch = getOwn(namedColors, input);
4142
if (namedColorsMatch) {
4243
const [r, g, b] = namedColorsMatch;
4344
return [r / 255, g / 255, b / 255, 1];

test/integration/expression/tests/to-color/basic/test.json

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,14 @@
2323
}
2424
}
2525
],
26+
[
27+
{},
28+
{
29+
"properties": {
30+
"x": "__proto__"
31+
}
32+
}
33+
],
2634
[
2735
{},
2836
{
@@ -85,6 +93,9 @@
8593
{
8694
"error": "Could not parse color from value 'invalid'"
8795
},
96+
{
97+
"error": "Could not parse color from value '__proto__'"
98+
},
8899
[
89100
0,
90101
1,

test/integration/style-spec/tests/bad-color.input.json

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,22 @@
2525
"paint": {
2626
"fill-outline-color": ["darken", 10, "#FF0000"]
2727
}
28+
},
29+
{
30+
"id": "prototype",
31+
"type": "fill",
32+
"source": "vector",
33+
"source-layer": "layer",
34+
"paint": {
35+
"fill-color": "__proto__",
36+
"fill-outline-color": {
37+
"property": "fill",
38+
"stops": [
39+
[{ "zoom": 10, "value": 10 }, "valueOf"],
40+
[{ "zoom": 11, "value": 20 }, "__proto__"]
41+
]
42+
}
43+
}
2844
}
2945
]
3046
}

test/integration/style-spec/tests/bad-color.output.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,17 @@
1010
{
1111
"message": "layers[1].paint.fill-outline-color: color expected, array found",
1212
"line": 26
13+
},
14+
{
15+
"message": "layers[2].paint.fill-color: color expected, \"__proto__\" found",
16+
"line": 35
17+
},
18+
{
19+
"message": "layers[2].paint.fill-outline-color.stops[0][1]: color expected, \"valueOf\" found",
20+
"line": 39
21+
},
22+
{
23+
"message": "layers[2].paint.fill-outline-color.stops[1][1]: color expected, \"__proto__\" found",
24+
"line": 40
1325
}
1426
]
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"version": 8,
3+
"sources": {
4+
"vector": {
5+
"type": "vector",
6+
"url": "https://demotiles.maplibre.org/tiles/tiles.json"
7+
}
8+
},
9+
"light": {
10+
"anchor": "map",
11+
"position": [1, 90, 90],
12+
"color": "__proto__",
13+
"intensity": 0.75
14+
},
15+
"layers": []
16+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
[
2+
{
3+
"message": "color: color expected, \"__proto__\" found",
4+
"line": 12
5+
}
6+
]

0 commit comments

Comments
 (0)