From c194c7703d4c2dca09296b1bea025c4ad5332ec3 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 30 Sep 2026 07:33:15 +0200 Subject: [PATCH] Assert that update() and merge_patch() are not called with *this Passing *this itself is cheap to detect; a value contained in *this is not. Document the assertion. Addresses review comment by @gregmarr. Signed-off-by: Niels Lohmann --- .../mkdocs/docs/api/basic_json/merge_patch.md | 5 +++- docs/mkdocs/docs/api/basic_json/update.md | 5 +++- docs/mkdocs/docs/features/assertions.md | 30 +++++++++++++++++++ include/nlohmann/json.hpp | 6 ++++ single_include/nlohmann/json.hpp | 6 ++++ tests/src/unit-assert_macro.cpp | 27 +++++++++++++++++ 6 files changed, 77 insertions(+), 2 deletions(-) diff --git a/docs/mkdocs/docs/api/basic_json/merge_patch.md b/docs/mkdocs/docs/api/basic_json/merge_patch.md index 712244022..9b69a1292 100644 --- a/docs/mkdocs/docs/api/basic_json/merge_patch.md +++ b/docs/mkdocs/docs/api/basic_json/merge_patch.md @@ -39,7 +39,7 @@ Linear in the lengths of `apply_patch`. ## Notes -!!! danger "Undefined behavior" +!!! danger "Undefined behavior and runtime assertions" `merge_patch()` reads `apply_patch` while it modifies `#!cpp *this`. `apply_patch` must not be `#!cpp *this` itself and must not refer to a value contained in `#!cpp *this` (for example, a subobject returned by @@ -51,6 +51,9 @@ Linear in the lengths of `apply_patch`. j.merge_patch(json(j)); // instead of j.merge_patch(j) ``` + Passing `#!cpp *this` itself is **guarded by a [runtime assertion](../../features/assertions.md)**; a value + contained in `#!cpp *this` is not detected. + See [GitHub issue #5641](https://github.com/nlohmann/json/issues/5641) for more information. ## Examples diff --git a/docs/mkdocs/docs/api/basic_json/update.md b/docs/mkdocs/docs/api/basic_json/update.md index 56d8aa1e4..dd97e9b1e 100644 --- a/docs/mkdocs/docs/api/basic_json/update.md +++ b/docs/mkdocs/docs/api/basic_json/update.md @@ -61,7 +61,7 @@ Basic guarantee: if an exception is thrown during the operation, the JSON value ## Notes -!!! danger "Undefined behavior" +!!! danger "Undefined behavior and runtime assertions" Both overloads read the argument while they modify `#!cpp *this`. The argument `j` (or, for overload (2), the range `[first, last)`) must not be `#!cpp *this` itself and must not refer to a value contained in @@ -73,6 +73,9 @@ Basic guarantee: if an exception is thrown during the operation, the JSON value j.update(json(j["defaults"])); // instead of j.update(j["defaults"]) ``` + Passing `#!cpp *this` itself is **guarded by a [runtime assertion](../../features/assertions.md)**; a value + contained in `#!cpp *this` is not detected. + See [GitHub issue #5641](https://github.com/nlohmann/json/issues/5641) for more information. ## Examples diff --git a/docs/mkdocs/docs/features/assertions.md b/docs/mkdocs/docs/features/assertions.md index 789af7989..06d44bf9b 100644 --- a/docs/mkdocs/docs/features/assertions.md +++ b/docs/mkdocs/docs/features/assertions.md @@ -103,6 +103,36 @@ behavior and yields a runtime assertion. Assertion failed: (m_object != nullptr), function operator++, file iter_impl.hpp, line 368. ``` +### Updating or merge-patching a value with itself + +Functions [`update`](../api/basic_json/update.md) and [`merge_patch`](../api/basic_json/merge_patch.md) read their +argument while they modify the value they are called on. Passing that value itself as the argument is undefined +behavior and yields a runtime assertion. Pass a copy instead, for example `#!cpp j.merge_patch(json(j))`. An argument +that refers to a value contained in the value (e.g., `#!cpp j.update(j["defaults"])`) is undefined behavior as well, but +is not detected. + +??? example "Example 4: Merge-patching a value with itself" + + The following code will trigger an assertion at runtime: + + ```cpp + #include + + using json = nlohmann::json; + + int main() + { + json j = {{"key", "value"}}; + j.merge_patch(j); + } + ``` + + Output: + + ``` + Assertion failed: (&apply_patch != this), function merge_patch, file json.hpp, line 6310. + ``` + ## Changes ### Reading from a null `FILE` or `char` pointer diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 500fcddf2..72d27c913 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -4218,6 +4218,9 @@ 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)); } + // the range must not be *this; a range inside *this is not detected + JSON_ASSERT(first.m_object != this); + update_members(first, last, merge_objects, 0); } @@ -6303,6 +6306,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @sa https://json.nlohmann.me/api/basic_json/merge_patch/ void merge_patch(const basic_json& apply_patch) { + // the patch must not be *this; a patch inside *this is not detected + JSON_ASSERT(&apply_patch != this); + apply_merge_patch(apply_patch, 0); } diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 576498738..ab2f436bc 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -30299,6 +30299,9 @@ 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)); } + // the range must not be *this; a range inside *this is not detected + JSON_ASSERT(first.m_object != this); + update_members(first, last, merge_objects, 0); } @@ -32384,6 +32387,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec /// @sa https://json.nlohmann.me/api/basic_json/merge_patch/ void merge_patch(const basic_json& apply_patch) { + // the patch must not be *this; a patch inside *this is not detected + JSON_ASSERT(&apply_patch != this); + apply_merge_patch(apply_patch, 0); } diff --git a/tests/src/unit-assert_macro.cpp b/tests/src/unit-assert_macro.cpp index adedfc934..0e72541ed 100644 --- a/tests/src/unit-assert_macro.cpp +++ b/tests/src/unit-assert_macro.cpp @@ -41,6 +41,33 @@ TEST_CASE("JSON_ASSERT(x)") // check that assertion actually happened CHECK(assert_counter == 1); } + + SECTION("update() and merge_patch() with *this") + { + // update() and merge_patch() must not be called with *this (#5641); + // the values are chosen so the calls still happen to work without + // aborting, and only the assertion counter is checked + json j = {{"a", 1}}; + + assert_counter = 0; + j.update(j); + CHECK(assert_counter == 1); + + assert_counter = 0; + j.update(j.cbegin(), j.cend()); + CHECK(assert_counter == 1); + + assert_counter = 0; + j.merge_patch(j); + CHECK(assert_counter == 1); + + // a copy is fine + assert_counter = 0; + j.update(json(j)); + j.merge_patch(json(j)); + CHECK(assert_counter == 0); + CHECK(j == json({{"a", 1}})); + } } #endif