Fix stack overflow converting deep values between specializations (#5723)

* Fix stack overflow converting deep values between specializations Constructing a basic_json from another specialization (json to ordered_json or back, also via get<ordered_json>()) converted every container with its range constructor, which calls the converting constructor for each element. The call stack therefore grew with every nesting level, and a value nested some 30,000 levels deep overflowed it. The conversion now bounds its descent the way the copy constructor does since #5387: the first 128 levels are converted exactly as before, and below that convert_iteratively() finishes the value with an explicit stack. It builds each container bottom-up from its converted elements with the container's range constructor, so member order and keys that become equal are handled as before, and it gives a value its type only once its container exists, so an exception leaves nothing behind that cannot be destroyed. Parents (JSON_DIAGNOSTICS) and positions (JSON_DIAGNOSTIC_POSITIONS) are set for every value. Converting a null value no longer resets its positions: the constructor assigned null to a value that already was null, which swapped in the positions of the temporary. Fixes #5650. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Explain why converting null keeps positions and why next is a reference Review feedback on #5723 (gregmarr): clarify in comments that the converting constructor has already copied the positions of val, which the null case keeps like every other case, and that next must be a reference into pending so that ++next advances the stored iterator. Comments only; no code change. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Refer to recursion_depth_limit() in the convert_structured() docs The comment still named nesting_depth_limit, which #5637 removed on develop in favor of detail::recursion_depth_limit(). Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Advance the pending iterator through pending.back() and shorten the null comment Signed-off-by: Niels Lohmann <mail@nlohmann.me> --------- Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-10-03 13:10:33 +00:00 · 2026-09-30 21:36:04 +02:00
parent 3b6ae43c53
commit 7d7055ec50
6 changed files with 735 additions and 94 deletions
@@ -479,6 +479,77 @@ TEST_CASE("deep copy uses the provided allocator")
    CHECK(copy == j);
 }

+namespace
+{
+// the number of constructions countdown_allocator lets happen, including the
+// one that fails; 0 means none ever fails
+std::size_t constructions_until_failure = 0;
+
+template<class T>
+struct countdown_allocator : std::allocator<T>
+{
+    using std::allocator<T>::allocator;
+
+    template<class U, class... Args>
+    void construct(U* p, Args&& ... args)
+    {
+        if (constructions_until_failure != 0 && --constructions_until_failure == 0)
+        {
+            throw std::bad_alloc();
+        }
+
+        ::new (static_cast<void*>(p)) U(std::forward<Args>(args)...);
+    }
+
+    template <class U>
+    struct rebind
+    {
+        using other = countdown_allocator<U>;
+    };
+};
+} // namespace
+
+TEST_CASE("converting a deeply nested value from another specialization fails cleanly (#5650)")
+{
+    using countdown_json = nlohmann::basic_json<std::map,
+          std::vector,
+          std::string,
+          bool,
+          std::int64_t,
+          std::uint64_t,
+          double,
+          countdown_allocator>;
+
+    // deeper than the 128 levels the converting constructor descends into, so
+    // that failures land on both sides of the bound - or, built with
+    // JSON_NO_THREAD_LOCAL, all in the iterative conversion
+    json j = {1, "two", {{"three", 3}}};
+    for (std::size_t i = 0; i < 150; ++i)
+    {
+        j = json{{"a", json::array({j, "sibling"})}};
+    }
+
+    // Fail every construction in turn. Each failure has to reach the caller,
+    // and everything built until then has to be destroyed cleanly.
+    std::size_t failures = 0;
+    for (std::size_t n = 1;; ++n)
+    {
+        constructions_until_failure = n;
+        try
+        {
+            const countdown_json converted = j;
+            constructions_until_failure = 0;
+            CHECK(converted.dump() == j.dump());
+            break;
+        }
+        catch (const std::bad_alloc&)
+        {
+            ++failures;
+        }
+    }
+    CHECK(failures > 0);
+}
+
 namespace
 {
 template<class T>
@@ -141,6 +141,58 @@ TEST_CASE("Better diagnostics with positions")
        check_objects(300);
    }

