mirror of
https://github.com/nlohmann/json.git
synced 2026-10-29 00:55:44 +00:00
Fix self-aliasing in json_pointer::operator/= and patch_inplace (#5936)
* Fix self-aliasing in json_pointer::operator/= and patch_inplace json_pointer::operator/=(const json_pointer&) passed iterators into its own token vector to insert() when a pointer was appended to itself (p /= p), which is undefined behavior; libstdc++ produced "/a/b//" for "/a/b". The pointer now appends a copy in that case. basic_json::patch_inplace iterated the patch while changing *this, so j.patch_inplace(j) read freed memory. It now applies a copy of the patch when the patch is the value itself. A patch stored inside the value remains undefined behavior and is documented as such; copying the patch unconditionally would double the time of a large patch. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Fix JSON_DIAGNOSTICS test crash and clang-tidy hit in patch_inplace self-aliasing The spare-capacity test reallocated non-empty elements through get_ref(), which leaves stale parent pointers under JSON_DIAGNOSTICS; reserve while the array is empty instead. Suppress performance-unnecessary-copy-initialization for the copy that is needed because *this is modified. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Detect a patch stored inside the patched value with JSON_DIAGNOSTICS patch_inplace applied a copy only when the patch was the value itself. With JSON_DIAGNOSTICS the parent pointers make it cheap to also detect a patch nested inside the value, so that case is handled the same way. Without diagnostics it stays documented undefined behavior. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Silence misc-const-correctness for patch_aliases_this Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Fix clang-tidy findings in the operator/= self-append test Signed-off-by: Niels Lohmann <mail@nlohmann.me> --------- Signed-off-by: Niels Lohmann <mail@nlohmann.me>
This commit is contained in:
Niels Lohmann authored and 
GitHub committed
1 parent
2120927a7a
commit
091c10d4a5
7 files changed
+234
-2
No files matched your search
@@ -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<json::array_t&>().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")
|
||||
|
||||
@@ -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<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);
|
||||
|
||||
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"};
|
||||
|
||||
Reference in new issue
Block a user