diff --git a/docs/mkdocs/docs/api/basic_json/insert.md b/docs/mkdocs/docs/api/basic_json/insert.md index fcb1e6e44..ff9082b56 100644 --- a/docs/mkdocs/docs/api/basic_json/insert.md +++ b/docs/mkdocs/docs/api/basic_json/insert.md @@ -195,5 +195,7 @@ Strong exception safety: if an exception occurs, the original value stays intact 1. Added in version 1.0.0. 2. Added in version 1.0.0. 3. Added in version 1.0.0. -4. Added in version 1.0.0. +4. Added in version 1.0.0. Fixed in version 3.13.0 to copy the values before inserting; before, an `ilist` that + referred to elements of the array being inserted into could insert wrong values, because the range insert could + move from or shift an element before it was copied. 5. Added in version 3.0.0. diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 7f0f1d7c2..7f9dd8caf 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -4158,8 +4158,16 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec JSON_THROW(invalid_iterator::create(202, "iterator does not fit current value", this)); } + // copy the values first: ilist may refer to elements of this array + array_t values; + detail::reserve_array(values, ilist.size(), detail::priority_tag<1> {}); + for (const auto& element : ilist) + { + values.push_back(element.moved_or_copied()); + } + // insert to array and return iterator - return insert_iterator(pos, ilist.begin(), ilist.end()); + return insert_iterator(pos, std::make_move_iterator(values.begin()), std::make_move_iterator(values.end())); } /// @brief inserts range of elements into object diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 96a3e67ad..27f65e412 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -31042,8 +31042,16 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec JSON_THROW(invalid_iterator::create(202, "iterator does not fit current value", this)); } + // copy the values first: ilist may refer to elements of this array + array_t values; + detail::reserve_array(values, ilist.size(), detail::priority_tag<1> {}); + for (const auto& element : ilist) + { + values.push_back(element.moved_or_copied()); + } + // insert to array and return iterator - return insert_iterator(pos, ilist.begin(), ilist.end()); + return insert_iterator(pos, std::make_move_iterator(values.begin()), std::make_move_iterator(values.end())); } /// @brief inserts range of elements into object diff --git a/tests/src/unit-custom-array-type.cpp b/tests/src/unit-custom-array-type.cpp index 00606c6e0..7a374b612 100644 --- a/tests/src/unit-custom-array-type.cpp +++ b/tests/src/unit-custom-array-type.cpp @@ -117,6 +117,22 @@ TEST_CASE("array type without capacity()") CHECK(nested.flatten().unflatten() == nested); } + SECTION("insert(pos, initializer_list) compiles and works without reserve()") + { + // std::deque has no reserve() either; insert(pos, ilist) must not + // require it (regression test for #5656, which also covers an ilist + // that refers to elements of the array being inserted into) + deque_json j = deque_json::array(); + j.push_back("a"); + j.push_back("b"); + j.push_back("c"); + + const deque_json& cj = j; + auto it = j.insert(j.begin(), {cj[0], cj[1]}); + CHECK(*it == deque_json("a")); + CHECK(j == deque_json({"a", "b", "a", "b", "c"})); + } + SECTION("references stay valid while the array grows") { deque_json j = deque_json::array(); diff --git a/tests/src/unit-modifiers.cpp b/tests/src/unit-modifiers.cpp index 1ec6016cb..d36ed5ad2 100644 --- a/tests/src/unit-modifiers.cpp +++ b/tests/src/unit-modifiers.cpp @@ -773,6 +773,34 @@ TEST_CASE("modifiers") } } + SECTION("initializer list referring to the array's own elements (#5656)") + { + SECTION("sufficient capacity (no reallocation)") + { + json j_own = json::array(); + j_own.get_ref().reserve(8); + j_own.push_back("a"); + j_own.push_back("b"); + j_own.push_back("c"); + + const json& j_own_cref = j_own; + auto it = j_own.insert(j_own.begin(), {j_own_cref[0], j_own_cref[1]}); + CHECK(*it == json("a")); + CHECK(j_own == json({"a", "b", "a", "b", "c"})); + } + + SECTION("insufficient capacity (reallocation)") + { + json j_own = {"a", "b", "c"}; + j_own.get_ref().shrink_to_fit(); + + const json& j_own_cref = j_own; + auto it = j_own.insert(j_own.begin(), {j_own_cref[2]}); + CHECK(*it == json("c")); + CHECK(j_own == json({"c", "a", "b", "c"})); + } + } + SECTION("invalid iterator") { // pass iterator to a different array