Merge branch 'master' into more-warnings

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.26.6
+
+* Be more careful about checking for overflow when parsing numbers
+
 ## 1.26.5
 
 * Support UTF-16 surrogate pairs in JSON strings

Makefile

@@ -91,7 +91,8 @@ test: tippecanoe tippecanoe-decode $(addsuffix .check,$(TESTS)) raw-tiles-test p
 	cmp $@.out $(patsubst %.check,%,$@)
 	rm $@.out $@.mbtiles
 
-geobuf-test: geojson2nd $(addsuffix .checkbuf,$(TESTS))
+# Don't test overflow with geobuf, because it fails (https://github.com/mapbox/geobuf/issues/87)
+geobuf-test: geojson2nd $(addsuffix .checkbuf,$(filter-out tests/overflow/out/-z0.json,$(TESTS)))
 
 # For quicker address sanitizer build, hope that regular JSON parsing is tested enough by parallel and join tests
 fewer-tests: tippecanoe tippecanoe-decode geobuf-test raw-tiles-test parallel-test pbf-test join-test enumerate-test decode-test join-filter-test unit

mvt.cpp

@@ -6,6 +6,7 @@
 #include <zlib.h>
 #include <errno.h>
 #include <limits.h>
+#include <ctype.h>
 #include "mvt.hpp"
 #include "geometry.hpp"
 #include "protozero/varint.hpp"
