diff --git a/Makefile b/Makefile index 88a593ce..3433af0a 100644 --- a/Makefile +++ b/Makefile @@ -261,6 +261,14 @@ raw-tiles-test: tippecanoe tippecanoe-decode tile-join ./tippecanoe-decode -x generator tests/raw-tiles/nothing > tests/raw-tiles/nothing.json.check cmp tests/raw-tiles/nothing.json.check tests/raw-tiles/nothing.json rm -r tests/raw-tiles/nothing tests/raw-tiles/nothing.json.check + # Test that a non-string value in metadata.json is reported and skipped + # instead of being read as a string (which used to crash) + ./tippecanoe -q -f -e tests/raw-tiles/nonstring tests/raw-tiles/hackspots.geojson + sed -i.bak 's/"minzoom": "0"/"minzoom": 0/' tests/raw-tiles/nonstring/metadata.json + rm tests/raw-tiles/nonstring/metadata.json.bak + grep -q '"minzoom": 0' tests/raw-tiles/nonstring/metadata.json + ./tippecanoe-decode -x generator tests/raw-tiles/nonstring > /dev/null + rm -r tests/raw-tiles/nonstring pmtiles-test: tippecanoe tippecanoe-decode tile-join ./tippecanoe -q -f -o tests/pmtiles/hackspots.pmtiles -r1 -pC tests/raw-tiles/hackspots.geojson diff --git a/dirtiles.cpp b/dirtiles.cpp index 452ecf9e..f930f2ad 100644 --- a/dirtiles.cpp +++ b/dirtiles.cpp @@ -261,8 +261,15 @@ sqlite3 *dirmeta2tmp(const char *fname) { } for (const auto &e : o->entries()) { + // Skip, rather than just warn about, anything that isn't a + // string/string pair: reading a non-string through string() + // would assert in a debug build and misinterpret the node's + // storage in a release build. (A metadata.json written by + // something other than tippecanoe may well have numeric + // minzoom/maxzoom or a nested "json" object.) if (e.key->type != JSON_STRING || e.value->type != JSON_STRING) { fprintf(stderr, "%s: non-string in metadata\n", name.c_str()); + continue; } char *sql = sqlite3_mprintf("INSERT INTO metadata (name, value) VALUES (%Q, %Q);", e.key->string().c_str(), e.value->string().c_str()); diff --git a/pmtiles_file.cpp b/pmtiles_file.cpp index 57695b1a..e4cb7947 100644 --- a/pmtiles_file.cpp +++ b/pmtiles_file.cpp @@ -416,6 +416,16 @@ sqlite3 *pmtilesmeta2tmp(const char *fname, const char *pmtiles_map) { state.json_write_hash(); for (const auto &e : o->entries()) { + // Establish that the key really is a string before reading it as + // one, rather than after: string() asserts on the type, so the + // check has to come first to be the thing that catches a bad key. + // (The parser rejects non-string hash keys, so this is belt and + // braces, but the ordering is what makes it meaningful.) + if (e.key->type != JSON_STRING) { + fprintf(stderr, "%s: non-string key in metadata\n", fname); + continue; + } + const std::string &key = e.key->string(); if (key == "vector_layers" && e.value->type == JSON_ARRAY) { has_json = true; @@ -441,7 +451,7 @@ sqlite3 *pmtilesmeta2tmp(const char *fname, const char *pmtiles_map) { fprintf(stderr, "set %s in metadata: %s\n", key.c_str(), err); } sqlite3_free(sql); - } else if (e.key->type != JSON_STRING || e.value->type != JSON_STRING) { + } else if (e.value->type != JSON_STRING) { fprintf(stderr, "%s\n", key.c_str()); fprintf(stderr, "%s: non-string in metadata\n", fname); } else {