+    SECTION("converting keeps the positions of nested values (#5650)")
+    {
+        // Values nested deeper than the converting constructor's descent bound
+        // are converted without the call stack, on a path that has to carry the
+        // positions of every value over itself. Objects and arrays take turns,
+        // and the innermost value is null, which used to lose its positions.
+        const auto check_conversion = [](std::size_t depth)
+        {
+            CAPTURE(depth)
+
+            std::string text;
+            std::string closing;
+            for (std::size_t i = 0; i < depth; ++i)
+            {
+                text += (i % 2 == 0) ? "[12, " : R"({"b":1, "a":)";
+                closing += (i % 2 == 0) ? ']' : '}';
+            }
+            text += "null";
+            text.append(closing.rbegin(), closing.rend());
+
+            const json original = json::parse(text);
+            const nlohmann::ordered_json converted = original;
+
+            const json* o = &original;
+            const nlohmann::ordered_json* c = &converted;
+            for (std::size_t level = 0; level <= depth; ++level)
+            {
+                CAPTURE(level)
+                REQUIRE(c->start_pos() == o->start_pos());
+                REQUIRE(c->end_pos() == o->end_pos());
+
+                if (level < depth)
+                {
+                    // the number beside the value nested next
+                    const json& o_number = o->is_object() ? o->at("b") : o->at(0);
+                    const nlohmann::ordered_json& c_number = c->is_object() ? c->at("b") : c->at(0);
+                    REQUIRE(c_number.start_pos() == o_number.start_pos());
+                    REQUIRE(c_number.end_pos() == o_number.end_pos());
+
+                    o = o->is_object() ? &o->at("a") : &o->at(1);
+                    c = c->is_object() ? &c->at("a") : &c->at(1);
+                }
+            }
+        };
+
+        check_conversion(1);
+        check_conversion(127);
+        check_conversion(128);
+        check_conversion(129);
+        check_conversion(300);
+    }
+
    SECTION("JSON patch add to primitive parent (#4292)")
    {
        // the JSON Patch "add" target /foo/bar/baz has a string parent
@@ -341,6 +341,36 @@ TEST_CASE("Regression tests for extended diagnostics")
        }
    }

+    SECTION("Regression test for issue #5650 - converting keeps the parents of nested values")
+    {
+        // A value nested deeper than the converting constructor's descent bound
+        // is converted without the call stack. Every container that path creates
+        // has to have the parents of its children set, or the JSON Pointer in the
+        // diagnostic is cut short. Objects and arrays take turns.
+        const std::size_t pairs = 150;
+
+        json j = "not a number";
+        std::string pointer;
+        for (std::size_t i = 0; i < pairs; ++i)
+        {
+            j = json{{"a", json::array({j})}};
+            pointer += "/a/0";
+        }
+
+        const nlohmann::ordered_json converted = j;
+
+        const nlohmann::ordered_json* inner = &converted;
+        for (std::size_t i = 0; i < pairs; ++i)
+        {
+            inner = &inner->at("a").at(0);
+        }
+
+        std::string const expected = "[json.exception.type_error.302] (" + pointer + ") type must be number, but is string";
+        int i = 0;
+        CHECK_THROWS_WITH_AS(i = inner->get<int>(), expected.c_str(), nlohmann::ordered_json::type_error);
+        CHECK(i == 0);
+    }
+
    SECTION("Regression test for issue #5668 - wrong path for std::map/unordered_map with non-string keys")
    {
        // a map with non-string keys is read from an array of [key, value] arrays;
@@ -13,6 +13,7 @@ using nlohmann::json;

 #include <algorithm>
 #include <string>
+#include <vector>

 TEST_CASE("tests on very large JSONs")
 {
@@ -53,6 +54,24 @@ const json* innermost_value(const json& j, std::size_t& depth)
    return current;
 }

+// The text of a value nested depth levels deep around the number 0. Level i is
+// an array if pattern[i % pattern.size()] is '[', and otherwise an object with
+// the single member "a", which every object type enumerates in the same order.
+std::string nested_text(std::size_t depth, const std::string& pattern)
+{
+    std::string text;
+    std::string closing;
+    for (std::size_t i = 0; i < depth; ++i)
+    {
+        const bool array = pattern[i % pattern.size()] == '[';
+        text += array ? "[" : "{\"a\":";
+        closing += array ? ']' : '}';
+    }
+    text += '0';
+    text.append(closing.rbegin(), closing.rend());
+    return text;
+}
+
 } // namespace

 TEST_CASE("tests on deeply nested JSONs")