@@ -420,7 +421,7 @@ std::string mvt_value::toString() {
 	if (type == mvt_string) {
 		return quote(string_value);
 	} else if (type == mvt_int) {
-		return std::to_string((long long) numeric_value.int_value);
+		return std::to_string(numeric_value.int_value);
 	} else if (type == mvt_double) {
 		double v = numeric_value.double_value;
 		if (v == (long long) v) {
@@ -436,9 +437,9 @@ std::string mvt_value::toString() {
 			return milo::dtoa_milo(v);
 		}
 	} else if (type == mvt_sint) {
-		return std::to_string((long long) numeric_value.sint_value);
+		return std::to_string(numeric_value.sint_value);
 	} else if (type == mvt_uint) {
-		return std::to_string((long long) numeric_value.uint_value);
+		return std::to_string(numeric_value.uint_value);
 	} else if (type == mvt_bool) {
 		return numeric_value.bool_value ? "true" : "false";
 	} else {
@@ -472,7 +473,7 @@ void mvt_layer::tag(mvt_feature &feature, std::string key, mvt_value value) {
 	feature.tags.push_back(vo);
 }
 
-static int is_integer(const char *s, long long *v) {
+bool is_integer(const char *s, long long *v) {
 	errno = 0;
 	char *endptr;
 
@@ -480,7 +481,47 @@ static int is_integer(const char *s, long long *v) {
 	if (*v == 0 && errno != 0) {
 		return 0;
 	}
-	if ((*v == LLONG_MIN || *v == LLONG_MAX) && (errno == ERANGE)) {
+	if ((*v == LLONG_MIN || *v == LLONG_MAX) && (errno == ERANGE || errno == EINVAL)) {
+		return 0;
+	}
+	if (*endptr != '\0') {
+		// Special case: If it is an integer followed by .0000 or similar,
+		// it is still an integer
+
+		if (*endptr != '.') {
+			return 0;
+		}
+		endptr++;
+		for (; *endptr != '\0'; endptr++) {
+			if (*endptr != '0') {
+				return 0;
+			}
+		}
+
+		return 1;
+	}
+
+	return 1;
+}
+
+bool is_unsigned_integer(const char *s, unsigned long long *v) {
+	errno = 0;
+	char *endptr;
+
+	// Special check because MacOS stroull() returns 1
+	// for -18446744073709551615
+	while (isspace(*s)) {
+		s++;
+	}
+	if (*s == '-') {
+		return 0;
+	}
+
+	*v = strtoull(s, &endptr, 0);
+	if (*v == 0 && errno != 0) {
+		return 0;
+	}
+	if ((*v == ULLONG_MAX) && (errno == ERANGE || errno == EINVAL)) {
 		return 0;
 	}
 	if (*endptr != '\0') {
@@ -508,23 +549,41 @@ mvt_value stringified_to_mvt_value(int type, const char *s) {
 
 	if (type == mvt_double) {
 		long long v;
-		if (is_integer(s, &v)) {
+		unsigned long long uv;
-			if (v >= 0) {
+		if (is_unsigned_integer(s, &uv)) {
+			if (uv <= LLONG_MAX) {
 				tv.type = mvt_int;
-				tv.numeric_value.int_value = v;
+				tv.numeric_value.int_value = uv;
 			} else {
-				tv.type = mvt_sint;
+				tv.type = mvt_uint;
-				tv.numeric_value.sint_value = v;
+				tv.numeric_value.uint_value = uv;
 			}
+		} else if (is_integer(s, &v)) {
+			tv.type = mvt_sint;
+			tv.numeric_value.sint_value = v;
 		} else {
-			double d = atof(s);
+			errno = 0;
+			char *endptr;
 
-			if (d == (float) d) {
+			float f = strtof(s, &endptr);
-				tv.type = mvt_float;
+
-				tv.numeric_value.float_value = d;
+			if (endptr == s || ((f == HUGE_VAL || f == HUGE_VALF || f == HUGE_VALL) && errno == ERANGE)) {
-			} else {
+				double d = strtod(s, &endptr);
+				if (endptr == s || ((d == HUGE_VAL || d == HUGE_VALF || d == HUGE_VALL) && errno == ERANGE)) {
+					fprintf(stderr, "Warning: numeric value %s could not be represented\n", s);
+				}
 				tv.type = mvt_double;
 				tv.numeric_value.double_value = d;
+			} else {
+				double d = atof(s);
+				if (f == d) {
+					tv.type = mvt_float;
+					tv.numeric_value.float_value = f;
+				} else {
+					// Conversion succeeded, but lost precision, so use double
+					tv.type = mvt_double;
+					tv.numeric_value.double_value = d;
+				}
 			}
 		}
 	} else if (type == mvt_bool) {

mvt.hpp

@@ -117,4 +117,7 @@ int compress(std::string const &input, std::string &output);
 int dezig(unsigned n);
 
 mvt_value stringified_to_mvt_value(int type, const char *s);
+
+bool is_integer(const char *s, long long *v);
+bool is_unsigned_integer(const char *s, unsigned long long *v);
 #endif

read_json.cpp

@@ -105,7 +105,17 @@ void parse_geometry(int t, json_object *j, drawvec &out, int op, const char *fna
 
 void canonicalize(json_object *o) {
 	if (o->type == JSON_NUMBER) {
-		std::string s = milo::dtoa_milo(o->number);
+		std::string s;
+		long long v;
+		unsigned long long uv;
+
+		if (is_integer(o->string, &v)) {
+			s = std::to_string(v);
+		} else if (is_unsigned_integer(o->string, &uv)) {
+			s = std::to_string(uv);
+		} else {
+			s = milo::dtoa_milo(o->number);
+		}
 		free(o->string);
 		o->string = strdup(s.c_str());
 	} else if (o->type == JSON_HASH) {
@@ -150,7 +160,17 @@ void stringify_value(json_object *value, int &type, std::string &stringified, co
 			}
 		} else if (vt == JSON_NUMBER) {
 			type = mvt_double;
-			stringified = milo::dtoa_milo(value->number);
+
+			long long v;
+			unsigned long long uv;
+
+			if (is_integer(value->string, &v)) {
+				stringified = std::to_string(v);
+			} else if (is_unsigned_integer(value->string, &uv)) {
+				stringified = std::to_string(uv);
+			} else {
+				stringified = milo::dtoa_milo(value->number);
+			}
 		} else if (vt == JSON_TRUE || vt == JSON_FALSE) {
 			type = mvt_bool;
 			stringified = val;

tests/overflow/in.json

@@ -0,0 +1,15 @@
+{ "type": "Feature", "properties": { "excess": 2222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222 }, "geometry": { "type": "Point", "coordinates": [ 0,0 ] } }
+{ "type": "Feature", "properties": { "excess": 22e291 }, "geometry": { "type": "Point", "coordinates": [ 0,0 ] } }
+{ "type": "Feature", "properties": { "excess": 2.5 }, "geometry": { "type": "Point", "coordinates": [ 0,0 ] } }
+{ "type": "Feature", "properties": { "excess": 2147483648 }, "geometry": { "type": "Point", "coordinates": [ 0,0 ] } }
+{ "type": "Feature", "properties": { "excess": -2147483648 }, "geometry": { "type": "Point", "coordinates": [ 0,0 ] } }
+{ "type": "Feature", "properties": { "excess": 2147483647 }, "geometry": { "type": "Point", "coordinates": [ 0,0 ] } }
+{ "type": "Feature", "properties": { "excess": -2147483647 }, "geometry": { "type": "Point", "coordinates": [ 0,0 ] } }
+{ "type": "Feature", "properties": { "excess": 18446744073709551616 }, "geometry": { "type": "Point", "coordinates": [ 0,0 ] } }
+{ "type": "Feature", "properties": { "excess": -18446744073709551616 }, "geometry": { "type": "Point", "coordinates": [ 0,0 ] } }
+{ "type": "Feature", "properties": { "excess": 18446744073709551615 }, "geometry": { "type": "Point", "coordinates": [ 0,0 ] } }
+{ "type": "Feature", "properties": { "excess": -18446744073709551615 }, "geometry": { "type": "Point", "coordinates": [ 0,0 ] } }
+{ "type": "Feature", "properties": { "excess": 9223372036854775808 }, "geometry": { "type": "Point", "coordinates": [ 0,0 ] } }
+{ "type": "Feature", "properties": { "excess": -9223372036854775808 }, "geometry": { "type": "Point", "coordinates": [ 0,0 ] } }
+{ "type": "Feature", "properties": { "excess": 9223372036854775807 }, "geometry": { "type": "Point", "coordinates": [ 0,0 ] } }
+{ "type": "Feature", "properties": { "excess": -9223372036854775807 }, "geometry": { "type": "Point", "coordinates": [ 0,0 ] } }

tests/overflow/out/-z0.json

@@ -0,0 +1,46 @@
+{ "type": "FeatureCollection", "properties": {
+"bounds": "0.000000,0.000000,0.000000,0.000000",
+"center": "0.000000,0.000000,0",
+"description": "tests/overflow/out/-z0.json.check.mbtiles",
+"format": "pbf",
+"json": "{\"vector_layers\": [ { \"id\": \"in\", \"description\": \"\", \"minzoom\": 0, \"maxzoom\": 0, \"fields\": {\"excess\": \"Number\"} } ],\"tilestats\": {\"layerCount\": 1,\"layers\": [{\"layer\": \"in\",\"count\": 15,\"geometry\": \"Point\",\"attributeCount\": 1,\"attributes\": [{\"attribute\": \"excess\",\"count\": 14,\"type\": \"number\",\"values\": [-18446744073709553000,-2147483647,-2147483648,-9223372036854775807,-9223372036854775808,18446744073709551615,18446744073709553000,2.2222222222222223e+291,2.2e+292,2.5,2147483647,2147483648,9223372036854775807,9223372036854775808],\"min\": -18446744073709553000,\"max\": 2.2e+292}]}]}}",
+"maxzoom": "0",
+"minzoom": "0",
+"name": "tests/overflow/out/-z0.json.check.mbtiles",
+"type": "overlay",
+"version": "2"
+}, "features": [
+{ "type": "FeatureCollection", "properties": { "zoom": 0, "x": 0, "y": 0 }, "features": [
+{ "type": "FeatureCollection", "properties": { "layer": "in", "version": 2, "extent": 4096 }, "features": [
+{ "type": "Feature", "properties": { "excess": 2.2222222222222223e+291 }, "geometry": { "type": "Point", "coordinates": [ 0.000000, 0.000000 ] } }
+,
+{ "type": "Feature", "properties": { "excess": 2.2e+292 }, "geometry": { "type": "Point", "coordinates": [ 0.000000, 0.000000 ] } }
+,
+{ "type": "Feature", "properties": { "excess": 2.5 }, "geometry": { "type": "Point", "coordinates": [ 0.000000, 0.000000 ] } }
+,
+{ "type": "Feature", "properties": { "excess": 2147483648 }, "geometry": { "type": "Point", "coordinates": [ 0.000000, 0.000000 ] } }
+,
+{ "type": "Feature", "properties": { "excess": -2147483648 }, "geometry": { "type": "Point", "coordinates": [ 0.000000, 0.000000 ] } }
+,
+{ "type": "Feature", "properties": { "excess": 2147483647 }, "geometry": { "type": "Point", "coordinates": [ 0.000000, 0.000000 ] } }
+,
+{ "type": "Feature", "properties": { "excess": -2147483647 }, "geometry": { "type": "Point", "coordinates": [ 0.000000, 0.000000 ] } }
+,
+{ "type": "Feature", "properties": { "excess": 18446744073709553000 }, "geometry": { "type": "Point", "coordinates": [ 0.000000, 0.000000 ] } }
+,
+{ "type": "Feature", "properties": { "excess": -18446744073709553000 }, "geometry": { "type": "Point", "coordinates": [ 0.000000, 0.000000 ] } }
+,
+{ "type": "Feature", "properties": { "excess": 18446744073709551615 }, "geometry": { "type": "Point", "coordinates": [ 0.000000, 0.000000 ] } }
+,
+{ "type": "Feature", "properties": { "excess": -18446744073709553000 }, "geometry": { "type": "Point", "coordinates": [ 0.000000, 0.000000 ] } }
+,
+{ "type": "Feature", "properties": { "excess": 9223372036854775808 }, "geometry": { "type": "Point", "coordinates": [ 0.000000, 0.000000 ] } }
+,
+{ "type": "Feature", "properties": { "excess": -9223372036854775808 }, "geometry": { "type": "Point", "coordinates": [ 0.000000, 0.000000 ] } }
+,
+{ "type": "Feature", "properties": { "excess": 9223372036854775807 }, "geometry": { "type": "Point", "coordinates": [ 0.000000, 0.000000 ] } }
+,
+{ "type": "Feature", "properties": { "excess": -9223372036854775807 }, "geometry": { "type": "Point", "coordinates": [ 0.000000, 0.000000 ] } }
+] }
+] }
+] }

version.hpp

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

write_json.cpp

@@ -114,7 +114,7 @@ void layer_to_geojson(FILE *fp, mvt_layer const &layer, unsigned z, unsigned x,
 				fprintq(fp, val.string_value.c_str());
 			} else if (val.type == mvt_int) {
 				fprintq(fp, key);
-				fprintf(fp, ": %lld", (long long) val.numeric_value.int_value);
+				fprintf(fp, ": %lld", val.numeric_value.int_value);
 			} else if (val.type == mvt_double) {
 				fprintq(fp, key);
 				double v = val.numeric_value.double_value;
@@ -133,10 +133,10 @@ void layer_to_geojson(FILE *fp, mvt_layer const &layer, unsigned z, unsigned x,
 				}
 			} else if (val.type == mvt_sint) {
 				fprintq(fp, key);
-				fprintf(fp, ": %lld", (long long) val.numeric_value.sint_value);
+				fprintf(fp, ": %lld", val.numeric_value.sint_value);
 			} else if (val.type == mvt_uint) {
 				fprintq(fp, key);
-				fprintf(fp, ": %lld", (long long) val.numeric_value.uint_value);
+				fprintf(fp, ": %llu", val.numeric_value.uint_value);
 			} else if (val.type == mvt_bool) {
 				fprintq(fp, key);
 				fprintf(fp, ": %s", val.numeric_value.bool_value ? "true" : "false");