Compare commits

..
Author SHA1 Message Date
Niels Lohmann 95c1df59ef 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>
2026-09-06 17:35:51 +02:00
Niels Lohmann f3f7a16dfc Add root-pointer and array-append-token edge case tests for the move prefix check
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-09-05 22:23:34 +02:00
Niels Lohmann f826b1e9f9 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>
2026-09-05 22:14:56 +02:00
6 changed files with 183 additions and 0 deletions
+4
View File
@@ -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.
+14
View File
@@ -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
+25
View File
@@ -5022,6 +5022,31 @@ 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);
// RFC 6902 (section 4.4) forbids "from" from being a
// proper prefix of "path": a location cannot be moved
// into one of its own children. Compare the pointers'
// 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. This is
// written as an explicit, manually-bounded loop (rather
// than std::equal(first1, last1, first2), whose second
// range has no explicit end iterator) so every access to
// ptr.reference_tokens is visibly guarded by the same
// index the loop condition already bounds against
// from_size -- from_size < ptr.reference_tokens.size()
// is checked once, up front, before the loop runs at all.
const auto from_size = from_ptr.reference_tokens.size();
bool from_is_proper_prefix_of_path = from_size < ptr.reference_tokens.size();
for (std::size_t i = 0; from_is_proper_prefix_of_path && i < from_size; ++i)
{
from_is_proper_prefix_of_path = from_ptr.reference_tokens[i] == ptr.reference_tokens[i];
}
if (JSON_HEDLEY_UNLIKELY(from_is_proper_prefix_of_path))
{
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);
+25
View File
@@ -26450,6 +26450,31 @@ 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);
// RFC 6902 (section 4.4) forbids "from" from being a
// proper prefix of "path": a location cannot be moved
// into one of its own children. Compare the pointers'
// 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. This is
// written as an explicit, manually-bounded loop (rather
// than std::equal(first1, last1, first2), whose second
// range has no explicit end iterator) so every access to
// ptr.reference_tokens is visibly guarded by the same
// index the loop condition already bounds against
// from_size -- from_size < ptr.reference_tokens.size()
// is checked once, up front, before the loop runs at all.
const auto from_size = from_ptr.reference_tokens.size();
bool from_is_proper_prefix_of_path = from_size < ptr.reference_tokens.size();
for (std::size_t i = 0; from_is_proper_prefix_of_path && i < from_size; ++i)
{
from_is_proper_prefix_of_path = from_ptr.reference_tokens[i] == ptr.reference_tokens[i];
}
if (JSON_HEDLEY_UNLIKELY(from_is_proper_prefix_of_path))
{
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);
+111
View File
@@ -1444,6 +1444,117 @@ 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"}}};
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&);
}
SECTION("object target")
{
json const doc = R"({"a": {"b": 1}})"_json;
json const patch = {{{"op", "move"}, {"from", "/a"}, {"path", "/a/b"}}};
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&);
}
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"}}};
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&);
}
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"}}};
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&);
}
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/-"}}};
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&);
}
}
TEST_CASE("JSON patch - diff emits array removals in descending index order")
{
SECTION("array shrunk to empty")