mirror of
https://github.com/json-c/json-c.git
synced 2026-11-05 21:20:16 +08:00
json_patch: fix move/copy false rejection between sibling keys sharing a prefix
json_patch_apply_move_copy() checks that "from" is not a proper prefix of "path" per RFC 6902 section 4.4 (a location cannot be moved into one of its own children), but it did so with a plain strncmp() over the raw pointer strings, without checking that the shared run of characters actually ends on a JSON Pointer reference-token boundary in "path". As a result, moving or copying between two sibling object keys where one name is a textual prefix of the other, e.g. "/aa" to "/aab", was rejected with "Invalid attempt to move parent under a child" even though "/aa" is not an ancestor pointer of "/aab" at all, just an unrelated key that happens to share a few leading characters. The fix requires that the character in "path" right after the shared "from" prefix be either the end of the string (equal paths, a no-op) or a '/' (a genuine nested reference token), before treating it as an attempt to move a location under its own descendant. Added two cases to json_patch_tests.json covering the move and copy operations between such sibling keys, and refreshed the corresponding golden test_json_patch.expected output.
This commit is contained in:
1 parent
25895a587e
commit
1b504e26ab
3 files changed
+24
-2
No files matched your search
+12
-2
@@ -215,7 +215,17 @@ static int json_patch_apply_move_copy(struct json_object **res,
|
|||||||
}
|
}
|
||||||
|
|
||||||
from_s_len = strlen(from_s);
|
from_s_len = strlen(from_s);
|
||||||
if (strncmp(from_s, path, from_s_len) == 0) {
|
/**
|
||||||
|
* "from" is only a (possibly improper) prefix of "path" when the
|
||||||
|
* shared run of characters ends on a reference-token boundary in
|
||||||
|
* "path", i.e. the next character is '/' or the strings end there.
|
||||||
|
* Without the boundary check, "from" being a plain string prefix of
|
||||||
|
* "path" (e.g. "/aa" and "/aab") was mistaken for "from" being an
|
||||||
|
* ancestor pointer of "path", rejecting moves/copies between mere
|
||||||
|
* sibling keys that happen to share a name prefix.
|
||||||
|
*/
|
||||||
|
if (strncmp(from_s, path, from_s_len) == 0 &&
|
||||||
|
(path[from_s_len] == '\0' || path[from_s_len] == '/')) {
|
||||||
/**
|
/**
|
||||||
* If lengths match, it's a noop, if they don't,
|
* If lengths match, it's a noop, if they don't,
|
||||||
* then we're trying to move a parent under a child
|
* then we're trying to move a parent under a child
|
||||||
@@ -223,7 +233,7 @@ static int json_patch_apply_move_copy(struct json_object **res,
|
|||||||
* The "from" location MUST NOT be a proper prefix of the "path"
|
* The "from" location MUST NOT be a proper prefix of the "path"
|
||||||
* location; i.e., a location cannot be moved into one of its children.
|
* location; i.e., a location cannot be moved into one of its children.
|
||||||
*/
|
*/
|
||||||
if (from_s_len == strlen(path))
|
if (path[from_s_len] == '\0')
|
||||||
return 0;
|
return 0;
|
||||||
_set_err(EINVAL, "Invalid attempt to move parent under a child");
|
_set_err(EINVAL, "Invalid attempt to move parent under a child");
|
||||||
return -1;
|
return -1;
|
||||||
|
|||||||
@@ -370,6 +370,16 @@
|
|||||||
"patch": [{"op": "move", "from": "/foo", "path": "/foo"}],
|
"patch": [{"op": "move", "from": "/foo", "path": "/foo"}],
|
||||||
"expected": {"foo": 1} },
|
"expected": {"foo": 1} },
|
||||||
|
|
||||||
|
{ "comment": "Move between sibling keys sharing a name prefix is not a parent/child move",
|
||||||
|
"doc": {"aa": 1, "aab": 2},
|
||||||
|
"patch": [{"op": "move", "from": "/aa", "path": "/aab"}],
|
||||||
|
"expected": {"aab": 1} },
|
||||||
|
|
||||||
|
{ "comment": "Copy between sibling keys sharing a name prefix is not a parent/child move",
|
||||||
|
"doc": {"aa": 1, "aab": 2},
|
||||||
|
"patch": [{"op": "copy", "from": "/aa", "path": "/aab"}],
|
||||||
|
"expected": {"aa": 1, "aab": 1} },
|
||||||
|
|
||||||
{ "doc": {"foo": 1, "baz": [{"qux": "hello"}]},
|
{ "doc": {"foo": 1, "baz": [{"qux": "hello"}]},
|
||||||
"patch": [{"op": "move", "from": "/foo", "path": "/bar"}],
|
"patch": [{"op": "move", "from": "/foo", "path": "/bar"}],
|
||||||
"expected": {"baz": [{"qux": "hello"}], "bar": 1} },
|
"expected": {"baz": [{"qux": "hello"}], "bar": 1} },
|
||||||
|
|||||||
@@ -100,6 +100,8 @@ Testing 'Test the whole document, no match', doc '{ "foo": 1 }' patch '[ { "op":
|
|||||||
Testing 'Empty-string element', doc '{ "": 1 }' patch '[ { "op": "test", "path": "\/", "value": 1 } ]' : OK
|
Testing 'Empty-string element', doc '{ "": 1 }' patch '[ { "op": "test", "path": "\/", "value": 1 } ]' : OK
|
||||||
Testing '(null)', doc '{ "foo": [ "bar", "baz" ], "": 0, "a\/b": 1, "c%d": 2, "e^f": 3, "g|h": 4, "i\\j": 5, "k\"l": 6, " ": 7, "m~n": 8 }' patch '[ { "op": "test", "path": "\/foo", "value": [ "bar", "baz" ] }, { "op": "test", "path": "\/foo\/0", "value": "bar" }, { "op": "test", "path": "\/", "value": 0 }, { "op": "test", "path": "\/a~1b", "value": 1 }, { "op": "test", "path": "\/c%d", "value": 2 }, { "op": "test", "path": "\/e^f", "value": 3 }, { "op": "test", "path": "\/g|h", "value": 4 }, { "op": "test", "path": "\/i\\j", "value": 5 }, { "op": "test", "path": "\/k\"l", "value": 6 }, { "op": "test", "path": "\/ ", "value": 7 }, { "op": "test", "path": "\/m~0n", "value": 8 } ]' : OK
|
Testing '(null)', doc '{ "foo": [ "bar", "baz" ], "": 0, "a\/b": 1, "c%d": 2, "e^f": 3, "g|h": 4, "i\\j": 5, "k\"l": 6, " ": 7, "m~n": 8 }' patch '[ { "op": "test", "path": "\/foo", "value": [ "bar", "baz" ] }, { "op": "test", "path": "\/foo\/0", "value": "bar" }, { "op": "test", "path": "\/", "value": 0 }, { "op": "test", "path": "\/a~1b", "value": 1 }, { "op": "test", "path": "\/c%d", "value": 2 }, { "op": "test", "path": "\/e^f", "value": 3 }, { "op": "test", "path": "\/g|h", "value": 4 }, { "op": "test", "path": "\/i\\j", "value": 5 }, { "op": "test", "path": "\/k\"l", "value": 6 }, { "op": "test", "path": "\/ ", "value": 7 }, { "op": "test", "path": "\/m~0n", "value": 8 } ]' : OK
|
||||||
Testing 'Move to same location has no effect', doc '{ "foo": 1 }' patch '[ { "op": "move", "from": "\/foo", "path": "\/foo" } ]' : OK
|
Testing 'Move to same location has no effect', doc '{ "foo": 1 }' patch '[ { "op": "move", "from": "\/foo", "path": "\/foo" } ]' : OK
|
||||||
|
Testing 'Move between sibling keys sharing a name prefix is not a parent/child move', doc '{ "aa": 1, "aab": 2 }' patch '[ { "op": "move", "from": "\/aa", "path": "\/aab" } ]' : OK
|
||||||
|
Testing 'Copy between sibling keys sharing a name prefix is not a parent/child move', doc '{ "aa": 1, "aab": 2 }' patch '[ { "op": "copy", "from": "\/aa", "path": "\/aab" } ]' : OK
|
||||||
Testing '(null)', doc '{ "foo": 1, "baz": [ { "qux": "hello" } ] }' patch '[ { "op": "move", "from": "\/foo", "path": "\/bar" } ]' : OK
|
Testing '(null)', doc '{ "foo": 1, "baz": [ { "qux": "hello" } ] }' patch '[ { "op": "move", "from": "\/foo", "path": "\/bar" } ]' : OK
|
||||||
Testing '(null)', doc '{ "baz": [ { "qux": "hello" } ], "bar": 1 }' patch '[ { "op": "move", "from": "\/baz\/0\/qux", "path": "\/baz\/1" } ]' : OK
|
Testing '(null)', doc '{ "baz": [ { "qux": "hello" } ], "bar": 1 }' patch '[ { "op": "move", "from": "\/baz\/0\/qux", "path": "\/baz\/1" } ]' : OK
|
||||||
Testing '(null)', doc '{ "baz": [ { "qux": "hello" } ], "bar": 1 }' patch '[ { "op": "copy", "from": "\/baz\/0", "path": "\/boo" } ]' : OK
|
Testing '(null)', doc '{ "baz": [ { "qux": "hello" } ], "bar": 1 }' patch '[ { "op": "copy", "from": "\/baz\/0", "path": "\/boo" } ]' : OK
|
||||||
|
|||||||
Reference in new issue
Block a user