diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index f2fd5d1a7..ed66b058a 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -3612,17 +3612,49 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec JSON_THROW(type_error::create(312, detail::concat("cannot use update() with ", first.m_object->type_name()), first.m_object)); } + update_members(first, last, merge_objects, 0); + } + + JSON_PRIVATE_UNLESS_TESTED: + /// the number of nested objects @ref update and @ref merge_patch descend + /// into before handing over to their iterative versions + static constexpr std::size_t merge_depth_limit() noexcept + { + return 128; + } + + private: + /*! + @brief the members loop of @ref update, for this object and range + + Merging a nested object calls this function again, once per nesting + level, so a value nested deeply enough used to exhaust the call stack and + terminate the process. The descent is bounded here: once @ref + merge_depth_limit levels have been entered, @ref update_members_iteratively + merges what is left without the call stack. + + @param[in] depth nesting level of this object, counted from the object + @ref update was called on + */ + void update_members(const const_iterator& first, const const_iterator& last, const bool merge_objects, const std::size_t depth) + { + if (JSON_HEDLEY_UNLIKELY(depth >= merge_depth_limit())) + { + update_members_iteratively(first, last); + return; + } + for (auto it = first; it != last; ++it) { if (merge_objects && it.value().is_object()) { - auto it2 = m_data.m_value.object->find(it.key()); + const auto it2 = m_data.m_value.object->find(it.key()); // Only recurse when the existing value is itself an object. // Otherwise overwrite, matching the documented "all other values // are overwritten as usual" behavior (see #5402). if (it2 != m_data.m_value.object->end() && it2->second.is_object()) { - it2->second.update(it.value(), true); + it2->second.update_members(it.value().cbegin(), it.value().cend(), true, depth + 1); #if JSON_DIAGNOSTICS it2->second.set_parents(); #endif @@ -3636,6 +3668,69 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } } + /*! + @brief merge @a first to @a last into this object without the call stack + + Does the same as @ref update_members with `merge_objects` set, keeping the + objects whose merge was interrupted by a nested one on an explicit stack + instead of descending into them. A nested object is still merged + completely before the next member, in the same order as the recursive + version. Only reached for values nested deeper than @ref merge_depth_limit. + */ + void update_members_iteratively(const_iterator first, const_iterator last) + { + struct update_frame + { + basic_json* target; + const_iterator position; + const_iterator last; + }; + std::vector stack; + + basic_json* target = this; + while (true) + { + if (first == last) + { + if (stack.empty()) + { + break; + } + + // a nested object is merged: continue with its parent +#if JSON_DIAGNOSTICS + target->set_parents(); +#endif + target = stack.back().target; + first = stack.back().position; + last = stack.back().last; + stack.pop_back(); + continue; + } + + if (first.value().is_object()) + { + const auto it2 = target->m_data.m_value.object->find(first.key()); + if (it2 != target->m_data.m_value.object->end() && it2->second.is_object()) + { + const basic_json& source = first.value(); + ++first; + stack.push_back({target, first, last}); + target = &it2->second; + first = source.cbegin(); + last = source.cend(); + continue; + } + } + target->m_data.m_value.object->operator[](first.key()) = first.value(); +#if JSON_DIAGNOSTICS + target->m_data.m_value.object->operator[](first.key()).m_parent = target; +#endif + ++first; + } + } + + public: /// @brief exchanges the values /// @sa https://json.nlohmann.me/api/basic_json/swap/ void swap(reference other) noexcept ( @@ -5530,9 +5625,30 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @brief applies a JSON Merge Patch /// @sa https://json.nlohmann.me/api/basic_json/merge_patch/ void merge_patch(const basic_json& apply_patch) + { + apply_merge_patch(apply_patch, 0); + } + + private: + /*! + @brief @ref merge_patch, for a patch at nesting level @a depth + + Applying a nested object calls this function again, once per nesting + level, so a patch nested deeply enough used to exhaust the call stack and + terminate the process. The descent is bounded here: once @ref + merge_depth_limit levels have been entered, @ref merge_patch_iteratively + applies what is left without the call stack. + */ + void apply_merge_patch(const basic_json& apply_patch, const std::size_t depth) { if (apply_patch.is_object()) { + if (JSON_HEDLEY_UNLIKELY(depth >= merge_depth_limit())) + { + merge_patch_iteratively(apply_patch); + return; + } + if (!is_object()) { *this = object(); @@ -5545,7 +5661,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } else { - operator[](it.key()).merge_patch(it.value()); + operator[](it.key()).apply_merge_patch(it.value(), depth + 1); } } } @@ -5555,6 +5671,67 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } } + /*! + @brief apply @a apply_patch to this value without the call stack + + Does the same as @ref merge_patch, keeping the objects being patched on an + explicit stack instead of descending into them. A nested object is still + patched completely before the next member, in the same order as the + recursive version. Only reached for patches nested deeper than @ref + merge_depth_limit. + */ + void merge_patch_iteratively(const basic_json& apply_patch) + { + struct merge_frame + { + basic_json* target; + const_iterator position; + const_iterator last; + }; + std::vector stack; + + // patch `target` with `patch`, or start patching it member by member + const auto apply = [&stack](basic_json & target, const basic_json & patch) + { + if (patch.is_object()) + { + if (!target.is_object()) + { + target = basic_json::object(); + } + stack.push_back({&target, patch.cbegin(), patch.cend()}); + } + else + { + target = patch; + } + }; + + apply(*this, apply_patch); + while (!stack.empty()) + { + merge_frame& frame = stack.back(); + if (frame.position == frame.last) + { + stack.pop_back(); + continue; + } + + const const_iterator member = frame.position; + ++frame.position; + if (member.value().is_null()) + { + frame.target->erase(member.key()); + } + else + { + // may push, which invalidates `frame` + apply(frame.target->operator[](member.key()), member.value()); + } + } + } + + public: /// @} }; diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index e3d11a2b1..0fd8c9d53 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -27971,17 +27971,49 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec JSON_THROW(type_error::create(312, detail::concat("cannot use update() with ", first.m_object->type_name()), first.m_object)); } + update_members(first, last, merge_objects, 0); + } + + JSON_PRIVATE_UNLESS_TESTED: + /// the number of nested objects @ref update and @ref merge_patch descend + /// into before handing over to their iterative versions + static constexpr std::size_t merge_depth_limit() noexcept + { + return 128; + } + + private: + /*! + @brief the members loop of @ref update, for this object and range + + Merging a nested object calls this function again, once per nesting + level, so a value nested deeply enough used to exhaust the call stack and + terminate the process. The descent is bounded here: once @ref + merge_depth_limit levels have been entered, @ref update_members_iteratively + merges what is left without the call stack. + + @param[in] depth nesting level of this object, counted from the object + @ref update was called on + */ + void update_members(const const_iterator& first, const const_iterator& last, const bool merge_objects, const std::size_t depth) + { + if (JSON_HEDLEY_UNLIKELY(depth >= merge_depth_limit())) + { + update_members_iteratively(first, last); + return; + } + for (auto it = first; it != last; ++it) { if (merge_objects && it.value().is_object()) { - auto it2 = m_data.m_value.object->find(it.key()); + const auto it2 = m_data.m_value.object->find(it.key()); // Only recurse when the existing value is itself an object. // Otherwise overwrite, matching the documented "all other values // are overwritten as usual" behavior (see #5402). if (it2 != m_data.m_value.object->end() && it2->second.is_object()) { - it2->second.update(it.value(), true); + it2->second.update_members(it.value().cbegin(), it.value().cend(), true, depth + 1); #if JSON_DIAGNOSTICS it2->second.set_parents(); #endif @@ -27995,6 +28027,69 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } } + /*! + @brief merge @a first to @a last into this object without the call stack + + Does the same as @ref update_members with `merge_objects` set, keeping the + objects whose merge was interrupted by a nested one on an explicit stack + instead of descending into them. A nested object is still merged + completely before the next member, in the same order as the recursive + version. Only reached for values nested deeper than @ref merge_depth_limit. + */ + void update_members_iteratively(const_iterator first, const_iterator last) + { + struct update_frame + { + basic_json* target; + const_iterator position; + const_iterator last; + }; + std::vector stack; + + basic_json* target = this; + while (true) + { + if (first == last) + { + if (stack.empty()) + { + break; + } + + // a nested object is merged: continue with its parent +#if JSON_DIAGNOSTICS + target->set_parents(); +#endif + target = stack.back().target; + first = stack.back().position; + last = stack.back().last; + stack.pop_back(); + continue; + } + + if (first.value().is_object()) + { + const auto it2 = target->m_data.m_value.object->find(first.key()); + if (it2 != target->m_data.m_value.object->end() && it2->second.is_object()) + { + const basic_json& source = first.value(); + ++first; + stack.push_back({target, first, last}); + target = &it2->second; + first = source.cbegin(); + last = source.cend(); + continue; + } + } + target->m_data.m_value.object->operator[](first.key()) = first.value(); +#if JSON_DIAGNOSTICS + target->m_data.m_value.object->operator[](first.key()).m_parent = target; +#endif + ++first; + } + } + + public: /// @brief exchanges the values /// @sa https://json.nlohmann.me/api/basic_json/swap/ void swap(reference other) noexcept ( @@ -29889,9 +29984,30 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @brief applies a JSON Merge Patch /// @sa https://json.nlohmann.me/api/basic_json/merge_patch/ void merge_patch(const basic_json& apply_patch) + { + apply_merge_patch(apply_patch, 0); + } + + private: + /*! + @brief @ref merge_patch, for a patch at nesting level @a depth + + Applying a nested object calls this function again, once per nesting + level, so a patch nested deeply enough used to exhaust the call stack and + terminate the process. The descent is bounded here: once @ref + merge_depth_limit levels have been entered, @ref merge_patch_iteratively + applies what is left without the call stack. + */ + void apply_merge_patch(const basic_json& apply_patch, const std::size_t depth) { if (apply_patch.is_object()) { + if (JSON_HEDLEY_UNLIKELY(depth >= merge_depth_limit())) + { + merge_patch_iteratively(apply_patch); + return; + } + if (!is_object()) { *this = object(); @@ -29904,7 +30020,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } else { - operator[](it.key()).merge_patch(it.value()); + operator[](it.key()).apply_merge_patch(it.value(), depth + 1); } } } @@ -29914,6 +30030,67 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } } + /*! + @brief apply @a apply_patch to this value without the call stack + + Does the same as @ref merge_patch, keeping the objects being patched on an + explicit stack instead of descending into them. A nested object is still + patched completely before the next member, in the same order as the + recursive version. Only reached for patches nested deeper than @ref + merge_depth_limit. + */ + void merge_patch_iteratively(const basic_json& apply_patch) + { + struct merge_frame + { + basic_json* target; + const_iterator position; + const_iterator last; + }; + std::vector stack; + + // patch `target` with `patch`, or start patching it member by member + const auto apply = [&stack](basic_json & target, const basic_json & patch) + { + if (patch.is_object()) + { + if (!target.is_object()) + { + target = basic_json::object(); + } + stack.push_back({&target, patch.cbegin(), patch.cend()}); + } + else + { + target = patch; + } + }; + + apply(*this, apply_patch); + while (!stack.empty()) + { + merge_frame& frame = stack.back(); + if (frame.position == frame.last) + { + stack.pop_back(); + continue; + } + + const const_iterator member = frame.position; + ++frame.position; + if (member.value().is_null()) + { + frame.target->erase(member.key()); + } + else + { + // may push, which invalidates `frame` + apply(frame.target->operator[](member.key()), member.value()); + } + } + } + + public: /// @} }; diff --git a/tests/src/unit-diagnostics.cpp b/tests/src/unit-diagnostics.cpp index 135ecf9d6..eb4ded5a3 100644 --- a/tests/src/unit-diagnostics.cpp +++ b/tests/src/unit-diagnostics.cpp @@ -306,3 +306,52 @@ TEST_CASE("Regression tests for extended diagnostics") } } +TEST_CASE("Better diagnostics past the descent bound of update() and merge_patch()") +{ + // Both merge objects nested more than basic_json::merge_depth_limit() + // (128) levels deep without recursing; the values they add or replace + // there must still know their parents. + const std::size_t depth = 200; + std::string target_text; + std::string patch_text; + std::string path; + for (std::size_t i = 0; i < depth; ++i) + { + target_text += "{\"a\":"; + patch_text += "{\"a\":"; + path += "/a"; + } + target_text += "{\"x\":1}" + std::string(depth, '}'); + patch_text += "{\"y\":2}" + std::string(depth, '}'); + const std::string expected_x = "[json.exception.type_error.304] (" + path + "/x) cannot use at() with number"; + const std::string expected_y = "[json.exception.type_error.304] (" + path + "/y) cannot use at() with number"; + + SECTION("update()") + { + json j = json::parse(target_text); + j.update(json::parse(patch_text), true); + + // walk down through const references, which leave m_parent alone + const json* p = &j; + for (std::size_t i = 0; i < depth; ++i) + { + p = &p->at("a"); + } + CHECK_THROWS_WITH_AS(p->at("x").at(0), expected_x.c_str(), json::type_error); + CHECK_THROWS_WITH_AS(p->at("y").at(0), expected_y.c_str(), json::type_error); + } + + SECTION("merge_patch()") + { + json j = json::parse(target_text); + j.merge_patch(json::parse(patch_text)); + + const json* p = &j; + for (std::size_t i = 0; i < depth; ++i) + { + p = &p->at("a"); + } + CHECK_THROWS_WITH_AS(p->at("x").at(0), expected_x.c_str(), json::type_error); + CHECK_THROWS_WITH_AS(p->at("y").at(0), expected_y.c_str(), json::type_error); + } +} diff --git a/tests/src/unit-merge_patch.cpp b/tests/src/unit-merge_patch.cpp index f02a1e991..7155932be 100644 --- a/tests/src/unit-merge_patch.cpp +++ b/tests/src/unit-merge_patch.cpp @@ -14,6 +14,60 @@ using nlohmann::json; using namespace nlohmann::literals; // NOLINT(google-build-using-namespace) #endif +#include + +namespace +{ +// RFC 7396's MergePatch, written recursively as in the RFC; only usable on +// values nested a few hundred levels deep +void reference_merge_patch(json& target, const json& patch) +{ + if (!patch.is_object()) + { + target = patch; + return; + } + if (!target.is_object()) + { + target = json::object(); + } + for (auto it = patch.begin(); it != patch.end(); ++it) + { + if (it.value().is_null()) + { + target.erase(it.key()); + } + else + { + reference_merge_patch(target[it.key()], it.value()); + } + } +} + +// objects nested `depth` levels deep under the key "a", with members that +// differ by `variant` on the way down +std::string nested_objects(const std::size_t depth, const int variant) +{ + std::string text; + for (std::size_t i = 0; i < depth; ++i) + { + text += "{"; + if ((i + static_cast(variant)) % 3 == 0) + { + text += "\"s" + std::to_string(variant) + "\":" + std::to_string(i) + ","; + } + if (variant == 2 && i % 5 == 0) + { + text += "\"s0\":null,"; + } + text += "\"a\":"; + } + text += variant == 1 ? "{\"x\":1,\"y\":null}" : "{\"y\":2}"; + text.append(depth, '}'); + return text; +} +} // namespace + TEST_CASE("JSON Merge Patch") { SECTION("examples from RFC 7396") @@ -242,3 +296,52 @@ TEST_CASE("JSON Merge Patch") } } } + +TEST_CASE("JSON Merge Patch on deeply nested values") +{ + SECTION("patching past the descent bound gives the same result") + { + // every depth on either side of where the iterative version takes + // over (basic_json::merge_depth_limit(), 128) + for (std::size_t depth = 0; depth <= 300; ++depth) + { + CAPTURE(depth); + for (int variant = 0; variant < 3; ++variant) + { + CAPTURE(variant); + const json patch = json::parse(nested_objects(depth, variant)); + + json result = json::parse(nested_objects(depth, (variant + 1) % 3)); + json expected = result; + result.merge_patch(patch); + reference_merge_patch(expected, patch); + CHECK(result == expected); + + // a target that is not an object, and an empty one + json from_null; + from_null.merge_patch(patch); + json expected_from_null; + reference_merge_patch(expected_from_null, patch); + CHECK(from_null == expected_from_null); + } + } + } + + SECTION("patches nested too deeply for the call stack (#5393)") + { + // applying a patch used to recurse once per nesting level. The result + // is only walked, never copied or compared, since those recurse too. + const std::size_t depth = 100000; + json target = json::parse(nested_objects(depth, 0)); + target.merge_patch(json::parse(nested_objects(depth, 1))); + + const json* p = ⌖ + for (std::size_t i = 0; i < depth; ++i) + { + p = &p->at("a"); + } + // {"y":2} patched with {"x":1,"y":null} + CHECK(p->size() == 1); + CHECK(p->at("x") == 1); + } +} diff --git a/tests/src/unit-modifiers.cpp b/tests/src/unit-modifiers.cpp index 369162772..6693b3189 100644 --- a/tests/src/unit-modifiers.cpp +++ b/tests/src/unit-modifiers.cpp @@ -11,6 +11,53 @@ #include using nlohmann::json; +#include + +namespace +{ +// update(source, true) as documented, written recursively; only usable on +// values nested a few hundred levels deep +void reference_update(json& target, const json& source) +{ + for (auto it = source.begin(); it != source.end(); ++it) + { + const auto existing = target.find(it.key()); + if (it.value().is_object() && existing != target.end() && existing->is_object()) + { + reference_update(*existing, it.value()); + } + else + { + target[it.key()] = it.value(); + } + } +} + +// objects nested `depth` levels deep under the key "a", with members that +// differ by `variant` on the way down +std::string nested_objects(const std::size_t depth, const int variant) +{ + std::string text; + for (std::size_t i = 0; i < depth; ++i) + { + text += "{"; + if ((i + static_cast(variant)) % 3 == 0) + { + text += "\"s" + std::to_string(variant) + "\":" + std::to_string(i) + ","; + } + if (variant == 2 && i % 5 == 0) + { + // an object replacing a primitive, which is not merged + text += "\"s0\":{\"o\":1},"; + } + text += "\"a\":"; + } + text += variant == 1 ? "{\"x\":1}" : "{\"y\":2}"; + text.append(depth, '}'); + return text; +} +} // namespace + TEST_CASE("modifiers") { SECTION("clear()") @@ -988,3 +1035,44 @@ TEST_CASE("modifiers") } } } + +TEST_CASE("update() on deeply nested values") +{ + SECTION("merging past the descent bound gives the same result") + { + // every depth on either side of where the iterative version takes + // over (basic_json::merge_depth_limit(), 128) + for (std::size_t depth = 0; depth <= 300; ++depth) + { + CAPTURE(depth); + for (int variant = 0; variant < 3; ++variant) + { + CAPTURE(variant); + const json source = json::parse(nested_objects(depth, variant)); + json result = json::parse(nested_objects(depth, (variant + 1) % 3)); + json expected = result; + result.update(source, true); + reference_update(expected, source); + CHECK(result == expected); + } + } + } + + SECTION("objects nested too deeply for the call stack (#5545)") + { + // merging used to recurse once per nesting level. The result is only + // walked, never copied or compared, since those recurse too. + const std::size_t depth = 100000; + json target = json::parse(nested_objects(depth, 0)); + target.update(json::parse(nested_objects(depth, 1)), true); + + const json* p = ⌖ + for (std::size_t i = 0; i < depth; ++i) + { + p = &p->at("a"); + } + CHECK(p->size() == 2); + CHECK(p->at("x") == 1); + CHECK(p->at("y") == 2); + } +}