From 8948bfc4d9e41c42a9e80494ba41be8d615c77bd Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Tue, 29 Sep 2026 23:37:46 +0200 Subject: [PATCH] Copy values before inserting an initializer list into an array insert(pos, {...}) inserted wrong values when the initializer list contained const references to elements of the array being inserted into. json_ref stores only a pointer for a const lvalue, so the initializer_list_t range passed straight to the array's range insert aliased the array's own storage; std::vector::insert(pos, first, last) may move or shift elements before copying from that range, so the source elements were already stale by the time they were read (different wrong results on libc++ and libstdc++). Copy the referenced values into a temporary array_t first, then move that temporary into place, so the source range never aliases the array being modified. Fixes #5656. Signed-off-by: Niels Lohmann --- docs/mkdocs/docs/api/basic_json/insert.md | 4 +++- include/nlohmann/json.hpp | 10 +++++++- single_include/nlohmann/json.hpp | 10 +++++++- tests/src/unit-modifiers.cpp | 28 +++++++++++++++++++++++ 4 files changed, 49 insertions(+), 3 deletions(-) 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 500fcddf2..a3f13d29e 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -4152,8 +4152,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; + values.reserve(ilist.size()); + 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 576498738..639f53c1a 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -30233,8 +30233,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; + values.reserve(ilist.size()); + 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-modifiers.cpp b/tests/src/unit-modifiers.cpp index c878ec15c..794cc241b 100644 --- a/tests/src/unit-modifiers.cpp +++ b/tests/src/unit-modifiers.cpp @@ -761,6 +761,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