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 2c5d868b0..76d85844b 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -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(); 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); diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 008de6516..28e56f8ac 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -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(); 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); diff --git a/tests/src/unit-json_patch.cpp b/tests/src/unit-json_patch.cpp index 5d16e8900..257e455aa 100644 --- a/tests/src/unit-json_patch.cpp +++ b/tests/src/unit-json_patch.cpp @@ -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")