@@ -224,5 +243,114 @@ TEST_CASE("tests on deeply nested JSONs")
            CHECK(*innermost_value(j, unused) == 0);
        }
    }
+
+    SECTION("issue #5650 - stack overflow converting between specializations")
+    {
+        const std::vector<std::string> patterns = {"[", "{", "[{"};
+
+        SECTION("json to ordered_json")
+        {
+            for (const auto& pattern : patterns)
+            {
+                CAPTURE(pattern);
+                const std::string text = nested_text(depth, pattern);
+                const json j = json::parse(text);
+
+                const nlohmann::ordered_json converted = j;
+                CHECK(converted.dump() == text);
+            }
+        }
+
+        SECTION("ordered_json to json")
+        {
+            for (const auto& pattern : patterns)
+            {
+                CAPTURE(pattern);
+                const std::string text = nested_text(depth, pattern);
+                const nlohmann::ordered_json o = nlohmann::ordered_json::parse(text);
+
+                const json converted = o;
+                CHECK(converted.dump() == text);
+            }
+        }
+
+        SECTION("get<ordered_json>()")
+        {
+            for (const auto& pattern : patterns)
+            {
+                CAPTURE(pattern);
+                const std::string text = nested_text(depth, pattern);
+                const json j = json::parse(text);
+
+                CHECK(j.get<nlohmann::ordered_json>().dump() == text);
+            }
+        }
+
+        SECTION("depths around the bound of the recursive descent")
+        {
+            for (std::size_t d = 1; d <= 300; ++d)
+            {
+                CAPTURE(d);
+                for (const auto& pattern : patterns)
+                {
+                    CAPTURE(pattern);
+                    const std::string text = nested_text(d, pattern);
+                    const json j = json::parse(text);
+
+                    const nlohmann::ordered_json converted = j;
+                    CHECK(converted.dump() == text);
+                    const json back = converted;
+                    CHECK(back.dump() == text);
+                }
+            }
+        }
+
+        SECTION("values below the bound are converted as values above it")
+        {
+            // Bury a value below the bound, where it is converted without the
+            // call stack, and compare it with the same value converted on its
+            // own by the containers' range constructors. Its objects have
+            // members that the two object types enumerate in different orders.
+            const auto bury = [](nlohmann::ordered_json value)
+            {
+                for (std::size_t i = 0; i < 200; ++i)
+                {
+                    value = nlohmann::ordered_json::array({std::move(value)});
+                }
+                return value;
+            };
+            const auto dig = [](const json & value)
+            {
+                const json* current = &value;
+                for (std::size_t i = 0; i < 200; ++i)
+                {
+                    current = &current->at(0);
+                }
+                return current;
+            };
+
+            nlohmann::ordered_json value = nlohmann::ordered_json::object();
+            value["z"] = {1, -2, 3U, 4.5, true, nullptr, "six", nlohmann::ordered_json::binary({7, 8}, 9),
+                          nlohmann::ordered_json::binary({10}), nlohmann::ordered_json::array(), nlohmann::ordered_json::object()
+                         };
+            value["y"] = {{"x", {{"w", 1}, {"v", 2}}}, {"u", {3, {{"t", 4}, {"s", 5}}}}};
+            value["r"] = nlohmann::ordered_json::array({nlohmann::ordered_json(nlohmann::ordered_json::value_t::discarded)});
+
+            const json converted_above = value;
+            const json buried = bury(value);
+            const json& converted_below = *dig(buried);
+
+            CHECK(converted_below.dump() == converted_above.dump());
+            CHECK(converted_below.at("z").at(7).get_binary().subtype() == 9);
+            CHECK_FALSE(converted_below.at("z").at(8).get_binary().has_subtype());
+            CHECK(converted_below.at("r").at(0).is_discarded());
+
+            // a discarded value is never equal to anything, so compare the rest
+            value.erase("r");
+            const json without_discarded_above = value;
+            const json without_discarded_buried = bury(value);
+            CHECK(*dig(without_discarded_buried) == without_discarded_above);
+        }
+    }
 }