Number validation should fail on NaN (#8615)

* Number validation should fail on NaN * Avoid Number.isNaN * Add runtime checks * Add render and unit test * Remove not useful unit test * Revert debug page change
mapbox · Nov 25, 2019 · ce520ff · ce520ff
1 parent efc0caa
commit ce520ff
Show file tree

Hide file tree

Showing 7 changed files with 185 additions and 2 deletions.
diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js
@@ -78,7 +78,8 @@ export class StyleExpression {
 
         try {
             const val = this.expression.evaluate(this._evaluator);
-            if (val === null || val === undefined) {
+            // eslint-disable-next-line no-self-compare
+            if (val === null || val === undefined || (typeof val === 'number' && val !== val)) {
                 return this._defaultValue;
             }
             if (this._enumValues && !(val in this._enumValues)) {

diff --git a/src/style-spec/validate/validate_number.js b/src/style-spec/validate/validate_number.js
@@ -6,7 +6,12 @@ export default function validateNumber(options) {
     const key = options.key;
     const value = options.value;
     const valueSpec = options.valueSpec;
-    const type = getType(value);
+    let type = getType(value);
+
+    // eslint-disable-next-line no-self-compare
+    if (type === 'number' && value !== value) {
+        type = 'NaN';
+    }
 
     if (type !== 'number') {
         return [new ValidationError(key, value, `number expected, ${type} found`)];

diff --git a/test/integration/render-tests/text-size/nan/expected.png b/test/integration/render-tests/text-size/nan/expected.png
diff --git a/test/integration/render-tests/text-size/nan/style.json b/test/integration/render-tests/text-size/nan/style.json
@@ -0,0 +1,37 @@
+{
+  "version": 8,
+  "metadata": {
+    "test": {
+      "width": 64,
+      "height": 64
+    }
+  },
+  "sources": {
+    "geojson": {
+      "type": "geojson",
+      "data": {
+        "type": "Point",
+        "coordinates": [
+          0,
+          0
+        ]
+      }
+    }
+  },
+  "glyphs": "local://glyphs/{fontstack}/{range}.pbf",
+  "layers": [
+    {
+      "id": "symbol",
+      "type": "symbol",
+      "source": "geojson",
+      "layout": {
+        "text-size": ["sqrt", -1],
+        "text-field": "ABC",
+        "text-font": [
+          "Open Sans Semibold",
+          "Arial Unicode MS Bold"
+        ]
+      }
+    }
+  ]
+}
diff --git a/test/unit/style-spec/fixture/numbers.input.json b/test/unit/style-spec/fixture/numbers.input.json
@@ -0,0 +1,94 @@
+{
+  "version": 8,
+  "sources": {
+    "point": {
+      "type": "geojson",
+      "data": {
+        "type": "FeatureCollection",
+        "features": [
+          {
+            "type": "Feature",
+            "geometry": {
+              "type": "Point",
+              "coordinates": [ 0, 0 ]
+            }
+          }
+        ]
+      }
+    }
+  },
+  "layers": [
+    {
+      "id": "valid",
+      "type": "circle",
+      "source": "point",
+      "paint": {
+        "circle-radius": 5
+      }
+    },
+    {
+      "id": "zero",
+      "type": "circle",
+      "source": "point",
+      "paint": {
+        "circle-radius": 0
+      }
+    },
+    {
+      "id": "less-than-zero",
+      "type": "circle",
+      "source": "point",
+      "paint": {
+        "circle-radius": -1
+      }
+    },
+    {
+      "id": "null-not-number",
+      "type": "circle",
+      "source": "point",
+      "paint": {
+        "circle-radius": null
+      }
+    },
+    {
+      "id": "object-not-number",
+      "type": "circle",
+      "source": "point",
+      "paint": {
+        "circle-radius": {}
+      }
+    },
+    {
+      "id": "array-not-number",
+      "type": "circle",
+      "source": "point",
+      "paint": {
+        "circle-radius": []
+      }
+    },
+    {
+      "id": "boolean-not-number",
+      "type": "circle",
+      "source": "point",
+      "paint": {
+        "circle-radius": true
+      }
+    },
+    {
+      "id": "expression",
+      "type": "circle",
+      "source": "point",
+      "paint": {
+        "circle-radius": ["sqrt", 16]
+      }
+    },
+    {
+      "id": "expression-invalid",
+      "type": "circle",
+      "source": "point",
+      "paint": {
+        "circle-radius": ["/", 0, 0]
+      }
+    }
+  ]
+}
diff --git a/test/unit/style-spec/fixture/numbers.output-api-supported.json b/test/unit/style-spec/fixture/numbers.output-api-supported.json
@@ -0,0 +1,25 @@
+[
+    {
+      "line": 42,
+      "message": "layers[2].paint.circle-radius: -1 is less than the minimum value 0"
+    },
+    {
+      "message": "layers[3].paint.circle-radius: number expected, null found"
+    },
+    {
+      "line": 58,
+      "message": "layers[4].paint.circle-radius: missing required property \"stops\""
+    },
+    {
+      "line": 66,
+      "message": "layers[5].paint.circle-radius: number expected, array found"
+    },
+    {
+      "line": 74,
+      "message": "layers[6].paint.circle-radius: number expected, boolean found"
+    },
+    {
+      "line": 6,
+      "message": "source.data: Unsupported property \"data\""
+    }
+  ]
diff --git a/test/unit/style-spec/fixture/numbers.output.json b/test/unit/style-spec/fixture/numbers.output.json
@@ -0,0 +1,21 @@
+[
+    {
+        "message": "layers[2].paint.circle-radius: -1 is less than the minimum value 0",
+        "line": 42
+    },
+    {
+        "message": "layers[3].paint.circle-radius: number expected, null found"
+    },
+    {
+        "message": "layers[4].paint.circle-radius: missing required property \"stops\"",
+        "line": 58
+    },
+    {
+        "message": "layers[5].paint.circle-radius: number expected, array found",
+        "line": 66
+    },
+    {
+        "message": "layers[6].paint.circle-radius: number expected, boolean found",
+        "line": 74
+    }
+]