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>
This commit is contained in:
6 files changed
+77
-2
No files matched your search
@@ -39,7 +39,7 @@ Linear in the lengths of `apply_patch`.
|
|||||||
|
|
||||||
## Notes
|
## Notes
|
||||||
|
|
||||||
!!! danger "Undefined behavior"
|
!!! danger "Undefined behavior and runtime assertions"
|
||||||
|
|
||||||
`merge_patch()` reads `apply_patch` while it modifies `#!cpp *this`. `apply_patch` must not be `#!cpp *this`
|
`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
|
itself and must not refer to a value contained in `#!cpp *this` (for example, a subobject returned by
|
||||||
@@ -51,6 +51,9 @@ Linear in the lengths of `apply_patch`.
|
|||||||
j.merge_patch(json(j)); // instead of j.merge_patch(j)
|
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.
|
See [GitHub issue #5641](https://github.com/nlohmann/json/issues/5641) for more information.
|
||||||
|
|
||||||
## Examples
|
## Examples
|
||||||
|
|||||||
@@ -61,7 +61,7 @@ Basic guarantee: if an exception is thrown during the operation, the JSON value
|
|||||||
|
|
||||||
## Notes
|
## Notes
|
||||||
|
|
||||||
!!! danger "Undefined behavior"
|
!!! danger "Undefined behavior and runtime assertions"
|
||||||
|
|
||||||
Both overloads read the argument while they modify `#!cpp *this`. The argument `j` (or, for overload (2), the
|
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
|
range `[first, last)`) must not be `#!cpp *this` itself and must not refer to a value contained in
|
||||||
@@ -73,6 +73,9 @@ Basic guarantee: if an exception is thrown during the operation, the JSON value
|
|||||||
j.update(json(j["defaults"])); // instead of j.update(j["defaults"])
|
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.
|
See [GitHub issue #5641](https://github.com/nlohmann/json/issues/5641) for more information.
|
||||||
|
|
||||||
## Examples
|
## Examples
|
||||||
|
|||||||
@@ -103,6 +103,36 @@ behavior and yields a runtime assertion.
|
|||||||
Assertion failed: (m_object != nullptr), function operator++, file iter_impl.hpp, line 368.
|
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
|
## Changes
|
||||||
|
|
||||||
### Reading from a null `FILE` or `char` pointer
|
### Reading from a null `FILE` or `char` pointer
|
||||||
|
|||||||
@@ -4218,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));
|
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);
|
update_members(first, last, merge_objects, 0);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -6303,6 +6306,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
|
|||||||
/// @sa https://json.nlohmann.me/api/basic_json/merge_patch/
|
/// @sa https://json.nlohmann.me/api/basic_json/merge_patch/
|
||||||
void merge_patch(const basic_json& apply_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);
|
apply_merge_patch(apply_patch, 0);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -30299,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));
|
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);
|
update_members(first, last, merge_objects, 0);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -32384,6 +32387,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
|
|||||||
/// @sa https://json.nlohmann.me/api/basic_json/merge_patch/
|
/// @sa https://json.nlohmann.me/api/basic_json/merge_patch/
|
||||||
void merge_patch(const basic_json& apply_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);
|
apply_merge_patch(apply_patch, 0);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -41,6 +41,33 @@ TEST_CASE("JSON_ASSERT(x)")
|
|||||||
// check that assertion actually happened
|
// check that assertion actually happened
|
||||||
CHECK(assert_counter == 1);
|
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
|
#endif
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user