diff --git a/docs/mkdocs/docs/api/basic_json/patch_inplace.md b/docs/mkdocs/docs/api/basic_json/patch_inplace.md index 5b0d501bb..7e4254ba4 100644 --- a/docs/mkdocs/docs/api/basic_json/patch_inplace.md +++ b/docs/mkdocs/docs/api/basic_json/patch_inplace.md @@ -62,6 +62,16 @@ Unlike [`patch`](patch.md), `patch_inplace` applies the operation "in place" and That makes it faster for large documents by avoiding the copy. However, the JSON value might be corrupted if the function throws an exception. +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" @@ -113,3 +123,4 @@ function throws an exception. - 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. diff --git a/docs/mkdocs/docs/api/json_pointer/operator_slasheq.md b/docs/mkdocs/docs/api/json_pointer/operator_slasheq.md index 05e8ccbcf..3d37b17b3 100644 --- a/docs/mkdocs/docs/api/json_pointer/operator_slasheq.md +++ b/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. diff --git a/include/nlohmann/detail/json_pointer.hpp b/include/nlohmann/detail/json_pointer.hpp index 4560714ab..aadb731d7 100644 --- a/include/nlohmann/detail/json_pointer.hpp +++ b/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()); diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 3d40682d9..c7be2595d 100644 --- a/include/nlohmann/json.hpp +++ b/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}; diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 1beba149b..74511d74f 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -23828,6 +23828,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()); @@ -42740,6 +42747,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}; diff --git a/tests/src/unit-json_patch.cpp b/tests/src/unit-json_patch.cpp index 67ffee6dd..1de823952 100644 --- a/tests/src/unit-json_patch.cpp +++ b/tests/src/unit-json_patch.cpp @@ -823,6 +823,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().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().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") diff --git a/tests/src/unit-json_pointer.cpp b/tests/src/unit-json_pointer.cpp index 7748a375a..f874c9691 100644 --- a/tests/src/unit-json_pointer.cpp +++ b/tests/src/unit-json_pointer.cpp @@ -779,6 +779,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 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"};