Merge origin/develop into fix-self-aliasing-pointer-patch

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

docs/mkdocs/docs/api/basic_json/patch_inplace.md

@@ -69,6 +69,16 @@ Duplicate member names inside an operation object are resolved by the parser bef
 is kept, see [`object_t`](object_t.md#behavior)), so `#!json {"op": "add", "op": "remove", "path": "/a"}` is a
 `remove` operation.
 
+The JSON patch may be the JSON value itself (`#!cpp j.patch_inplace(j)`): the operations are then read from a copy of
+the patch made before the first operation is applied, so the result is the same as that of
+`#!cpp j = j.patch(j)`, and the exception safety is the same as above.
+
+!!! danger "Undefined behavior"
+
+    The JSON patch must not be a value stored inside the JSON value (e.g., `#!cpp j.patch_inplace(j["patch"])`), as the
+    operations may change or destroy it while it is read. Pass a copy (`#!cpp j.patch_inplace(json(j["patch"]))`) or use
+    [`patch`](patch.md) instead.
+
 ## Examples
 
 ??? example "Example: apply a JSON patch in place"
@@ -121,3 +131,4 @@ is kept, see [`object_t`](object_t.md#behavior)), so `#!json {"op": "add", "op":
 - Throws [`parse_error.109`](../../home/exceptions.md#jsonexceptionparse_error109) instead of
   [`out_of_range.404`](../../home/exceptions.md#jsonexceptionout_of_range404) for a one-character array index that is
   not a digit (e.g., `/x`) in version 3.13.0, as it already did for longer ones.
+- Applying a JSON value as patch to itself no longer results in undefined behavior in version 3.13.0.

docs/mkdocs/docs/api/json_pointer/operator_slasheq.md

@@ -18,7 +18,8 @@ json_pointer& operator/=(std::size_t array_idx)
 ## Parameters
 
 `ptr` (in)
-:    JSON pointer to append
+:    JSON pointer to append; may be the JSON pointer itself (`#!cpp p /= p`), which then appends a copy of its
+     reference tokens
 
 `token` (in)
 :    reference token to append
@@ -68,6 +69,7 @@ json_pointer& operator/=(std::size_t array_idx)
 
 ## Version history
 
-1. Added in version 3.6.0.
+1. Added in version 3.6.0. Appending a JSON pointer to itself no longer results in undefined behavior in version
+   3.13.0.
 2. Added in version 3.6.0. Changed type of `token` to `string_t` in version 3.11.0.
 3. Added in version 3.6.0.

include/nlohmann/detail/json_pointer.hpp

@@ -243,6 +243,13 @@ class json_pointer
     /// @sa https://json.nlohmann.me/api/json_pointer/operator_slasheq/
     json_pointer& operator/=(const json_pointer& ptr)
     {
+        // insert() must not be given iterators into the vector it inserts
+        // into: append a copy when a pointer is appended to itself
+        if (JSON_HEDLEY_UNLIKELY(&ptr == this))
+        {
+            return *this /= json_pointer(ptr);
+        }
+
         reference_tokens.insert(reference_tokens.end(),
                                 ptr.reference_tokens.begin(),
                                 ptr.reference_tokens.end());

include/nlohmann/json.hpp

@@ -6950,6 +6950,15 @@ public:
     /// @sa https://json.nlohmann.me/api/basic_json/patch/
     void patch_inplace(const basic_json& json_patch)
     {
+        // the operations below change *this while iterating json_patch: apply
+        // a copy when a value is applied as patch to itself
+        if (JSON_HEDLEY_UNLIKELY(&json_patch == this))
+        {
+            const basic_json json_patch_copy = json_patch;
+            patch_inplace(json_patch_copy);
+            return;
+        }
+
         basic_json& result = *this;
         // the valid JSON Patch operations
         enum class patch_operations {add, remove, replace, move, copy, test, invalid};

include/nlohmann/json_view.hpp

@@ -1313,7 +1313,7 @@ class basic_json_document
     /// member is set (added if missing), an array element assigned, and "-"
     /// or the size of the array appends. A null parent becomes what
     /// basic_json's operator[](json_pointer) makes of it: an array for "-"
-    /// and for array indices (padded with nulls up to the index), an object otherwise.
+    /// and for digits (padded with nulls up to the index), an object otherwise.
     template<typename V>
     view_type set(const json_pointer& ptr, V&& value)
     {
@@ -1329,11 +1329,7 @@ class basic_json_document
             {
                 return push_back(parent, std::forward<V>(value));
             }
-            // (a token that cannot be an array index, such as "01" or "", names an
+            const bool digits = std::all_of(token.begin(), token.end(), [](const char c)
-            // object member, as in basic_json)
-            const bool digits = !token.empty()
-                                && (token.size() == 1 || token[0] != '0')
-                                && std::all_of(token.begin(), token.end(), [](const char c)
             {
                 return c >= '0' && c <= '9';
             });

single_include/nlohmann/json.hpp

@@ -24307,6 +24307,13 @@ class json_pointer
     /// @sa https://json.nlohmann.me/api/json_pointer/operator_slasheq/
     json_pointer& operator/=(const json_pointer& ptr)
     {
+        // insert() must not be given iterators into the vector it inserts
+        // into: append a copy when a pointer is appended to itself
+        if (JSON_HEDLEY_UNLIKELY(&ptr == this))
+        {
+            return *this /= json_pointer(ptr);
+        }
+
         reference_tokens.insert(reference_tokens.end(),
                                 ptr.reference_tokens.begin(),
                                 ptr.reference_tokens.end());
@@ -43245,6 +43252,15 @@ public:
     /// @sa https://json.nlohmann.me/api/basic_json/patch/
     void patch_inplace(const basic_json& json_patch)
     {
+        // the operations below change *this while iterating json_patch: apply
+        // a copy when a value is applied as patch to itself
+        if (JSON_HEDLEY_UNLIKELY(&json_patch == this))
+        {
+            const basic_json json_patch_copy = json_patch;
+            patch_inplace(json_patch_copy);
+            return;
+        }
+
         basic_json& result = *this;
         // the valid JSON Patch operations
         enum class patch_operations {add, remove, replace, move, copy, test, invalid};

single_include/nlohmann/json_view.hpp

@@ -9067,7 +9067,7 @@ class basic_json_document
     /// member is set (added if missing), an array element assigned, and "-"
     /// or the size of the array appends. A null parent becomes what
     /// basic_json's operator[](json_pointer) makes of it: an array for "-"
-    /// and for array indices (padded with nulls up to the index), an object otherwise.
+    /// and for digits (padded with nulls up to the index), an object otherwise.
     template<typename V>
     view_type set(const json_pointer& ptr, V&& value)
     {
@@ -9083,11 +9083,7 @@ class basic_json_document
             {
                 return push_back(parent, std::forward<V>(value));
             }
-            // (a token that cannot be an array index, such as "01" or "", names an
+            const bool digits = std::all_of(token.begin(), token.end(), [](const char c)
-            // object member, as in basic_json)
-            const bool digits = !token.empty()
-                                && (token.size() == 1 || token[0] != '0')
-                                && std::all_of(token.begin(), token.end(), [](const char c)
             {
                 return c >= '0' && c <= '9';
             });

tests/src/unit-json_patch.cpp

@@ -826,6 +826,109 @@ TEST_CASE("JSON patch")
             CHECK(doc.at("foo") == "bar");
         }
 #endif // !defined(JSON_NOEXCEPTION)
+
+        SECTION("applying a value as patch to itself")
+        {
+            // a JSON patch is an array, so an array of operations can be
+            // applied to itself; the operations are read from the patch as
+            // it was before the first operation, as patch() does it
+            const json copy_op = {{"op", "copy"}, {"from", "/0"}, {"path", "/-"}};
+            json copy_ops = json::array();
+            for (int i = 0; i < 5; ++i)
+            {
+                copy_ops.push_back(copy_op);
+            }
+
+            json expected_copies = copy_ops;
+            for (int i = 0; i < 5; ++i)
+            {
+                expected_copies.push_back(copy_op);
+            }
+
+            SECTION("copy operations without spare capacity")
+            {
+                json doc = copy_ops;
+                doc.get_ref<json::array_t&>().shrink_to_fit();
+                CHECK(doc.patch(doc) == expected_copies);
+
+                doc.patch_inplace(doc);
+                CHECK(doc == expected_copies);
+            }
+
+            SECTION("copy operations with spare capacity")
+            {
+                json doc = copy_ops;
+                doc.get_ref<json::array_t&>().reserve(100);
+                CHECK(doc.patch(doc) == expected_copies);
+
+                doc.patch_inplace(doc);
+                CHECK(doc == expected_copies);
+            }
+
+            SECTION("remove operations")
+            {
+                json doc = R"([
+                        {"op": "remove", "path": "/0"},
+                        {"op": "remove", "path": "/0"},
+                        {"op": "remove", "path": "/0"}
+                    ])"_json;
+                CHECK(doc.patch(doc) == json::array());
+
+                doc.patch_inplace(doc);
+                CHECK(doc == json::array());
+            }
+
+            SECTION("operations that read a changed element")
+            {
+                // the second operation is still a "test" operation, although
+                // the first one replaced it in the document
+                json doc = R"([
+                        {"op": "replace", "path": "/1", "value": 1},
+                        {"op": "test", "path": "/1", "value": 1},
+                        {"op": "add", "path": "/-", "value": 2}
+                    ])"_json;
+                const json expected = R"([
+                        {"op": "replace", "path": "/1", "value": 1},
+                        1,
+                        {"op": "add", "path": "/-", "value": 2},
+                        2
+                    ])"_json;
+                CHECK(doc.patch(doc) == expected);
+
+                doc.patch_inplace(doc);
+                CHECK(doc == expected);
+            }
+
+            SECTION("replacing the root")
+            {
+                json doc = R"([
+                        {"op": "add", "path": "", "value": [1, 2]},
+                        {"op": "add", "path": "/-", "value": 3},
+                        {"op": "replace", "path": "", "value": {"a": 4}}
+                    ])"_json;
+                const json expected = {{"a", 4}};
+                CHECK(doc.patch(doc) == expected);
+
+                doc.patch_inplace(doc);
+                CHECK(doc == expected);
+            }
+
+#if !defined(JSON_NOEXCEPTION)
+            SECTION("failing operation")
+            {
+                // the operations before the failing one remain applied
+                json doc = R"([
+                        {"op": "add", "path": "/-", "value": 1},
+                        {"op": "test", "path": "/0/op", "value": "remove"}
+                    ])"_json;
+                CHECK_THROWS_AS(doc.patch(doc), json::other_error&);
+
+                CHECK_THROWS_AS(doc.patch_inplace(doc), json::other_error&);
+                CHECK(doc.size() == 3);
+                CHECK(doc.back() == 1);
+            }
+#endif // !defined(JSON_NOEXCEPTION)
+        }
     }
 
     SECTION("errors")

