Fix update() and merge_patch() when the argument is *this or one of its members (#5678)

tests/src/unit-diagnostics.cpp

@@ -236,6 +236,31 @@ TEST_CASE("Regression tests for extended diagnostics")
         }
     }
 
+    SECTION("Regression test for issue #5641 - parent pointers after update()/merge_patch() with an aliasing argument")
+    {
+        // update()'s and merge_patch()'s argument may be *this or one of its
+        // descendants; the values moved out of the (temporary) copy must end
+        // up with their parent pointing at their new location in *this
+        {
+            json j = {{"a", {{"a", 1}, {"b", 2}}}};
+            j.update(j["a"]);
+            CHECK(j == json({{"a", 1}, {"b", 2}}));
+
+            // Must call operator[] on const element, otherwise m_parent gets updated.
+            auto const& constJ = j;
+            CHECK_THROWS_WITH_AS(constJ["a"].at(0), "[json.exception.type_error.304] (/a) cannot use at() with number", json::type_error);
+        }
+
+        {
+            json j = {{"a", {{"a", nullptr}, {"b", 2}}}};
+            j.merge_patch(j["a"]);
+            CHECK(j == json({{"b", 2}}));
+
+            auto const& constJ = j;
+            CHECK_THROWS_WITH_AS(constJ["b"].at(0), "[json.exception.type_error.304] (/b) cannot use at() with number", json::type_error);
+        }
+    }
+
     SECTION("Regression test for issue #3032 - Yet another assertion failure when inserting into arrays with JSON_DIAGNOSTICS set")
     {
         // reference operator[](size_type idx)

tests/src/unit-merge_patch.cpp

@@ -374,3 +374,52 @@ TEST_CASE("JSON Merge Patch and update on ordered_json")
         CHECK(target == ordered_json::parse(R"({"a": 1, "e": {"y": 5}, "g": 6})"));
     }
 }
+
+TEST_CASE("merge_patch() with an argument that aliases *this (#5641)")
+{
+    SECTION("j.merge_patch(j): erasing a member destroys the node the loop's iterator points to")
+    {
+        // reproduces issue #5641, case 1
+        json j = {{"a", nullptr}, {"b", 1}};
+        j.merge_patch(j);
+        CHECK(j == json({{"b", 1}}));
+    }
+
+    SECTION("j.merge_patch(j[\"a\"]): removing \"a\" destroys the patch while it is iterated")
+    {
+        // reproduces issue #5641, case 2
+        json j = {{"a", {{"a", nullptr}, {"b", 2}}}};
+        j.merge_patch(j["a"]);
+        CHECK(j == json({{"b", 2}}));
+    }
+
+    SECTION("a patch nested past the iterative descent bound aliases *this")
+    {
+        // every depth on either side of where the iterative version takes
+        // over (detail::recursion_depth_limit(), 128); patching *this with
+        // itself is idempotent, aliased or not
+        for (const std::size_t depth :
+                {
+                    std::size_t{0}, std::size_t{127}, std::size_t{128}, std::size_t{300}
+                })
+        {
+            CAPTURE(depth);
+            json j = json::parse(nested_objects(depth, 0));
+            const json expected = j;
+            j.merge_patch(j);
+            CHECK(j == expected);
+        }
+    }
+
+    SECTION("ordered_json")
+    {
+        using nlohmann::ordered_json;
+
+        SECTION("merge_patch with a member of *this")
+        {
+            ordered_json j = {{"a", {{"a", nullptr}, {"b", 2}}}};
+            j.merge_patch(j["a"]);
+            CHECK(j == ordered_json({{"b", 2}}));
+        }
+    }
+}

tests/src/unit-modifiers.cpp

@@ -1116,3 +1116,72 @@ TEST_CASE("update() on deeply nested values")
         CHECK(p->at("y") == 2);
     }
 }
+
+TEST_CASE("update() with an argument that aliases *this (#5641)")
+{
+    SECTION("the target is checked before the argument, as before the copy")
+    {
+        json j = 1;
+        CHECK_THROWS_WITH_AS(j.update(json::array()), "[json.exception.type_error.312] cannot use update() with number", json::type_error&);
+        CHECK_THROWS_WITH_AS(j.update(j.cbegin(), j.cend()), "[json.exception.type_error.312] cannot use update() with number", json::type_error&);
+
+        json k;
+        CHECK_THROWS_WITH_AS(k.update(json::array()), "[json.exception.type_error.312] cannot use update() with array", json::type_error&);
+        CHECK(k == json::object());
+    }
+
+    SECTION("const reference")
+    {
+        SECTION("j.update(j[\"a\"]): assigning into the argument's parent destroys it mid-iteration")
+        {
+            // reproduces issue #5641, case 3
+            json j = {{"a", {{"a", 1}, {"b", 2}}}};
+            j.update(j["a"]);
+            CHECK(j == json({{"a", 1}, {"b", 2}}));
+        }
+
+        SECTION("merge_objects with an argument that is a member of *this")
+        {
+            json j = {{"defaults", {{"opts", {{"a", 1}}}}}, {"opts", {{"b", 2}}}};
+            j.update(j["defaults"], true);
+            CHECK(j == json({{"defaults", {{"opts", {{"a", 1}}}}}, {"opts", {{"a", 1}, {"b", 2}}}}));
+        }
+
+        SECTION("ordered_json: inserting a new key relocates the vector behind the argument")
+        {
+            // reproduces issue #5641, case 4
+            using nlohmann::ordered_json;
+            ordered_json j = {{"a", {{"x", 1}, {"y", 2}, {"z", 3}}}};
+            j.update(j["a"]);
+            CHECK(j == ordered_json({{"a", {{"x", 1}, {"y", 2}, {"z", 3}}}, {"x", 1}, {"y", 2}, {"z", 3}}));
+        }
+    }
+
+    SECTION("iterator range")
+    {
+        SECTION("range that is a member of *this")
+        {
+            json j = {{"a", {{"a", 1}, {"b", 2}}}};
+            j.update(j["a"].begin(), j["a"].end());
+            CHECK(j == json({{"a", 1}, {"b", 2}}));
+        }
+    }
+
+    SECTION("nested past the iterative descent bound aliases *this")
+    {
+        // every depth on either side of where the iterative version takes
+        // over (detail::recursion_depth_limit(), 128); merging *this into
+        // itself is idempotent, aliased or not
+        for (const std::size_t depth :
+                {
+                    std::size_t{0}, std::size_t{127}, std::size_t{128}, std::size_t{300}
+                })
+        {
+            CAPTURE(depth);
+            json j = json::parse(nested_objects(depth, 0));
+            const json expected = j;
+            j.update(j, true);
+            CHECK(j == expected);
+        }
+    }
+}