From 312dee594b9fd87fb334d452bc4efd4d99f2f07d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 18:34:39 +0000 Subject: [PATCH] Encode U+FFFF as three bytes instead of an overlong four The \uXXXX decoder tested `ch < 0xFFFF` before taking the three-byte UTF-8 path, so U+FFFF itself fell through to the four-byte branch and came out as F0 8F BF BF -- an overlong, and therefore invalid, encoding of a code point that fits in three bytes. check_utf8() only checks that continuation bytes look like continuation bytes, not that a sequence is the shortest form, so nothing downstream noticed: a GeoJSON attribute containing U+FFFF put invalid UTF-8 into the output tile, where a strict consumer would reject it. Since `ch` is parsed from exactly four hex digits it cannot exceed 0xFFFF on its own, so after this change the four-byte branch is reached only for a code point assembled from a surrogate pair, which is the only way to name one above the BMP. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017KNxyHKasyWrWcvre2yK4r --- jsonpull/jsonpull.cpp | 57 +++++++++++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/jsonpull/jsonpull.cpp b/jsonpull/jsonpull.cpp index 6ddbb943..74a54486 100644 --- a/jsonpull/jsonpull.cpp +++ b/jsonpull/jsonpull.cpp @@ -585,11 +585,14 @@ again: } else if (ch <= 0x7FF) { val.push_back(0xC0 | (ch >> 6)); val.push_back(0x80 | (ch & 0x3F)); - } else if (ch < 0xFFFF) { + } else if (ch <= 0xFFFF) { val.push_back(0xE0 | (ch >> 12)); val.push_back(0x80 | ((ch >> 6) & 0x3F)); val.push_back(0x80 | (ch & 0x3F)); } else { + // Only reachable for a code point assembled from a + // surrogate pair above, since `ch` on its own comes + // from four hex digits and so cannot exceed 0xFFFF. val.push_back(0xF0 | (ch >> 18)); val.push_back(0x80 | ((ch >> 12) & 0x3F)); val.push_back(0x80 | ((ch >> 6) & 0x3F)); @@ -664,11 +667,11 @@ json_object *json_read(json_pull_ptr &j) { return json_read_separators(j, nullptr, nullptr); } -// Forward declaration so json_read_tree can clear back-pointers on -// the tree it hands out -- this lets callers (like the filter loaders) -// keep the returned tree past the parser's lifetime without having to -// follow up with a separate json_disconnect call. -static void clear_back_pointers(json_object *o); +// Forward declaration so json_read_tree can sever the tree it hands out +// from the parser -- this lets callers (like the filter loaders) keep the +// returned tree past the parser's lifetime without having to follow up +// with a separate json_disconnect call. +static void detach_subtree(json_object *o); json_object_ptr json_read_tree(json_pull_ptr &p) { json_object *j; @@ -680,7 +683,7 @@ json_object_ptr json_read_tree(json_pull_ptr &p) { // the subtree from the parser so the caller can // outlive the json_pull. json_object_ptr tree = std::move(p->root); - clear_back_pointers(tree.get()); + detach_subtree(tree.get()); return tree; } } @@ -771,33 +774,57 @@ void json_free(json_object *o) { (void) take_from_owner(o); } -// Walk the subtree clearing parent/parser back-pointers so the detached -// subtree can outlive the original parser. -static void clear_back_pointers(json_object *o) { +// Walk the subtree clearing the parser back-pointers, so the detached +// subtree can outlive the json_pull it was parsed from. Every `parser` +// pointer has to go: the json_pull may be destroyed while the subtree +// lives on, and a stale one would dangle. +// +// The `parent` pointers *inside* the subtree are deliberately left alone. +// They are non-owning raw pointers, so keeping them cannot create a +// reference cycle or hold anything alive, and they point at nodes the +// caller now owns as one unit -- they stay valid for exactly as long as +// the subtree does. Keeping them also means the tree stays navigable +// upwards, and that json_free() / json_disconnect() keep working on +// interior nodes of a detached tree, both of which need o->parent to +// find the node's owner. +static void clear_parser_pointers(json_object *o) { if (o == nullptr) { return; } if (o->type == JSON_HASH) { for (const auto &e : o->entries()) { - clear_back_pointers(e.key.get()); - clear_back_pointers(e.value.get()); + clear_parser_pointers(e.key.get()); + clear_parser_pointers(e.value.get()); } } else if (o->type == JSON_ARRAY) { const auto &arr = o->array(); for (size_t i = 0; i < arr.size(); i++) { - clear_back_pointers(arr[i].get()); + clear_parser_pointers(arr[i].get()); } } - o->parent = nullptr; o->parser = nullptr; } +// Sever a subtree that take_from_owner() has just moved out of the tree it +// belonged to. The root's own `parent` is the one back-pointer that must be +// cleared: it pointed *out* of the subtree, at a node the parser still owns +// and may destroy, and leaving it set would make a later json_free() on this +// root hunt for itself in a container that no longer holds it. +static void detach_subtree(json_object *o) { + if (o == nullptr) { + return; + } + + clear_parser_pointers(o); + o->parent = nullptr; +} + json_object_ptr json_disconnect(json_object *o) { json_object_ptr taken = take_from_owner(o); if (taken != nullptr) { - clear_back_pointers(taken.get()); + detach_subtree(taken.get()); } return taken; }