From 05b17620771c7f6d3e736d7766c58638cacb5a45 Mon Sep 17 00:00:00 2001 From: Erica Fischer Date: Sat, 30 May 2026 14:25:39 -0700 Subject: [PATCH] Fix bugs flagged in code review of jsonpull C++ port - jsontool.cpp `out()`: route JSON_NUMBER (and anything else non-string) through `json_stringify` instead of `o->string()`, which now asserts on a non-string type and would crash `--extract` on numeric attributes. - geojson.{hpp,cpp} `json_end_map`: take `json_pull_ptr` by reference so the caller's shared_ptr is released, null-guard before touching `jp->source`, and clear `jp->source` after delete to avoid a dangling pointer. - jsonpull/jsonpull.cpp: low-surrogate range check was comparing the outer-loop byte `c` instead of the parsed code unit `ch`, breaking surrogate-pair decoding for some \\uXXXX escapes. Pre-existing bug preserved across the port. - tile-join.cpp `handle_vector_layers`: require the field value to have type JSON_STRING (and the key to be non-null) before calling `string()`; the previous truthy `type` check would assert on a non-string value. Co-authored-by: Cursor --- geojson.cpp | 6 +++++- geojson.hpp | 2 +- jsonpull/jsonpull.cpp | 2 +- jsontool.cpp | 6 +++++- tile-join.cpp | 3 ++- 5 files changed, 14 insertions(+), 5 deletions(-) diff --git a/geojson.cpp b/geojson.cpp index 66cad85b..3a64e08f 100644 --- a/geojson.cpp +++ b/geojson.cpp @@ -306,7 +306,11 @@ json_pull_ptr json_begin_map(char *map, long long len) { return json_begin(json_map_read, jm); } -void json_end_map(json_pull_ptr jp) { +void json_end_map(json_pull_ptr &jp) { + if (jp == nullptr) { + return; + } delete (struct jsonmap *) jp->source; + jp->source = nullptr; json_end(jp); } diff --git a/geojson.hpp b/geojson.hpp index aca48008..cb0776a4 100644 --- a/geojson.hpp +++ b/geojson.hpp @@ -22,7 +22,7 @@ struct parse_json_args { }; json_pull_ptr json_begin_map(char *map, long long len); -void json_end_map(json_pull_ptr jp); +void json_end_map(json_pull_ptr &jp); void parse_json(struct serialization_state *sst, json_pull_ptr jp, int layer, std::string layername); void *run_parse_json(void *v); diff --git a/jsonpull/jsonpull.cpp b/jsonpull/jsonpull.cpp index 22e0cd94..3929e1f0 100644 --- a/jsonpull/jsonpull.cpp +++ b/jsonpull/jsonpull.cpp @@ -551,7 +551,7 @@ again: surrogate = ch; } continue; - } else if (ch >= 0xdc00 && c <= 0xdfff) { + } else if (ch >= 0xdc00 && ch <= 0xdfff) { if (surrogate >= 0) { long c1 = surrogate - 0xd800; long c2 = ch - 0xdc00; diff --git a/jsontool.cpp b/jsontool.cpp index 79077396..52cb1c33 100644 --- a/jsontool.cpp +++ b/jsontool.cpp @@ -148,9 +148,13 @@ void out(std::string const &s, int type, json_object_ptr properties) { json_object_ptr o = json_hash_get(properties, extract); if (o != nullptr) { found = true; - if (o->type == JSON_STRING || o->type == JSON_NUMBER) { + if (o->type == JSON_STRING) { extracted = sort_quote(o->string().c_str()); } else { + // Numbers, booleans, null, and any other non-string + // values are rendered via json_stringify(); calling + // o->string() here would assert because the type-tagged + // accessor requires JSON_STRING. extracted = sort_quote(json_stringify(o).c_str()); } } diff --git a/tile-join.cpp b/tile-join.cpp index 5f7af368..b729230c 100644 --- a/tile-join.cpp +++ b/tile-join.cpp @@ -1035,7 +1035,8 @@ void handle_vector_layers(json_object_ptr vector_layers, std::maparray()[i], "fields"); if (fields != nullptr && fields->type == JSON_HASH) { for (const auto &e : fields->entries()) { - if (e.key->type == JSON_STRING && e.value->type) { + if (e.key != nullptr && e.key->type == JSON_STRING && + e.value != nullptr && e.value->type == JSON_STRING) { const std::string &desc2 = e.value->string(); if (desc2 != "Number" &&