From 7d7055ec500a519b0497f1c0aeefa977dcbfc0ab Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 30 Sep 2026 21:36:04 +0200 Subject: [PATCH] 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()) 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 * 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 * 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 * Advance the pending iterator through pending.back() and shorten the null comment Signed-off-by: Niels Lohmann --------- Signed-off-by: Niels Lohmann --- include/nlohmann/json.hpp | 274 ++++++++++++++++++++---- single_include/nlohmann/json.hpp | 274 ++++++++++++++++++++---- tests/src/unit-allocator.cpp | 71 ++++++ tests/src/unit-diagnostic-positions.cpp | 52 +++++ tests/src/unit-diagnostics.cpp | 30 +++ tests/src/unit-large_json.cpp | 128 +++++++++++ 6 files changed, 735 insertions(+), 94 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index a82c1eb11..344c9209b 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -906,11 +906,11 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /*! @brief how many levels the operation going on in this thread has descended into - Copying a value and comparing two values share this count. The library never - nests one inside the other - copying a value does not compare one, and - comparing two values does not copy them - and where user code nests them - anyway, sharing the count only ends a descent sooner than it had to, which - costs a little speed and is never wrong. + Copying a value, converting one from another specialization, and comparing + two values share this count. The library never nests one of them inside + another - none of them does either of the other two on the way - and where + user code nests them anyway, sharing the count only ends a descent sooner + than it had to, which costs a little speed and is never wrong. A byte is enough: the count never exceeds the limit by more than the single level that notices the limit has been reached. @@ -1267,6 +1267,222 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec copy_iteratively(src); } + /*! + @brief convert the value @a val of another specialization into this null + value; @a val must be neither an object nor an array + + Converting such a value never descends, so both ways of converting an + object or an array (@ref convert_structured) leave their elements of this + kind to the converting constructor, which leaves them to this. + */ + template + void convert_leaf(const BasicJsonType& val) + { + using other_boolean_t = typename BasicJsonType::boolean_t; + using other_number_float_t = typename BasicJsonType::number_float_t; + using other_number_integer_t = typename BasicJsonType::number_integer_t; + using other_number_unsigned_t = typename BasicJsonType::number_unsigned_t; + using other_string_t = typename BasicJsonType::string_t; + using other_binary_t = typename BasicJsonType::binary_t; + + switch (val.type()) + { + case value_t::boolean: + JSONSerializer::to_json(*this, val.template get()); + break; + case value_t::number_float: + JSONSerializer::to_json(*this, val.template get()); + break; + case value_t::number_integer: + JSONSerializer::to_json(*this, val.template get()); + break; + case value_t::number_unsigned: + JSONSerializer::to_json(*this, val.template get()); + break; + case value_t::string: + JSONSerializer::to_json(*this, val.template get_ref()); + break; + case value_t::binary: + JSONSerializer::to_json(*this, val.template get_ref()); + break; + case value_t::null: + // m_data.m_type is already value_t::null + break; + case value_t::discarded: + m_data.m_type = value_t::discarded; + break; + case value_t::object: // LCOV_EXCL_LINE + case value_t::array: // LCOV_EXCL_LINE + default: // LCOV_EXCL_LINE + JSON_ASSERT(false); // NOLINT(cert-dcl03-c,hicpp-static-assert,misc-static-assert) LCOV_EXCL_LINE + } + } + + /// scratch space for the converted elements of the arrays that + /// @ref convert_iteratively has yet to create + using convert_scratch_t = std::vector>; + + /*! + @brief create the object or array @a val converted into this null value + + Its converted elements are the last `val.size()` entries of @a elements (an + array) or of @a members (an object); they are moved into the container in + one go and then removed. + */ + template + void convert_level(const BasicJsonType& val, convert_scratch_t& elements, copy_scratch_t& members) + { + if (val.is_object()) + { + const auto first = members.end() - static_cast(val.size()); + m_data.m_value.object = create(std::make_move_iterator(first), + std::make_move_iterator(members.end())); + // only now that the object exists may this stop being a null value + m_data.m_type = value_t::object; + members.erase(first, members.end()); + } + else + { + const auto first = elements.end() - static_cast(val.size()); + m_data.m_value.array = create(std::make_move_iterator(first), + std::make_move_iterator(elements.end())); + // only now that the array exists may this stop being a null value + m_data.m_type = value_t::array; + elements.erase(first, elements.end()); + } + + set_parents(); + } + + /*! + @brief convert the object or array @a val of another specialization into + this null value without recursing + + The containers whose conversion has begun are kept on an explicit stack + rather than on the call stack. Unlike @ref copy_iteratively, this builds + every container from the bottom up: all its elements are converted first, + and the container is then created from them in one go, the way the range + constructor that converts the levels above the bound does. The two object + types need not enumerate their members in the same order, so the members + could not be paired up by position anyway, and building from a range keeps + what the range constructor does with keys that become equal on conversion. + + Every value is complete before it is handed on, and a container gets its + type only once it exists, so whatever throws, every value left behind can + be destroyed. + */ + template + void convert_iteratively(const BasicJsonType& val) + { + using other_const_iterator = typename BasicJsonType::const_iterator; + + // the containers whose conversion has begun, innermost last, each with + // its element to convert next + std::vector> pending; + + // the converted elements of the pending arrays and the converted + // members of the pending objects, those of the innermost one last + convert_scratch_t elements; + copy_scratch_t members; + + pending.emplace_back(&val, val.cbegin()); + + for (;;) + { + const BasicJsonType& container = *pending.back().first; + // a copy, as descending below can reallocate pending; the + // iterator kept in pending is only advanced through pending.back() + const other_const_iterator next = pending.back().second; + + if (next != container.cend()) + { + if (next->is_structured()) + { + // convert its elements first; next stays where it is until + // the converted container is handed back to this one + pending.emplace_back(&*next, next->cbegin()); + continue; + } + + // the converting constructor does not descend into this value + if (container.is_object()) + { + members.emplace_back(next.key(), *next); + } + else + { + elements.emplace_back(*next); + } + ++pending.back().second; + continue; + } + + // all elements of the container are converted: create it + pending.pop_back(); + + if (pending.empty()) + { + convert_level(container, elements, members); + return; + } + + basic_json converted; + converted.convert_level(container, elements, members); +#if JSON_DIAGNOSTIC_POSITIONS + converted.start_position = container.start_pos(); + converted.end_position = container.end_pos(); +#endif + + // hand it to the container it is an element of + if (pending.back().first->is_object()) + { + members.emplace_back(pending.back().second.key(), std::move(converted)); + } + else + { + elements.push_back(std::move(converted)); + } + ++pending.back().second; + } + } + + /*! + @brief convert the object or array @a val of another specialization into + this null value + + Converting a container converts its elements, so a value nested deeply + enough used to exhaust the call stack. The descent is bounded here as in + @ref copy_structured: the first `detail::recursion_depth_limit()` levels + are converted by the containers' range constructors, just as they always were, + and anything below that is converted without the call stack by + @ref convert_iteratively. + + @sa https://github.com/nlohmann/json/issues/5650 + */ + template + void convert_structured(const BasicJsonType& val) + { + const nesting_depth_guard guard; + + if (JSON_HEDLEY_LIKELY(guard.okay())) + { + // every element comes back to the converting constructor + if (val.is_object()) + { + using other_object_t = typename BasicJsonType::object_t; + JSONSerializer::to_json(*this, val.template get_ref()); + } + else + { + using other_array_t = typename BasicJsonType::array_t; + JSONSerializer::to_json(*this, val.template get_ref()); + } + return; + } + + convert_iteratively(val); + } + /// the result of comparing two values, including values that cannot be /// ordered at all, such as a discarded value or a NaN @@ -1622,49 +1838,13 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec end_position(val.end_pos()) #endif { - using other_boolean_t = typename BasicJsonType::boolean_t; - using other_number_float_t = typename BasicJsonType::number_float_t; - using other_number_integer_t = typename BasicJsonType::number_integer_t; - using other_number_unsigned_t = typename BasicJsonType::number_unsigned_t; - using other_string_t = typename BasicJsonType::string_t; - using other_object_t = typename BasicJsonType::object_t; - using other_array_t = typename BasicJsonType::array_t; - using other_binary_t = typename BasicJsonType::binary_t; - - switch (val.type()) + if (val.is_structured()) { - case value_t::boolean: - JSONSerializer::to_json(*this, val.template get()); - break; - case value_t::number_float: - JSONSerializer::to_json(*this, val.template get()); - break; - case value_t::number_integer: - JSONSerializer::to_json(*this, val.template get()); - break; - case value_t::number_unsigned: - JSONSerializer::to_json(*this, val.template get()); - break; - case value_t::string: - JSONSerializer::to_json(*this, val.template get_ref()); - break; - case value_t::object: - JSONSerializer::to_json(*this, val.template get_ref()); - break; - case value_t::array: - JSONSerializer::to_json(*this, val.template get_ref()); - break; - case value_t::binary: - JSONSerializer::to_json(*this, val.template get_ref()); - break; - case value_t::null: - *this = nullptr; - break; - case value_t::discarded: - m_data.m_type = value_t::discarded; - break; - default: // LCOV_EXCL_LINE - JSON_ASSERT(false); // NOLINT(cert-dcl03-c,hicpp-static-assert,misc-static-assert) LCOV_EXCL_LINE + convert_structured(val); + } + else + { + convert_leaf(val); } JSON_ASSERT(m_data.m_type == val.type()); diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index d6d2ef347..1fa63f344 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -27850,11 +27850,11 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /*! @brief how many levels the operation going on in this thread has descended into - Copying a value and comparing two values share this count. The library never - nests one inside the other - copying a value does not compare one, and - comparing two values does not copy them - and where user code nests them - anyway, sharing the count only ends a descent sooner than it had to, which - costs a little speed and is never wrong. + Copying a value, converting one from another specialization, and comparing + two values share this count. The library never nests one of them inside + another - none of them does either of the other two on the way - and where + user code nests them anyway, sharing the count only ends a descent sooner + than it had to, which costs a little speed and is never wrong. A byte is enough: the count never exceeds the limit by more than the single level that notices the limit has been reached. @@ -28211,6 +28211,222 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec copy_iteratively(src); } + /*! + @brief convert the value @a val of another specialization into this null + value; @a val must be neither an object nor an array + + Converting such a value never descends, so both ways of converting an + object or an array (@ref convert_structured) leave their elements of this + kind to the converting constructor, which leaves them to this. + */ + template + void convert_leaf(const BasicJsonType& val) + { + using other_boolean_t = typename BasicJsonType::boolean_t; + using other_number_float_t = typename BasicJsonType::number_float_t; + using other_number_integer_t = typename BasicJsonType::number_integer_t; + using other_number_unsigned_t = typename BasicJsonType::number_unsigned_t; + using other_string_t = typename BasicJsonType::string_t; + using other_binary_t = typename BasicJsonType::binary_t; + + switch (val.type()) + { + case value_t::boolean: + JSONSerializer::to_json(*this, val.template get()); + break; + case value_t::number_float: + JSONSerializer::to_json(*this, val.template get()); + break; + case value_t::number_integer: + JSONSerializer::to_json(*this, val.template get()); + break; + case value_t::number_unsigned: + JSONSerializer::to_json(*this, val.template get()); + break; + case value_t::string: + JSONSerializer::to_json(*this, val.template get_ref()); + break; + case value_t::binary: + JSONSerializer::to_json(*this, val.template get_ref()); + break; + case value_t::null: + // m_data.m_type is already value_t::null + break; + case value_t::discarded: + m_data.m_type = value_t::discarded; + break; + case value_t::object: // LCOV_EXCL_LINE + case value_t::array: // LCOV_EXCL_LINE + default: // LCOV_EXCL_LINE + JSON_ASSERT(false); // NOLINT(cert-dcl03-c,hicpp-static-assert,misc-static-assert) LCOV_EXCL_LINE + } + } + + /// scratch space for the converted elements of the arrays that + /// @ref convert_iteratively has yet to create + using convert_scratch_t = std::vector>; + + /*! + @brief create the object or array @a val converted into this null value + + Its converted elements are the last `val.size()` entries of @a elements (an + array) or of @a members (an object); they are moved into the container in + one go and then removed. + */ + template + void convert_level(const BasicJsonType& val, convert_scratch_t& elements, copy_scratch_t& members) + { + if (val.is_object()) + { + const auto first = members.end() - static_cast(val.size()); + m_data.m_value.object = create(std::make_move_iterator(first), + std::make_move_iterator(members.end())); + // only now that the object exists may this stop being a null value + m_data.m_type = value_t::object; + members.erase(first, members.end()); + } + else + { + const auto first = elements.end() - static_cast(val.size()); + m_data.m_value.array = create(std::make_move_iterator(first), + std::make_move_iterator(elements.end())); + // only now that the array exists may this stop being a null value + m_data.m_type = value_t::array; + elements.erase(first, elements.end()); + } + + set_parents(); + } + + /*! + @brief convert the object or array @a val of another specialization into + this null value without recursing + + The containers whose conversion has begun are kept on an explicit stack + rather than on the call stack. Unlike @ref copy_iteratively, this builds + every container from the bottom up: all its elements are converted first, + and the container is then created from them in one go, the way the range + constructor that converts the levels above the bound does. The two object + types need not enumerate their members in the same order, so the members + could not be paired up by position anyway, and building from a range keeps + what the range constructor does with keys that become equal on conversion. + + Every value is complete before it is handed on, and a container gets its + type only once it exists, so whatever throws, every value left behind can + be destroyed. + */ + template + void convert_iteratively(const BasicJsonType& val) + { + using other_const_iterator = typename BasicJsonType::const_iterator; + + // the containers whose conversion has begun, innermost last, each with + // its element to convert next + std::vector> pending; + + // the converted elements of the pending arrays and the converted + // members of the pending objects, those of the innermost one last + convert_scratch_t elements; + copy_scratch_t members; + + pending.emplace_back(&val, val.cbegin()); + + for (;;) + { + const BasicJsonType& container = *pending.back().first; + // a copy, as descending below can reallocate pending; the + // iterator kept in pending is only advanced through pending.back() + const other_const_iterator next = pending.back().second; + + if (next != container.cend()) + { + if (next->is_structured()) + { + // convert its elements first; next stays where it is until + // the converted container is handed back to this one + pending.emplace_back(&*next, next->cbegin()); + continue; + } + + // the converting constructor does not descend into this value + if (container.is_object()) + { + members.emplace_back(next.key(), *next); + } + else + { + elements.emplace_back(*next); + } + ++pending.back().second; + continue; + } + + // all elements of the container are converted: create it + pending.pop_back(); + + if (pending.empty()) + { + convert_level(container, elements, members); + return; + } + + basic_json converted; + converted.convert_level(container, elements, members); +#if JSON_DIAGNOSTIC_POSITIONS + converted.start_position = container.start_pos(); + converted.end_position = container.end_pos(); +#endif + + // hand it to the container it is an element of + if (pending.back().first->is_object()) + { + members.emplace_back(pending.back().second.key(), std::move(converted)); + } + else + { + elements.push_back(std::move(converted)); + } + ++pending.back().second; + } + } + + /*! + @brief convert the object or array @a val of another specialization into + this null value + + Converting a container converts its elements, so a value nested deeply + enough used to exhaust the call stack. The descent is bounded here as in + @ref copy_structured: the first `detail::recursion_depth_limit()` levels + are converted by the containers' range constructors, just as they always were, + and anything below that is converted without the call stack by + @ref convert_iteratively. + + @sa https://github.com/nlohmann/json/issues/5650 + */ + template + void convert_structured(const BasicJsonType& val) + { + const nesting_depth_guard guard; + + if (JSON_HEDLEY_LIKELY(guard.okay())) + { + // every element comes back to the converting constructor + if (val.is_object()) + { + using other_object_t = typename BasicJsonType::object_t; + JSONSerializer::to_json(*this, val.template get_ref()); + } + else + { + using other_array_t = typename BasicJsonType::array_t; + JSONSerializer::to_json(*this, val.template get_ref()); + } + return; + } + + convert_iteratively(val); + } + /// the result of comparing two values, including values that cannot be /// ordered at all, such as a discarded value or a NaN @@ -28566,49 +28782,13 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec end_position(val.end_pos()) #endif { - using other_boolean_t = typename BasicJsonType::boolean_t; - using other_number_float_t = typename BasicJsonType::number_float_t; - using other_number_integer_t = typename BasicJsonType::number_integer_t; - using other_number_unsigned_t = typename BasicJsonType::number_unsigned_t; - using other_string_t = typename BasicJsonType::string_t; - using other_object_t = typename BasicJsonType::object_t; - using other_array_t = typename BasicJsonType::array_t; - using other_binary_t = typename BasicJsonType::binary_t; - - switch (val.type()) + if (val.is_structured()) { - case value_t::boolean: - JSONSerializer::to_json(*this, val.template get()); - break; - case value_t::number_float: - JSONSerializer::to_json(*this, val.template get()); - break; - case value_t::number_integer: - JSONSerializer::to_json(*this, val.template get()); - break; - case value_t::number_unsigned: - JSONSerializer::to_json(*this, val.template get()); - break; - case value_t::string: - JSONSerializer::to_json(*this, val.template get_ref()); - break; - case value_t::object: - JSONSerializer::to_json(*this, val.template get_ref()); - break; - case value_t::array: - JSONSerializer::to_json(*this, val.template get_ref()); - break; - case value_t::binary: - JSONSerializer::to_json(*this, val.template get_ref()); - break; - case value_t::null: - *this = nullptr; - break; - case value_t::discarded: - m_data.m_type = value_t::discarded; - break; - default: // LCOV_EXCL_LINE - JSON_ASSERT(false); // NOLINT(cert-dcl03-c,hicpp-static-assert,misc-static-assert) LCOV_EXCL_LINE + convert_structured(val); + } + else + { + convert_leaf(val); } JSON_ASSERT(m_data.m_type == val.type()); diff --git a/tests/src/unit-allocator.cpp b/tests/src/unit-allocator.cpp index ce498558d..151c268d7 100644 --- a/tests/src/unit-allocator.cpp +++ b/tests/src/unit-allocator.cpp @@ -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 +struct countdown_allocator : std::allocator +{ + using std::allocator::allocator; + + template + void construct(U* p, Args&& ... args) + { + if (constructions_until_failure != 0 && --constructions_until_failure == 0) + { + throw std::bad_alloc(); + } + + ::new (static_cast(p)) U(std::forward(args)...); + } + + template + struct rebind + { + using other = countdown_allocator; + }; +}; +} // namespace + +TEST_CASE("converting a deeply nested value from another specialization fails cleanly (#5650)") +{ + using countdown_json = nlohmann::basic_json; + + // 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 diff --git a/tests/src/unit-diagnostic-positions.cpp b/tests/src/unit-diagnostic-positions.cpp index 5326094c7..f5c6bc648 100644 --- a/tests/src/unit-diagnostic-positions.cpp +++ b/tests/src/unit-diagnostic-positions.cpp @@ -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 diff --git a/tests/src/unit-diagnostics.cpp b/tests/src/unit-diagnostics.cpp index b3778e802..ee7360eb1 100644 --- a/tests/src/unit-diagnostics.cpp +++ b/tests/src/unit-diagnostics.cpp @@ -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(), 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; diff --git a/tests/src/unit-large_json.cpp b/tests/src/unit-large_json.cpp index 8204ed9b6..97f665848 100644 --- a/tests/src/unit-large_json.cpp +++ b/tests/src/unit-large_json.cpp @@ -13,6 +13,7 @@ using nlohmann::json; #include #include +#include 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 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()") + { + for (const auto& pattern : patterns) + { + CAPTURE(pattern); + const std::string text = nested_text(depth, pattern); + const json j = json::parse(text); + + CHECK(j.get().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 = ¤t->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); + } + } }