Cover the U+FFFF encoding and detached-tree parent links

Each of the new assertions fails against the previous behavior, so they
pin the two fixes rather than merely passing alongside them:

  - the U+FFFF test, plus the U+FFFE boundary below it and a surrogate pair
    above it, so the three-byte and four-byte paths are both held in place
  - json_disconnect() leaving the parent links inside the subtree intact
    while clearing the root's
  - json_free() pruning an interior node of a tree whose parser is already
    gone, which only works because those links survive
  - json_free() of a hash value leaving the key paired with a JSON_NULL
    placeholder, which is the documented behavior and not a removal

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KNxyHKasyWrWcvre2yK4r
This commit is contained in:
Claude
2026-08-12 18:35:59 +00:00
parent 8d187f72c8
commit fda97a7ca9
2 changed files with 103 additions and 4 deletions
+2
View File
@@ -11,6 +11,8 @@
* Fix decoding of a `\u` escape sequence in which a high surrogate is followed
by a non-surrogate BMP code point, which combined the two into a single
wrong code point. (#388)
* Fix a `\uFFFF` escape being decoded to the overlong, invalid four-byte
UTF-8 sequence `F0 8F BF BF` instead of `EF BF BF`. (#388)
* Fix tile-join reading a non-string field type out of a tileset's tilejson.
(#388)
* Fix `tippecanoe-decode` and tile-join crashing on a directory tileset whose
+101 -4
View File
@@ -286,14 +286,20 @@ TEST_CASE("json_disconnect hands a subtree to the caller", "[jsonpull][ownership
REQUIRE(outer->array()[0]->array()[0]->number() == 1);
REQUIRE(outer->array()[0]->array()[1]->number() == 2);
// The detached subtree holds no back-pointers into the parser...
REQUIRE(taken->parent == nullptr);
// The detached subtree holds no pointers into the parser...
REQUIRE(taken->parser == nullptr);
REQUIRE(taken->array()[0]->parser == nullptr);
REQUIRE(taken->array()[1]->parser == nullptr);
// ...so it stays valid once the parser, and the tree the parser still
// owns, are destroyed.
// ...and its root no longer points out at the array it was spliced
// from, but the parent links *within* it are left intact so the tree
// stays navigable upwards.
REQUIRE(taken->parent == nullptr);
REQUIRE(taken->array()[0]->parent == taken.get());
REQUIRE(taken->array()[1]->parent == taken.get());
// So the subtree stays valid once the parser, and the tree the parser
// still owns, are destroyed.
jp.reset();
// outer is dangling now; do not dereference.
@@ -303,6 +309,97 @@ TEST_CASE("json_disconnect hands a subtree to the caller", "[jsonpull][ownership
REQUIRE(taken->array()[1]->number() == 4);
}
// Preserving the parent links inside a detached tree is what lets
// json_free() keep working on its interior nodes: json_free() finds a
// node's owner through o->parent, so a detached tree whose parent links
// had been cleared would silently ignore the request and leave the node
// in place.
TEST_CASE("json_free prunes an interior node of a detached tree", "[jsonpull][ownership]") {
json_pull_ptr jp = json_begin_string("[[1, 2], [3, 4], [5, 6]]");
json_object_ptr tree = json_read_tree(jp);
REQUIRE(tree != nullptr);
REQUIRE(tree->type == JSON_ARRAY);
REQUIRE(tree->array().size() == 3);
// Destroy the parser first, so this is unambiguously operating on a
// tree that no longer has one.
jp.reset();
REQUIRE(tree->parser == nullptr);
json_object *drop = tree->array()[1].get();
REQUIRE(drop->array()[0]->number() == 3);
REQUIRE(drop->parent == tree.get());
json_free(drop);
// drop is dangling now; do not dereference.
// Had the parent links been cleared on detach, json_free would not
// have found an owner to splice the node out of, and the array would
// still have three elements.
REQUIRE(tree->array().size() == 2);
REQUIRE(tree->array()[0]->array()[0]->number() == 1);
REQUIRE(tree->array()[1]->array()[0]->number() == 5);
}
// The hash case is not a removal: json_free() of a hash value leaves the
// key in place with a JSON_NULL stand-in, so that detaching one half of a
// pair cannot disturb the key/value pairing of the entries around it. The
// entry is only erased once both halves are gone.
TEST_CASE("json_free of a hash value leaves a null placeholder", "[jsonpull][ownership]") {
json_pull_ptr jp = json_begin_string(R"({"keep": 1, "drop": [2, 3]})");
json_object_ptr tree = json_read_tree(jp);
REQUIRE(tree != nullptr);
REQUIRE(tree->entries().size() == 2);
json_object *drop = json_hash_get(tree, "drop");
REQUIRE(drop != nullptr);
REQUIRE(drop->type == JSON_ARRAY);
json_free(drop);
// drop is dangling now; do not dereference.
// The key survives, now paired with a null rather than the array.
REQUIRE(tree->entries().size() == 2);
json_object *after = json_hash_get(tree, "drop");
REQUIRE(after != nullptr);
REQUIRE(after->type == JSON_NULL);
// The neighbouring pair is untouched.
REQUIRE(json_hash_get(tree, "keep") != nullptr);
REQUIRE(json_hash_get(tree, "keep")->number() == 1);
}
// A \uXXXX escape can only name a code point up to U+FFFF, and U+FFFF
// itself used to fall through the `< 0xFFFF` test into the four-byte
// branch, which emitted the overlong sequence F0 8F BF BF. check_utf8()
// only validates continuation-byte structure, so that invalid UTF-8 was
// copied into tiles unnoticed.
TEST_CASE("jsonpull encodes U+FFFF as three bytes", "[jsonpull][utf8]") {
json_pull_ptr jp = json_begin_string("\"a\\uFFFFb\"");
json_object_ptr o = json_read_tree(jp);
REQUIRE(jp->error == nullptr);
REQUIRE(o != nullptr);
REQUIRE(o->type == JSON_STRING);
REQUIRE(o->string() == "a\xEF\xBF\xBF" "b");
REQUIRE(o->string() != "a\xF0\x8F\xBF\xBF" "b");
// The boundary below it, and a genuine supplementary code point built
// from a surrogate pair, both keep their existing encodings.
json_pull_ptr jp2 = json_begin_string("\"\\uFFFE\"");
json_object_ptr o2 = json_read_tree(jp2);
REQUIRE(o2 != nullptr);
REQUIRE(o2->string() == "\xEF\xBF\xBE");
json_pull_ptr jp3 = json_begin_string("\"\\uD83D\\uDC00\"");
json_object_ptr o3 = json_read_tree(jp3);
REQUIRE(o3 != nullptr);
REQUIRE(o3->string() == "\xF0\x9F\x90\x80");
}
TEST_CASE("Polygon cleaning drops a hole that no ring can parent", "[wagyu]") {
// Two mutually reversed self-intersecting rings whose union leaves a hole
// that wagyu's topology correction cannot assign to any surviving parent