Guard against non-numeric id in GeoJSON features (#4581)

* Add failing regression test for #4494 * Guard against non-numeric id in GeoJSON features Closes #4494 * Add explanatory comment * Update tests to conform to numeric-only feature.id * Add explanatory comment
mapbox · Apr 12, 2017 · a2dc4fa · a2dc4fa
1 parent 30d09af
commit a2dc4fa
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 6 deletions.
diff --git a/src/source/geojson_worker_source.js b/src/source/geojson_worker_source.js
@@ -47,6 +47,9 @@ class GeoJSONWorkerSource extends VectorTileWorkerSource {
             return callback(null, null); // nothing in the given tile
         }
 
+        // Encode the geojson-vt tile into binary vector tile form form.  This
+        // is a convenience that allows `FeatureIndex` to operate the same way
+        // across `VectorTileSource` and `GeoJSONSource` data.
         const geojsonWrapper = new GeoJSONWrapper(geoJSONTile.features);
         geojsonWrapper.name = '_geojsonTileLayer';
         let pbf = vtpbf({ layers: { '_geojsonTileLayer': geojsonWrapper }});

diff --git a/src/source/geojson_wrapper.js b/src/source/geojson_wrapper.js
@@ -17,8 +17,15 @@ class FeatureWrapper {
             this.rawGeometry = feature.geometry;
         }
         this.properties = feature.tags;
-        if ('id' in feature) {
-            this.id = feature.id;
+
+        // If the feature has a top-level `id` property, copy it over, but only
+        // if it can be coerced to an integer, because this wrapper is used for
+        // serializing geojson feature data into vector tile PBF data, and the
+        // vector tile spec only supports integer values for feature ids --
+        // allowing non-integer values here results in a non-compliant PBF
+        // that causes an exception when it is parsed with vector-tile-js
+        if ('id' in feature && !isNaN(feature.id)) {
+            this.id = parseInt(feature.id, 10);
         }
         this.extent = EXTENT;
     }

diff --git a/test/integration/query-tests/regressions/mapbox-gl-js#4494/expected.json b/test/integration/query-tests/regressions/mapbox-gl-js#4494/expected.json
@@ -0,0 +1,33 @@
+[
+  {
+    "geometry": {
+      "type": "Polygon",
+      "coordinates": [
+        [
+          [
+            -10.01953125,
+            10.012129557908139
+          ],
+          [
+            10.01953125,
+            10.012129557908139
+          ],
+          [
+            10.01953125,
+            -10.012129557908139
+          ],
+          [
+            -10.01953125,
+            -10.012129557908139
+          ],
+          [
+            -10.01953125,
+            10.012129557908139
+          ]
+        ]
+      ]
+    },
+    "type": "Feature",
+    "properties": {}
+  }
+]
diff --git a/test/integration/query-tests/regressions/mapbox-gl-js#4494/style.json b/test/integration/query-tests/regressions/mapbox-gl-js#4494/style.json
@@ -0,0 +1,44 @@
+{
+  "version": 8,
+  "metadata": {
+    "test": {
+      "width": 64,
+      "height": 64,
+      "queryGeometry": [
+        32,
+        32
+      ],
+      "debug": true
+    }
+  },
+  "zoom": 0,
+  "sources": {
+    "geojson": {
+      "type": "geojson",
+      "data": {
+        "type": "Feature",
+        "id": "foo",
+        "properties": {},
+        "geometry": {
+          "type": "Polygon",
+          "coordinates": [
+            [
+              [ -10, 10 ],
+              [ 10, 10 ],
+              [ 10, -10 ],
+              [ -10, -10 ],
+              [ -10, 10 ]
+            ]
+          ]
+        }
+      }
+    }
+  },
+  "layers": [
+    {
+      "id": "fill",
+      "type": "fill",
+      "source": "geojson"
+    }
+  ]
+}
diff --git a/test/integration/render-tests/regressions/mapbox-gl-js#4235/style.json b/test/integration/render-tests/regressions/mapbox-gl-js#4235/style.json
@@ -15,7 +15,7 @@
           {
             "type": "Feature",
             "properties": {},
-            "id": "a",
+            "id": 1,
             "geometry": {
               "type": "Point",
               "coordinates": [

diff --git a/test/integration/render-tests/regressions/mapbox-gl-native#7792/style.json b/test/integration/render-tests/regressions/mapbox-gl-native#7792/style.json
@@ -15,7 +15,7 @@
           {
             "type": "Feature",
             "properties": {},
-            "id": "a",
+            "id": 1,
             "geometry": {
               "type": "Point",
               "coordinates": [
@@ -27,7 +27,7 @@
           {
             "type": "Feature",
             "properties": {},
-            "id": "b",
+            "id": 2,
             "geometry": {
               "type": "Point",
               "coordinates": [
@@ -48,7 +48,7 @@
       "filter": [
         "==",
         "$id",
-        "a"
+        1
       ]
     }
   ]