diff --git a/docs/mkdocs/docs/api/basic_json/patch_inplace.md b/docs/mkdocs/docs/api/basic_json/patch_inplace.md index 67c6e8814..ff65e8dbd 100644 --- a/docs/mkdocs/docs/api/basic_json/patch_inplace.md +++ b/docs/mkdocs/docs/api/basic_json/patch_inplace.md @@ -69,6 +69,19 @@ 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. + +If [`JSON_DIAGNOSTICS`](../macros/json_diagnostics.md) is enabled, the same holds for a JSON patch stored inside the +JSON value (e.g., `#!cpp j.patch_inplace(j["patch"])`): it is detected via the parent pointers. + +!!! danger "Undefined behavior" + + Without [`JSON_DIAGNOSTICS`](../macros/json_diagnostics.md), 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" @@ -122,3 +135,5 @@ 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 (or, with `JSON_DIAGNOSTICS`, to a value stored inside it) 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 e0d4560ec..2f2820d4f 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 @@ -69,6 +70,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 eef4ce0a4..6df49fbc0 100644 --- a/include/nlohmann/detail/json_pointer.hpp +++ b/include/nlohmann/detail/json_pointer.hpp @@ -266,6 +266,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 c2dddd55a..607fd1e5a 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -6950,6 +6950,25 @@ 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 or, with + // JSON_DIAGNOSTICS, to a value stored inside it + // NOLINTNEXTLINE(misc-const-correctness): only the JSON_DIAGNOSTICS build modifies it + bool patch_aliases_this = (&json_patch == this); +#if JSON_DIAGNOSTICS + for (const basic_json* p = json_patch.m_parent; !patch_aliases_this && p != nullptr; p = p->m_parent) + { + patch_aliases_this = (p == this); + } +#endif + if (JSON_HEDLEY_UNLIKELY(patch_aliases_this)) + { + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization): *this is modified below + 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 085b55afb..0ebaebaf5 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -24331,6 +24331,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()); @@ -43701,6 +43708,25 @@ 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 or, with + // JSON_DIAGNOSTICS, to a value stored inside it + // NOLINTNEXTLINE(misc-const-correctness): only the JSON_DIAGNOSTICS build modifies it + bool patch_aliases_this = (&json_patch == this); +#if JSON_DIAGNOSTICS + for (const basic_json* p = json_patch.m_parent; !patch_aliases_this && p != nullptr; p = p->m_parent) + { + patch_aliases_this = (p == this); + } +#endif + if (JSON_HEDLEY_UNLIKELY(patch_aliases_this)) + { + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization): *this is modified below + 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 cc1b26a40..83df15350 100644 --- a/tests/src/unit-json_patch.cpp +++ b/tests/src/unit-json_patch.cpp @@ -826,6 +826,129 @@ 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") + { + // a copy has exactly as much capacity as elements + json doc = copy_ops; + CHECK(doc.patch(doc) == expected_copies); + + doc.patch_inplace(doc); + CHECK(doc == expected_copies); + } + +#if JSON_DIAGNOSTICS + SECTION("patch stored inside the patched value") + { + // the parent pointers of JSON_DIAGNOSTICS make the aliasing detectable + const json op = {{"op", "replace"}, {"path", "/patch"}, {"value", 1}}; + json doc = {{"a", 1}, {"patch", json::array({op})}}; + const json expected = doc.patch(doc["patch"]); + + doc.patch_inplace(doc["patch"]); + CHECK(doc == expected); + } +#endif + + SECTION("copy operations with spare capacity") + { + // reserve while the array is empty: reallocating non-empty + // elements behind the library's back leaves stale parent + // pointers with JSON_DIAGNOSTICS + json doc = json::array(); + doc.get_ref().reserve(100); + for (int i = 0; i < 5; ++i) + { + doc.push_back(copy_op); + } + 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 81c588544..0e225b86e 100644 --- a/tests/src/unit-json_pointer.cpp +++ b/tests/src/unit-json_pointer.cpp @@ -917,6 +917,46 @@ 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); + + const 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; + std::string quadruple = s + s; + quadruple += quadruple; + CHECK(ptr.to_string() == quadruple); + + // 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"};