tests/src/unit-json_pointer.cpp

@@ -845,6 +845,44 @@ TEST_CASE("JSON pointers")
         CHECK(ptr.to_string() == "/object/~1");
     }
 
+    SECTION("appending a pointer to itself")
+    {
+        // operator/= must not pass iterators into its own token vector to
+        // insert(); each pointer is appended to itself and must yield the
+        // pointer whose string representation is the original one twice
+        const std::vector<std::string> pointers =
+        {
+            "",
+            "/",
+            "/a",
+            "/a/b",
+            "/foo/0/-",
+            "/a~1b/~0c/~01",
+            "/1/2/3/4/5/6/7/8/9/10/11/12/13/14/15/16/17"
+        };
+
+        for (const auto& s : pointers)
+        {
+            CAPTURE(s)
+            json::json_pointer ptr(s);
+            const json::json_pointer expected(s + s);
+
+            json::json_pointer& ret = (ptr /= ptr);
+            CHECK(&ret == &ptr);
+            CHECK(ptr == expected);
+            CHECK(ptr.to_string() == s + s);
+
+            // appending to itself again doubles the pointer once more
+            ptr /= ptr;
+            CHECK(ptr.to_string() == s + s + s + s);
+
+            // operator/ appends to a copy and leaves the operand unchanged
+            const json::json_pointer original(s);
+            CHECK((original / original) == expected);
+            CHECK(original.to_string() == s);
+        }
+    }
+
     SECTION("equality comparison")
     {
         std::string ptr_string{"/foo/bar"};
@@ -1122,17 +1160,17 @@ TEST_CASE("flatten of structured values")
 
         // the same for other keys that are not valid array indices
         CHECK_THROWS_WITH_AS(json({{"/a/0", 1}, {"/a/1a", 2}}).unflatten(),
-        "[json.exception.out_of_range.404] unresolved reference token '1a'", json::out_of_range&);
+                             "[json.exception.out_of_range.404] unresolved reference token '1a'", json::out_of_range&);
         CHECK_THROWS_WITH_AS(json({{"/a/0", 1}, {"/a/-", 2}}).unflatten(),
-        "[json.exception.out_of_range.404] unresolved reference token '-'", json::out_of_range&);
+                             "[json.exception.out_of_range.404] unresolved reference token '-'", json::out_of_range&);
     }
 
     SECTION("unflatten with the empty token among array indices")
     {
         CHECK_THROWS_WITH_AS(json({{"/a/0", 1}, {"/a/", 2}}).unflatten(),
-        "[json.exception.out_of_range.404] unresolved reference token ''", json::out_of_range&);
+                             "[json.exception.out_of_range.404] unresolved reference token ''", json::out_of_range&);
         CHECK_THROWS_WITH_AS(json({{"/a/0", 1}, {"/a/1", 2}, {"/a/", 3}}).unflatten(),
-        "[json.exception.out_of_range.404] unresolved reference token ''", json::out_of_range&);
+                             "[json.exception.out_of_range.404] unresolved reference token ''", json::out_of_range&);
 
         // without an index "0", the empty token is an ordinary object key
         CHECK(json({{"/a/", 2}}).unflatten() == json({{"a", {{"", 2}}}}));