Merge pull request #585 from mapbox/nulls-from-filter

Be careful not to allow null attributes from prefilter/postfilter output to make it into tiles

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.29.2
+
+* Be careful to remove null attributes from prefilter/postfilter output
+
 ## 1.29.1
 
 * Add --use-source-polygon-winding and --reverse-source-polygon-winding

geojson.cpp

@@ -341,7 +341,7 @@ void parse_json(struct serialization_state *sst, json_pull *jp, int layer, std::
 		json_object *id = json_hash_get(j, "id");
 
 		json_object *geometries = json_hash_get(geometry, "geometries");
-		if (geometries != NULL) {
+		if (geometries != NULL && geometries->type == JSON_ARRAY) {
 			size_t g;
 			for (g = 0; g < geometries->length; g++) {
 				serialize_geojson_feature(sst, geometries->array[g], properties, id, layer, tippecanoe, j, layername);

mvt.cpp

@@ -122,6 +122,9 @@ bool mvt_tile::decode(std::string &message, bool &was_compressed) {
 					protozero::pbf_reader value_reader(layer_reader.get_message());
 					mvt_value value;
 
+					value.type = mvt_null;
+					value.numeric_value.null_value = 0;
+
 					while (value_reader.next()) {
 						switch (value_reader.tag()) {
 						case 1: /* string */
@@ -303,6 +306,12 @@ std::string mvt_tile::encode() {
 				value_writer.add_sint64(6, pbv.numeric_value.sint_value);
 			} else if (pbv.type == mvt_bool) {
 				value_writer.add_bool(7, pbv.numeric_value.bool_value);
+			} else if (pbv.type == mvt_null) {
+				fprintf(stderr, "Internal error: trying to write null attribute to tile\n");
+				exit(EXIT_FAILURE);
+			} else {
+				fprintf(stderr, "Internal error: trying to write undefined attribute type to tile\n");
+				exit(EXIT_FAILURE);
 			}
 
 			layer_writer.add_message(4, value_string);
@@ -388,7 +397,8 @@ bool mvt_value::operator<(const mvt_value &o) const {
 		    (type == mvt_int && numeric_value.int_value < o.numeric_value.int_value) ||
 		    (type == mvt_uint && numeric_value.uint_value < o.numeric_value.uint_value) ||
 		    (type == mvt_sint && numeric_value.sint_value < o.numeric_value.sint_value) ||
-		    (type == mvt_bool && numeric_value.bool_value < o.numeric_value.bool_value)) {
+		    (type == mvt_bool && numeric_value.bool_value < o.numeric_value.bool_value) ||
+		    (type == mvt_null && numeric_value.null_value < o.numeric_value.null_value)) {
 			return true;
 		}
 	}
@@ -442,6 +452,8 @@ std::string mvt_value::toString() {
 		return std::to_string(numeric_value.uint_value);
 	} else if (type == mvt_bool) {
 		return numeric_value.bool_value ? "true" : "false";
+	} else if (type == mvt_null) {
+		return "null";
 	} else {
 		return "unknown";
 	}
@@ -591,6 +603,7 @@ mvt_value stringified_to_mvt_value(int type, const char *s) {
 		tv.numeric_value.bool_value = (s[0] == 't');
 	} else if (type == mvt_null) {
 		tv.type = mvt_null;
+		tv.numeric_value.null_value = 0;
 	} else {
 		tv.type = mvt_string;
 		tv.string_value = s;

mvt.hpp

@@ -77,6 +77,7 @@ struct mvt_value {
 		unsigned long long uint_value;
 		long long sint_value;
 		bool bool_value;
+		int null_value;
 	} numeric_value;
 
 	bool operator<(const mvt_value &o) const;

plugin.cpp

@@ -259,7 +259,11 @@ std::vector<mvt_layer> parse_layers(int fd, int z, unsigned x, unsigned y, std::
 				std::string s;
 
 				stringify_value(properties->values[i], tp, s, "Filter output", jp->line, j);
-				if (tp >= 0) {
+
+				// Nulls can be excluded here because this is the postfilter
+				// and it is nearly time to create the vector representation
+
+				if (tp >= 0 && tp != mvt_null) {
 					mvt_value v = stringified_to_mvt_value(tp, s.c_str());
 					l->second.tag(feature, std::string(properties->keys[i]->string), v);
 
@@ -493,7 +497,10 @@ serial_feature parse_feature(json_pull *jp, int z, unsigned x, unsigned y, std::
 
 				stringify_value(properties->values[i], v.type, v.s, "Filter output", jp->line, j);
 
-				if (v.type >= 0) {
+				// Nulls can be excluded here because the expression evaluation filter
+				// would have already run before prefiltering
+
+				if (v.type >= 0 && v.type != mvt_null) {
 					sf.full_keys.push_back(std::string(properties->keys[i]->string));
 					sf.full_values.push_back(v);
 

tests/filter/null

@@ -0,0 +1,3 @@
+#!/bin/sh
+
+echo '{ "type": "Feature", "properties": { "scalerank": 1, "featurecla": "Admin-0 country", "labelrank": 3.000000, "sovereignt": "Afghanistan", "sov_a3": "AFG", "adm0_dif": 0.000000, "level": 2.000000, "type": "Sovereign country", "admin": "Afghanistan", "adm0_a3": "AFG", "geou_dif": 0.000000, "geounit": "Afghanistan", "gu_a3": "AFG", "su_dif": 0.000000, "subunit": "Afghanistan", "su_a3": "AFG", "brk_diff": 0.000000, "name": "Afghanistan", "name_long": "Afghanistan", "brk_a3": "AFG", "brk_name": "Afghanistan", "brk_group": null, "abbrev": "Afg.", "postal": "AF", "formal_en": "Islamic State of Afghanistan", "formal_fr": null, "note_adm0": null, "note_brk": null, "name_sort": "Afghanistan", "name_alt": null, "mapcolor7": 5.000000, "mapcolor8": 6.000000, "mapcolor9": 8.000000, "mapcolor13": 7.000000, "pop_est": 28400000.000000, "gdp_md_est": 22270.000000, "pop_year": -99.000000, "lastcensus": 1979.000000, "gdp_year": -99.000000, "economy": "7. Least developed region", "income_grp": "5. Low income", "wikipedia": -99.000000, "fips_10": null, "iso_a2": "AF", "iso_a3": "AFG", "iso_n3": "004", "un_a3": "004", "wb_a2": "AF", "wb_a3": "AFG", "woe_id": -99.000000, "adm0_a3_is": "AFG", "adm0_a3_us": "AFG", "adm0_a3_un": -99.000000, "adm0_a3_wb": -99.000000, "continent": "Asia", "region_un": "Asia", "subregion": "Southern Asia", "region_wb": "South Asia", "name_len": 11.000000, "long_len": 11.000000, "abbrev_len": 4.000000, "tiny": -99.000000, "homepart": 1.000000 }, "geometry": { "type": "Polygon", "coordinates": [ [ [ 61.210817, 35.650072 ], [ 62.230651, 35.270663 ], [ 62.984662, 35.404040 ], [ 63.193538, 35.857165 ], [ 63.982895, 36.007957 ], [ 64.546479, 36.312073 ], [ 64.746105, 37.111817 ], [ 65.588947, 37.305216 ], [ 65.745630, 37.661164 ], [ 66.217384, 37.393790 ], [ 66.518606, 37.362784 ], [ 67.075782, 37.356143 ], [ 67.829999, 37.144994 ], [ 68.135562, 37.023115 ], [ 68.859445, 37.344335 ], [ 69.196272, 37.151143 ], [ 69.518785, 37.608996 ], [ 70.116578, 37.588222 ], [ 70.270574, 37.735164 ], [ 70.376304, 38.138395 ], [ 70.806820, 38.486281 ], [ 71.348131, 38.258905 ], [ 71.239403, 37.953265 ], [ 71.541917, 37.905774 ], [ 71.448693, 37.065644 ], [ 71.844638, 36.738171 ], [ 72.193040, 36.948287 ], [ 72.636889, 37.047558 ], [ 73.260055, 37.495256 ], [ 73.948695, 37.421566 ], [ 74.980002, 37.419990 ], [ 75.158027, 37.133030 ], [ 74.575892, 37.020841 ], [ 74.067551, 36.836175 ], [ 72.920024, 36.720007 ], [ 71.846291, 36.509942 ], [ 71.262348, 36.074387 ], [ 71.498767, 35.650563 ], [ 71.613076, 35.153203 ], [ 71.115018, 34.733125 ], [ 71.156773, 34.348911 ], [ 70.881803, 33.988855 ], [ 69.930543, 34.020120 ], [ 70.323594, 33.358532 ], [ 69.687147, 33.105498 ], [ 69.262522, 32.501944 ], [ 69.317764, 31.901412 ], [ 68.926676, 31.620189 ], [ 68.556932, 31.713310 ], [ 67.792689, 31.582930 ], [ 67.683393, 31.303154 ], [ 66.938891, 31.304911 ], [ 66.381457, 30.738899 ], [ 66.346472, 29.887943 ], [ 65.046862, 29.472180 ], [ 64.350418, 29.560030 ], [ 64.148002, 29.340819 ], [ 63.550260, 29.468330 ], [ 62.549856, 29.318572 ], [ 60.874248, 29.829238 ], [ 61.781221, 30.735850 ], [ 61.699314, 31.379506 ], [ 60.941944, 31.548074 ], [ 60.863654, 32.182919 ], [ 60.536077, 32.981268 ], [ 60.963700, 33.528832 ], [ 60.528429, 33.676446 ], [ 60.803193, 34.404101 ], [ 61.210817, 35.650072 ] ] ] } }'

tile.cpp

@@ -1388,7 +1388,7 @@ serial_feature next_feature(FILE *geoms, std::atomic<long long> *geompos_in, cha
 			sf.dropped = true;
 		}
 
-		// Remove nulls, now that the filter has run
+		// Remove nulls, now that the expression evaluation filter has run
 
 		for (ssize_t i = (ssize_t) sf.keys.size() - 1; i >= 0; i--) {
 			int type = (stringpool + pool_off[sf.segment])[sf.values[i]];

version.hpp

@@ -1,6 +1,6 @@
 #ifndef VERSION_HPP
 #define VERSION_HPP
 
-#define VERSION "tippecanoe v1.29.1\n"
+#define VERSION "tippecanoe v1.29.2\n"
 
 #endif

write_json.cpp

@@ -337,6 +337,12 @@ void layer_to_geojson(mvt_layer const &layer, unsigned z, unsigned x, unsigned y
 			} else if (val.type == mvt_bool) {
 				state.json_write_string(key);
 				state.json_write_bool(val.numeric_value.bool_value);
+			} else if (val.type == mvt_null) {
+				state.json_write_string(key);
+				state.json_write_null();
+			} else {
+				fprintf(stderr, "Internal error: property with unknown type\n");
+				exit(EXIT_FAILURE);
 			}
 		}