From f826b1e9f9a22471de0cea30344f88c13b4eb1f8 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Sat, 5 Sep 2026 22:14:56 +0200 Subject: [PATCH] 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 --- docs/mkdocs/docs/api/basic_json/patch.md | 4 + .../docs/api/basic_json/patch_inplace.md | 4 + docs/mkdocs/docs/home/exceptions.md | 14 ++++ include/nlohmann/json.hpp | 13 +++ single_include/nlohmann/json.hpp | 13 +++ tests/src/unit-json_patch.cpp | 81 +++++++++++++++++++ 6 files changed, 129 insertions(+) diff --git a/docs/mkdocs/docs/api/basic_json/patch.md b/docs/mkdocs/docs/api/basic_json/patch.md index 0deadc25a..fa25b2699 100644 --- a/docs/mkdocs/docs/api/basic_json/patch.md +++ b/docs/mkdocs/docs/api/basic_json/patch.md @@ -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. diff --git a/docs/mkdocs/docs/api/basic_json/patch_inplace.md b/docs/mkdocs/docs/api/basic_json/patch_inplace.md index 99af445d9..7ae85aaaa 100644 --- a/docs/mkdocs/docs/api/basic_json/patch_inplace.md +++ b/docs/mkdocs/docs/api/basic_json/patch_inplace.md @@ -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. diff --git a/docs/mkdocs/docs/home/exceptions.md b/docs/mkdocs/docs/home/exceptions.md index 806b9887e..8c6649ef2 100644 --- a/docs/mkdocs/docs/home/exceptions.md +++ b/docs/mkdocs/docs/home/exceptions.md @@ -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 diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 950c7efc9..59d7a879a 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -5022,6 +5022,19 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const auto from_path = get_value("move", "from", true).template get(); 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. + if (JSON_HEDLEY_UNLIKELY(from_ptr.reference_tokens.size() < ptr.reference_tokens.size() + && std::equal(from_ptr.reference_tokens.begin(), from_ptr.reference_tokens.end(), ptr.reference_tokens.begin()))) + { + 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); diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index a257b79ab..687a148a9 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -26450,6 +26450,19 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec const auto from_path = get_value("move", "from", true).template get(); 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. + if (JSON_HEDLEY_UNLIKELY(from_ptr.reference_tokens.size() < ptr.reference_tokens.size() + && std::equal(from_ptr.reference_tokens.begin(), from_ptr.reference_tokens.end(), ptr.reference_tokens.begin()))) + { + 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); diff --git a/tests/src/unit-json_patch.cpp b/tests/src/unit-json_patch.cpp index 5d16e8900..d3f8fa8f5 100644 --- a/tests/src/unit-json_patch.cpp +++ b/tests/src/unit-json_patch.cpp @@ -1444,6 +1444,87 @@ 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); + } +} + TEST_CASE("JSON patch - diff emits array removals in descending index order") { SECTION("array shrunk to empty")