mirror of
https://github.com/nlohmann/json.git
synced 2026-09-09 17:58:00 +00:00
Reject JSON Patch move when from is a proper prefix of path (#5497)
* Reject JSON Patch move when from is a proper prefix of path RFC 6902 (section 4.4) forbids "from" from being a proper prefix of "path" for a "move" operation: "a location cannot be moved into one of its children." "move" is implemented as remove-then-add with no check for this. For object targets, the subsequent "add" happened to throw as a side effect of resolving through the now-removed parent, but for array targets, removing the "from" element shifts subsequent indices, so "path" silently re-resolves to a different element and the operation "succeeds" with a silently corrupted document. Add a check, before performing the remove/add, for whether "from" is a proper prefix of "path" at the reference-token level. This compares json_pointer's already-unescaped reference_tokens vectors (basic_json is a friend of json_pointer) rather than the raw pointer strings, so that tokens containing escaped '/' or '~' characters are compared correctly, and a token that merely looks like a string prefix (e.g. "/ab" vs "/abc/x") is not mistaken for a pointer-token prefix. When "from" is a proper prefix of "path", throw out_of_range.414. Fixes #5397. Stacked on top of the fix for #5396 (branch issue-5396-patch-remove-primitive-parent), since both touch the same patch_inplace move/remove handling in include/nlohmann/json.hpp. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Add root-pointer and array-append-token edge case tests for the move prefix check Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Replace std::equal with an explicitly-bounded loop in the move prefix check The three-iterator std::equal(first1, last1, first2) form has no explicit end iterator for the second range, which a static analyzer (Flawfinder, CWE-126) flags as a potential over-read even though the preceding size comparison already guarantees the second range is long enough. Rather than argue the point, make the bound visible in the code itself via an explicit loop -- every access to ptr.reference_tokens is now guarded by the same index the loop condition bounds against from_size. (The C++14 four-iterator std::equal(first1, last1, first2, last2) form was tried first as a more minimal fix, but this codebase targets C++11 and that overload is not safely usable under -std=c++11 with all supported standard library implementations.) Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Extract the move prefix check into a named helper lambda Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Account for JSON_DIAGNOSTIC_POSITIONS in the move-prefix-check error messages out_of_range::create() includes a "(bytes X-Y)" position annotation when JSON_DIAGNOSTIC_POSITIONS is enabled, which the ci_test_diagnostic_positions CI job builds the whole suite with. The five new out_of_range.414 assertions only checked the annotation-free message. Confirmed JSON_DIAGNOSTICS produces the same (annotation-free) message as the default build for this particular throw site (its path-based annotation is empty at the root, where &result always points here), so only two message variants are needed, not three. Signed-off-by: Niels Lohmann <mail@nlohmann.me> --------- Signed-off-by: Niels Lohmann <mail@nlohmann.me>
This commit is contained in:
Niels Lohmann
@@ -36,6 +36,8 @@ Strong guarantee: if an exception is thrown, there are no changes in the JSON va
|
||||
location has a parent that is neither an object nor an array.
|
||||
- Throws [`out_of_range.413`](../../home/exceptions.md#jsonexceptionout_of_range413) if a "remove" operation's target
|
||||
location has a parent that is neither an object nor an array.
|
||||
- Throws [`out_of_range.414`](../../home/exceptions.md#jsonexceptionout_of_range414) if a "move" operation's "from"
|
||||
location is a proper prefix of its "path" location.
|
||||
- Throws [`other_error.501`](../../home/exceptions.md#jsonexceptionother_error501) if "test" operation was
|
||||
unsuccessful.
|
||||
|
||||
@@ -79,3 +81,5 @@ is thrown. In any case, the original value is not changed: the patch is applied
|
||||
target location has a non-object/non-array parent in version 3.13.0.
|
||||
- Added [`out_of_range.413`](../../home/exceptions.md#jsonexceptionout_of_range413) and stopped silently ignoring a "remove" operation whose target
|
||||
location has a non-object/non-array parent in version 3.13.0.
|
||||
- Added [`out_of_range.414`](../../home/exceptions.md#jsonexceptionout_of_range414) and rejected a "move" operation whose "from" location is a proper
|
||||
prefix of its "path" location instead of silently producing a corrupted result in version 3.13.0.
|
||||
|
||||
@@ -32,6 +32,8 @@ No guarantees, value may be corrupted by an unsuccessful patch operation.
|
||||
location has a parent that is neither an object nor an array.
|
||||
- Throws [`out_of_range.413`](../../home/exceptions.md#jsonexceptionout_of_range413) if a "remove" operation's target
|
||||
location has a parent that is neither an object nor an array.
|
||||
- Throws [`out_of_range.414`](../../home/exceptions.md#jsonexceptionout_of_range414) if a "move" operation's "from"
|
||||
location is a proper prefix of its "path" location.
|
||||
- Throws [`other_error.501`](../../home/exceptions.md#jsonexceptionother_error501) if "test" operation was
|
||||
unsuccessful.
|
||||
|
||||
@@ -76,3 +78,5 @@ function throws an exception.
|
||||
target location has a non-object/non-array parent in version 3.13.0.
|
||||
- Added [`out_of_range.413`](../../home/exceptions.md#jsonexceptionout_of_range413) and stopped silently ignoring a "remove" operation whose target
|
||||
location has a non-object/non-array parent in version 3.13.0.
|
||||
- Added [`out_of_range.414`](../../home/exceptions.md#jsonexceptionout_of_range414) and rejected a "move" operation whose "from" location is a proper
|
||||
prefix of its "path" location instead of silently producing a corrupted result in version 3.13.0.
|
||||
|
||||
@@ -947,6 +947,20 @@ A JSON Patch `remove` operation cannot be applied because the target location's
|
||||
|
||||
This exception was added in version 3.13.0. Before that, this situation was silently ignored (the `remove` operation had no effect).
|
||||
|
||||
### json.exception.out_of_range.414
|
||||
|
||||
A JSON Patch `move` operation's `"from"` location is a proper prefix of its `"path"` location. Per [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902) (section 4.4), a location cannot be moved into one of its own children.
|
||||
|
||||
!!! failure "Example message"
|
||||
|
||||
```
|
||||
cannot move value: 'from' path '/0' is a proper prefix of 'path' '/0/0'
|
||||
```
|
||||
|
||||
!!! note
|
||||
|
||||
This exception was added in version 3.13.0. Before that, this situation could succeed with a corrupted result: for an array target, removing the "from" element before the "add" step shifted subsequent indices, so "path" silently re-resolved to a different element than intended.
|
||||
|
||||
## Further exceptions
|
||||
|
||||
This exception is thrown in case of errors that cannot be classified with the
|
||||
|
||||
@@ -4948,6 +4948,30 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
|
||||
}
|
||||
};
|
||||
|
||||
// RFC 6902 (section 4.4) forbids "from" from being a proper prefix
|
||||
// of "path" for a "move" operation: a location cannot be moved into
|
||||
// one of its own children. Compares reference tokens (already
|
||||
// unescaped by json_pointer's parser) rather than the raw pointer
|
||||
// strings, since a token may itself contain an escaped '/' or '~'
|
||||
// that would defeat a naive string-prefix comparison. "from" equal
|
||||
// to "path" is *not* a proper prefix and must return false.
|
||||
const auto is_proper_prefix = [](const json_pointer & from, const json_pointer & to)
|
||||
{
|
||||
const auto from_size = from.reference_tokens.size();
|
||||
if (from_size >= to.reference_tokens.size())
|
||||
{
|
||||
return false;
|
||||
}
|
||||
for (std::size_t i = 0; i < from_size; ++i)
|
||||
{
|
||||
if (!(from.reference_tokens[i] == to.reference_tokens[i]))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
};
|
||||
|
||||
// type check: top level value must be an array
|
||||
if (JSON_HEDLEY_UNLIKELY(!json_patch.is_array()))
|
||||
{
|
||||
@@ -5023,6 +5047,11 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
|
||||
const auto from_path = get_value("move", "from", true).template get<string_t>();
|
||||
json_pointer from_ptr(from_path);
|
||||
|
||||
if (JSON_HEDLEY_UNLIKELY(is_proper_prefix(from_ptr, ptr)))
|
||||
{
|
||||
JSON_THROW(out_of_range::create(414, detail::concat("cannot move value: 'from' path '", from_path, "' is a proper prefix of 'path' '", path, "'"), &result));
|
||||
}
|
||||
|
||||
// the "from" location must exist - use at()
|
||||
basic_json const v = result.at(from_ptr);
|
||||
|
||||
|
||||
@@ -26502,6 +26502,30 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
|
||||
}
|
||||
};
|
||||
|
||||
// RFC 6902 (section 4.4) forbids "from" from being a proper prefix
|
||||
// of "path" for a "move" operation: a location cannot be moved into
|
||||
// one of its own children. Compares reference tokens (already
|
||||
// unescaped by json_pointer's parser) rather than the raw pointer
|
||||
// strings, since a token may itself contain an escaped '/' or '~'
|
||||
// that would defeat a naive string-prefix comparison. "from" equal
|
||||
// to "path" is *not* a proper prefix and must return false.
|
||||
const auto is_proper_prefix = [](const json_pointer & from, const json_pointer & to)
|
||||
{
|
||||
const auto from_size = from.reference_tokens.size();
|
||||
if (from_size >= to.reference_tokens.size())
|
||||
{
|
||||
return false;
|
||||
}
|
||||
for (std::size_t i = 0; i < from_size; ++i)
|
||||
{
|
||||
if (!(from.reference_tokens[i] == to.reference_tokens[i]))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
};
|
||||
|
||||
// type check: top level value must be an array
|
||||
if (JSON_HEDLEY_UNLIKELY(!json_patch.is_array()))
|
||||
{
|
||||
@@ -26577,6 +26601,11 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
|
||||
const auto from_path = get_value("move", "from", true).template get<string_t>();
|
||||
json_pointer from_ptr(from_path);
|
||||
|
||||
if (JSON_HEDLEY_UNLIKELY(is_proper_prefix(from_ptr, ptr)))
|
||||
{
|
||||
JSON_THROW(out_of_range::create(414, detail::concat("cannot move value: 'from' path '", from_path, "' is a proper prefix of 'path' '", path, "'"), &result));
|
||||
}
|
||||
|
||||
// the "from" location must exist - use at()
|
||||
basic_json const v = result.at(from_ptr);
|
||||
|
||||
|
||||
@@ -1444,6 +1444,137 @@ TEST_CASE("JSON patch - remove with primitive or null parent (regression #5396)"
|
||||
}
|
||||
}
|
||||
|
||||
TEST_CASE("JSON patch - move where 'from' is a proper prefix of 'path' (regression #5397)")
|
||||
{
|
||||
// Regression test for https://github.com/nlohmann/json/issues/5397
|
||||
//
|
||||
// RFC 6902 (§4.4) forbids "from" from being a proper prefix of "path"
|
||||
// for a "move" operation: "a location cannot be moved into one of its
|
||||
// children." "move" is implemented as remove-then-add; for an object
|
||||
// target this happened to throw anyway as a side effect of the "add"
|
||||
// step re-resolving through the now-removed parent, but for an array
|
||||
// target the removal shifted subsequent indices, so "path" silently
|
||||
// re-resolved to a different element and the operation "succeeded"
|
||||
// with a corrupted result. It now throws out_of_range.414 for both
|
||||
// object and array targets.
|
||||
|
||||
SECTION("array target (from the issue)")
|
||||
{
|
||||
json const doc = R"([[1,2],[3]])"_json;
|
||||
json const patch = {{{"op", "move"}, {"from", "/0"}, {"path", "/0/0"}}};
|
||||
#if JSON_DIAGNOSTIC_POSITIONS
|
||||
CHECK_THROWS_WITH_AS(doc.patch(patch), "[json.exception.out_of_range.414] (bytes 0-11) cannot move value: 'from' path '/0' is a proper prefix of 'path' '/0/0'", json::out_of_range&);
|
||||
#else
|
||||
CHECK_THROWS_WITH_AS(doc.patch(patch), "[json.exception.out_of_range.414] cannot move value: 'from' path '/0' is a proper prefix of 'path' '/0/0'", json::out_of_range&);
|
||||
#endif
|
||||
}
|
||||
|
||||
SECTION("object target")
|
||||
{
|
||||
json const doc = R"({"a": {"b": 1}})"_json;
|
||||
json const patch = {{{"op", "move"}, {"from", "/a"}, {"path", "/a/b"}}};
|
||||
#if JSON_DIAGNOSTIC_POSITIONS
|
||||
CHECK_THROWS_WITH_AS(doc.patch(patch), "[json.exception.out_of_range.414] (bytes 0-15) cannot move value: 'from' path '/a' is a proper prefix of 'path' '/a/b'", json::out_of_range&);
|
||||
#else
|
||||
CHECK_THROWS_WITH_AS(doc.patch(patch), "[json.exception.out_of_range.414] cannot move value: 'from' path '/a' is a proper prefix of 'path' '/a/b'", json::out_of_range&);
|
||||
#endif
|
||||
}
|
||||
|
||||
SECTION("from == path is not a proper prefix and must not be rejected")
|
||||
{
|
||||
// "from" equal to "path" is a no-op move; it is not a *proper*
|
||||
// prefix relationship, so this new check must not reject it.
|
||||
json const doc = R"({"a": 1, "b": 2})"_json;
|
||||
json const patch = {{{"op", "move"}, {"from", "/a"}, {"path", "/a"}}};
|
||||
CHECK(doc.patch(patch) == doc);
|
||||
}
|
||||
|
||||
SECTION("raw string prefix that is not a pointer-token prefix must be allowed")
|
||||
{
|
||||
// "/ab" is a string-prefix of "/abc/x" as raw text, but "ab" and
|
||||
// "abc" are different reference tokens, so this is NOT a
|
||||
// pointer-token prefix relationship and the move must succeed.
|
||||
// This is the key case proving the check compares tokens, not
|
||||
// raw pointer text (a naive std::string prefix/rfind check on
|
||||
// the undecoded pointer would wrongly reject this).
|
||||
json const doc = R"({"ab": 1, "abc": {"x": 2}})"_json;
|
||||
json const patch = {{{"op", "move"}, {"from", "/ab"}, {"path", "/abc/x"}}};
|
||||
json const result = R"({"abc": {"x": 1}})"_json;
|
||||
CHECK(doc.patch(patch) == result);
|
||||
}
|
||||
|
||||
SECTION("escaped reference tokens are compared unescaped")
|
||||
{
|
||||
// "from" is the single token "a/b" (escaped as "a~1b"); "path"
|
||||
// addresses member "x" of that same value, so "from" is a
|
||||
// proper (token-level) prefix of "path" and must be rejected.
|
||||
json const doc = R"({"a/b": {"x": 1}})"_json;
|
||||
json const patch = {{{"op", "move"}, {"from", "/a~1b"}, {"path", "/a~1b/x"}}};
|
||||
#if JSON_DIAGNOSTIC_POSITIONS
|
||||
CHECK_THROWS_WITH_AS(doc.patch(patch), "[json.exception.out_of_range.414] (bytes 0-17) cannot move value: 'from' path '/a~1b' is a proper prefix of 'path' '/a~1b/x'", json::out_of_range&);
|
||||
#else
|
||||
CHECK_THROWS_WITH_AS(doc.patch(patch), "[json.exception.out_of_range.414] cannot move value: 'from' path '/a~1b' is a proper prefix of 'path' '/a~1b/x'", json::out_of_range&);
|
||||
#endif
|
||||
}
|
||||
|
||||
SECTION("ordinary valid moves still work")
|
||||
{
|
||||
// unrelated top-level members
|
||||
json const doc1 = R"({"a": 1, "b": 2})"_json;
|
||||
json const patch1 = {{{"op", "move"}, {"from", "/a"}, {"path", "/c"}}};
|
||||
CHECK(doc1.patch(patch1) == R"({"b": 2, "c": 1})"_json);
|
||||
|
||||
// sibling paths that share a textual prefix but are unrelated
|
||||
json const doc2 = R"({"a": {"x": 1}, "b": {"y": 2}})"_json;
|
||||
json const patch2 = {{{"op", "move"}, {"from", "/a/x"}, {"path", "/b/z"}}};
|
||||
CHECK(doc2.patch(patch2) == R"({"a": {}, "b": {"y": 2, "z": 1}})"_json);
|
||||
|
||||
// "path" is a proper prefix of "from" (the reverse relationship,
|
||||
// which RFC 6902 does not forbid)
|
||||
json const doc3 = R"({"a": {"b": 1}})"_json;
|
||||
json const patch3 = {{{"op", "move"}, {"from", "/a/b"}, {"path", "/a"}}};
|
||||
CHECK(doc3.patch(patch3) == R"({"a": 1})"_json);
|
||||
}
|
||||
|
||||
SECTION("root 'from' is a proper prefix of every non-root 'path'")
|
||||
{
|
||||
// the whole document is a proper prefix of any location inside it
|
||||
json const doc = R"({"a": 1})"_json;
|
||||
json const patch = {{{"op", "move"}, {"from", ""}, {"path", "/a"}}};
|
||||
#if JSON_DIAGNOSTIC_POSITIONS
|
||||
CHECK_THROWS_WITH_AS(doc.patch(patch), "[json.exception.out_of_range.414] (bytes 0-8) cannot move value: 'from' path '' is a proper prefix of 'path' '/a'", json::out_of_range&);
|
||||
#else
|
||||
CHECK_THROWS_WITH_AS(doc.patch(patch), "[json.exception.out_of_range.414] cannot move value: 'from' path '' is a proper prefix of 'path' '/a'", json::out_of_range&);
|
||||
#endif
|
||||
}
|
||||
|
||||
SECTION("root 'path' is never a proper prefix violation for a non-root 'from'")
|
||||
{
|
||||
// the reverse of the above: moving a non-root location to the root
|
||||
// is the "path is a prefix of from" relationship, which RFC 6902
|
||||
// permits (already covered generally above; this pins the root
|
||||
// case specifically, since root is the one path with no reference
|
||||
// tokens at all)
|
||||
json const doc = R"({"a": {"b": 1}})"_json;
|
||||
json const patch = {{{"op", "move"}, {"from", "/a"}, {"path", ""}}};
|
||||
CHECK(doc.patch(patch) == R"({"b": 1})"_json);
|
||||
}
|
||||
|
||||
SECTION("the array-append token '-' is an ordinary child token")
|
||||
{
|
||||
// "-" (append-to-array) addresses a location *inside* the array,
|
||||
// so "from" pointing at the array is still a proper prefix of
|
||||
// "path" ending in "-" and must be rejected like any other child.
|
||||
json const doc = R"({"a": [1, 2]})"_json;
|
||||
json const patch = {{{"op", "move"}, {"from", "/a"}, {"path", "/a/-"}}};
|
||||
#if JSON_DIAGNOSTIC_POSITIONS
|
||||
CHECK_THROWS_WITH_AS(doc.patch(patch), "[json.exception.out_of_range.414] (bytes 0-13) cannot move value: 'from' path '/a' is a proper prefix of 'path' '/a/-'", json::out_of_range&);
|
||||
#else
|
||||
CHECK_THROWS_WITH_AS(doc.patch(patch), "[json.exception.out_of_range.414] cannot move value: 'from' path '/a' is a proper prefix of 'path' '/a/-'", json::out_of_range&);
|
||||
#endif
|
||||
}
|
||||
}
|
||||
|
||||
TEST_CASE("JSON patch - diff emits array removals in descending index order")
|
||||
{
|
||||
SECTION("array shrunk to empty")
|
||||
|
||||
Reference in New Issue
Block a user