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 <mail@nlohmann.me>

docs/mkdocs/docs/api/basic_json/merge_patch.md

@@ -37,6 +37,25 @@ Thereby, `Target` is the current object; that is, the patch is applied to the cu
 
 Linear in the lengths of `apply_patch`.
 
+## Notes
+
+!!! 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
+    `#!cpp (*this)[key]`). Calling `merge_patch()` with such an argument reads the argument after it has been
+    invalidated by the modification, which is undefined behavior. If the patch may alias `#!cpp *this`, pass a copy
+    instead:
+
+    ```cpp
+    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
 
 ??? example
@@ -61,3 +80,5 @@ Linear in the lengths of `apply_patch`.
 ## Version history
 
 - Added in version 3.0.0.
+- Documented that `apply_patch` must not be `#!cpp *this` or refer to a value contained in `#!cpp *this`, in version
+  3.13.0.

docs/mkdocs/docs/api/basic_json/operator[].md

@@ -70,9 +70,6 @@ Strong exception safety: if an exception occurs, the original value stays intact
 1. The function can throw the following exceptions:
     - Throws [`type_error.305`](../../home/exceptions.md#jsonexceptiontype_error305) if the JSON value is not an array
       or null; in that case, using the `[]` operator with an index makes no sense.
-    - Throws `#!cpp std::length_error` if `idx` equals the maximum value of `size_type`; the array is left unchanged.
-      (This is the one index for which growing the array to hold it cannot be expressed as a `size_type` size, the same
-      way an oversized [`resize`](https://en.cppreference.com/w/cpp/container/vector/resize) throws.)
 2. The function can throw the following exceptions:
     - Throws [`type_error.305`](../../home/exceptions.md#jsonexceptiontype_error305) if the JSON value is not an object
       or null; in that case, using the `[]` operator with a key makes no sense.
@@ -260,8 +257,7 @@ Strong exception safety: if an exception occurs, the original value stays intact
 
 ## Version history
 
-1. Added in version 1.0.0. Fixed in version 3.13.0 to throw `#!cpp std::length_error` instead of emptying the array and
-   accessing it out of bounds when `idx` equals the maximum value of `size_type`.
+1. Added in version 1.0.0.
 2. Added in version 1.0.0. Added overloads for `T* key` in version 1.1.0. Removed overloads for `T* key` (replaced by 3)
    in version 3.11.0.
 3. Added in version 3.11.0. Fixed in version 3.13.0 to consistently accept `std::string_view`-convertible keys, as

docs/mkdocs/docs/api/basic_json/update.md

@@ -59,6 +59,25 @@ Basic guarantee: if an exception is thrown during the operation, the JSON value
 1. O(N*log(size() + N)), where N is the number of elements to insert.
 2. O(N*log(size() + N)), where N is the number of elements to insert.
 
+## Notes
+
+!!! 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
+    `#!cpp *this` (for example, a subobject returned by `#!cpp (*this)[key]`). Calling `update()` with such an
+    argument reads the argument after it has been invalidated by the modification, which is undefined behavior. If
+    the argument may alias `#!cpp *this`, pass a copy instead:
+
+    ```cpp
+    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
 
 ??? example
@@ -155,3 +174,5 @@ Basic guarantee: if an exception is thrown during the operation, the JSON value
 
 - Added in version 3.0.0.
 - Added `merge_objects` parameter in 3.10.5.
+- Documented that the argument must not be `#!cpp *this` or refer to a value contained in `#!cpp *this`, in version
+  3.13.0.

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 <nlohmann/json.hpp>
+    
+    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

include/nlohmann/json.hpp

@@ -36,9 +36,7 @@
     #include <iosfwd> // istream, ostream
 #endif  // JSON_NO_IO
 #include <iterator> // make_move_iterator, random_access_iterator_tag
-#include <limits> // numeric_limits
 #include <memory> // unique_ptr
-#include <stdexcept> // length_error
 #include <string> // string, stoi, to_string
 #include <utility> // declval, forward, move, pair, swap
 #include <vector> // vector
@@ -2832,13 +2830,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
             // fill up the array with null values if given idx is outside the range
             if (idx >= m_data.m_value.array->size())
             {
-                // idx + 1 would overflow size_type and wrap to 0, which would empty
-                // the array instead of growing it; reject such an idx the same way
-                // resize() rejects other indices that are too large to represent
-                if (JSON_HEDLEY_UNLIKELY(idx == (std::numeric_limits<size_type>::max)()))
-                {
-                    JSON_THROW(std::length_error(detail::concat("array index ", std::to_string(idx), " exceeds size_type")));
-                }
 #if JSON_DIAGNOSTICS
                 // remember array size & capacity before resizing
                 const auto old_size = m_data.m_value.array->size();
@@ -4227,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);
     }
 
@@ -6312,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);
     }
 

single_include/nlohmann/json.hpp

@@ -36,9 +36,7 @@
     #include <iosfwd> // istream, ostream
 #endif  // JSON_NO_IO
 #include <iterator> // make_move_iterator, random_access_iterator_tag
-#include <limits> // numeric_limits
 #include <memory> // unique_ptr
-#include <stdexcept> // length_error
 #include <string> // string, stoi, to_string
 #include <utility> // declval, forward, move, pair, swap
 #include <vector> // vector
@@ -28913,13 +28911,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
             // fill up the array with null values if given idx is outside the range
             if (idx >= m_data.m_value.array->size())
             {
-                // idx + 1 would overflow size_type and wrap to 0, which would empty
-                // the array instead of growing it; reject such an idx the same way
-                // resize() rejects other indices that are too large to represent
-                if (JSON_HEDLEY_UNLIKELY(idx == (std::numeric_limits<size_type>::max)()))
-                {
-                    JSON_THROW(std::length_error(detail::concat("array index ", std::to_string(idx), " exceeds size_type")));
-                }
 #if JSON_DIAGNOSTICS
                 // remember array size & capacity before resizing
                 const auto old_size = m_data.m_value.array->size();
@@ -30308,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);
     }
 
@@ -32393,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);
     }
 

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
 

tests/src/unit-element_access1.cpp

@@ -147,24 +147,6 @@ TEST_CASE("element access 1")
                 CHECK(j_const[7] == json({1, 2, 3}));
             }
 
-            SECTION("SIZE_MAX index (#5647)")
-            {
-                // idx + 1 must not be computed for idx == SIZE_MAX: it wraps to 0,
-                // which would empty the array and then write out of bounds instead
-                // of growing it; reject it like an oversized resize() would and
-                // leave the array unchanged
-                const auto max_idx = (std::numeric_limits<json::size_type>::max)();
-                const std::string expected = "array index " + std::to_string(max_idx) + " exceeds size_type";
-                const json j_before = j; // NOLINT(performance-unnecessary-copy-initialization)
-
-                CHECK_THROWS_WITH_AS(j[max_idx] = 1, expected.c_str(), std::length_error&);
-                CHECK(j == j_before);
-
-                json j_empty = json::array();
-                CHECK_THROWS_WITH_AS(j_empty[max_idx] = 1, expected.c_str(), std::length_error&);
-                CHECK(j_empty == json::array());
-            }
-
             SECTION("access on non-array type")
             {
                 SECTION("null")