diff --git a/json_patch.c b/json_patch.c index dbc54ec..5bbc308 100644 --- a/json_patch.c +++ b/json_patch.c @@ -81,7 +81,10 @@ static int __json_patch_apply_remove(struct json_pointer_get_result *jpres) json_object_object_del(jpres->parent, jpres->key_in_parent); return 0; } else { - return json_object_put(jpres->obj); + // We're removing the root object + (void)json_object_put(jpres->obj); + jpres->obj = NULL; + return 0; } } @@ -99,6 +102,9 @@ static int json_patch_apply_remove(struct json_object **res, const char *path, s rc = __json_patch_apply_remove(&jpres); if (rc < 0) _set_err(EINVAL, "Unable to remove path referenced by 'path' field"); + // This means we removed and freed the root object, i.e. *res + if (jpres.parent == NULL) + *res = NULL; return rc; } @@ -223,6 +229,8 @@ static int json_patch_apply_move_copy(struct json_object **res, return rc; } + // Note: it's impossible for json_pointer to find the root obj, due + // to the path check above, so from.parent is guaranteed non-NULL json_object_get(from.obj); if (!move) { diff --git a/tests/json_patch_spec_tests.json b/tests/json_patch_spec_tests.json index c160535..d7d407d 100644 --- a/tests/json_patch_spec_tests.json +++ b/tests/json_patch_spec_tests.json @@ -192,8 +192,8 @@ "patch": [ { "op": "add", "path": "/baz", "value": "qux", "op": "remove" } ], - "error": "operation has two 'op' members", - "disabled": true + "error_wont_happen_in_jsonc": "operation has two 'op' members", + "error": "Did not find element referenced by path field" }, { diff --git a/tests/json_patch_tests.json b/tests/json_patch_tests.json index 4124d51..8cafb93 100644 --- a/tests/json_patch_tests.json +++ b/tests/json_patch_tests.json @@ -52,8 +52,8 @@ { "comment": "Toplevel scalar values OK?", "doc": "foo", "patch": [{"op": "replace", "path": "", "value": "bar"}], - "expected": "bar", - "disabled": true }, + "expected": "bar" + }, { "comment": "replace object document with array document?", "doc": {}, @@ -202,6 +202,55 @@ "patch": [{"op": "replace", "path": "", "value": {"baz": "qux"}}], "expected": {"baz": "qux"} }, + { "comment": "add whole document, null", + "doc": {}, + "Note1": "We can't pass null in to json_patch_apply, so start with _something_ and remove it", + "patch": [ + {"op": "remove", "path": ""}, + {"op": "add", "path": "", "value": {"baz": "qux"}} + ], + "expected": {"baz": "qux"} }, + + { "comment": "replace whole document, null", + "doc": {}, + "Note1": "We can't pass null in to json_patch_apply, so start with _something_ and remove it", + "patch": [ + {"op": "remove", "path": ""}, + {"op": "replace", "path": "", "value": {"baz": "qux"}} + ], + "error": "The spec says the target location must exist, so replacing a null document fails" + }, + + { "comment": "remove whole document", + "doc": {"foo": "bar"}, + "patch": [{"op": "remove", "path": ""}], + "expected": null }, + + { "comment": "remove whole document", + "doc": {"foo": "bar"}, + "patch": [{"op": "remove", "path": ""}], + "expected": null }, + + { "comment": "remove whole document, array", + "doc": ["foo", "bar"], + "patch": [{"op": "remove", "path": ""}], + "expected": null }, + + { "comment": "remove whole document, string", + "doc": "foo", + "patch": [{"op": "remove", "path": ""}], + "expected": null }, + + { "comment": "remove whole document, null", + "doc": {}, + "Note1": "We can't pass null in to json_patch_apply, so start with _something_ and remove it", + "patch": [ + {"op": "remove", "path": ""}, + {"op": "remove", "path": ""}, + ], + "error": "The spec says the target location must exist, so removing a null document fails" + }, + { "comment": "test replace with missing parent key should fail", "doc": {"bar": "baz"}, "patch": [{"op": "replace", "path": "/foo/bar", "value": false}], @@ -261,10 +310,16 @@ "patch": [{"op": "test", "path": "/foo", "value": [1, 2]}], "error": "test op should fail" }, - { "comment": "Whole document", + { "comment": "Test the whole document", "doc": { "foo": 1 }, "patch": [{"op": "test", "path": "", "value": {"foo": 1}}], - "disabled": true }, + "expected": { "foo": 1 } }, + + { "comment": "Test the whole document, no match", + "doc": { "foo": 1 }, + "patch": [{"op": "test", "path": "", "value": {"foo": 2}}], + "expected": { "foo": 1 }, + "error": "Tested value does not match original doc" }, { "comment": "Empty-string element", "doc": { "": 1 }, @@ -438,13 +493,13 @@ "patch": [ { "op": "move", "from": "/bar", "path": "/foo" } ], "error": "missing 'from' location" }, - { "comment": "duplicate ops", + { "comment": "duplicate ops, json-c parses this as op:move", "doc": { "foo": "bar" }, "patch": [ { "op": "add", "path": "/baz", "value": "qux", "op": "move", "from":"/foo" } ], - "error": "patch has two 'op' members", - "disabled_in_json_c": true, - "disabled": true }, + "error_wont_happen_in_jsonc": "patch has two 'op' members", + "expected": { "baz": "bar" } + }, { "comment": "unrecognized op should fail", "doc": {"foo": 1}, diff --git a/tests/test_json_patch.c b/tests/test_json_patch.c index d53c980..eeb791c 100644 --- a/tests/test_json_patch.c +++ b/tests/test_json_patch.c @@ -21,9 +21,9 @@ void test_json_patch_op(struct json_object *jo) const char *comment = json_object_get_string(json_object_object_get(jo, "comment")); struct json_object *doc = json_object_object_get(jo, "doc"); struct json_object *patch = json_object_object_get(jo, "patch"); - struct json_object *expected = json_object_object_get(jo, "expected"); + struct json_object *expected = NULL; + json_bool have_expected = json_object_object_get_ex(jo, "expected", &expected); struct json_object *error = json_object_object_get(jo, "error"); - int disabled_test = json_object_get_boolean(json_object_object_get(jo, "disabled_in_json_c")); const char *error_s = json_object_get_string(error); struct json_object *res = NULL; int ret; @@ -32,13 +32,9 @@ void test_json_patch_op(struct json_object *jo) comment ? comment : error_s, json_object_get_string(doc), json_object_get_string(patch)); - if (disabled_test) { - printf("SKIPPING - disabled in the test spec\n"); - return; - } - if (!error && !expected) { - printf("SKIPPING - no expected or error conditions in test\n"); - return; + if (!error && !have_expected) { + printf("BAD TEST - no expected or error conditions in test: %s\n", json_object_to_json_string(jo)); + assert(0); } fflush(stdout); struct json_patch_error jperr; @@ -54,13 +50,13 @@ void test_json_patch_op(struct json_object *jo) if (ret) { fprintf(stderr, "json_patch_apply() returned '%d'\n", ret); fprintf(stderr, "Expected: %s\n", json_object_get_string(expected)); - fprintf(stderr, "Got: %s\n", json_object_get_string(res)); + fprintf(stderr, "Got: %s\n", res ? json_object_get_string(res) : "(null)"); fprintf(stderr, "json_patch_apply failed: %s at patch idx %zu: %s\n", strerror(jperr.errno_code), jperr.patch_failure_idx, jperr.errmsg); fflush(stderr); assert(0); } - assert(res != NULL); + // Note: res might be NULL if the whole document was removed assert(jperr.errno_code == 0); ret = json_object_equal(expected, res); if (ret == 0) { @@ -83,6 +79,7 @@ void test_json_patch_using_file(const char *testdir, const char *filename) (void)snprintf(full_filename, sizeof(full_filename), "%s/%s", testdir, filename); size_t ii; + printf("Testing using file %s\n", filename); json_object *jo = json_object_from_file(full_filename); if (!jo) { fprintf(stderr, "FAIL: unable to open %s: %s\n", full_filename, strerror(errno)); diff --git a/tests/test_json_patch.expected b/tests/test_json_patch.expected index 9d2887a..523d276 100644 --- a/tests/test_json_patch.expected +++ b/tests/test_json_patch.expected @@ -1,3 +1,4 @@ +Testing using file json_patch_spec_tests.json Testing '4.1. add with missing object', doc '{ "q": { "bar": 2 } }' patch '[ { "op": "add", "path": "\/a\/b", "value": 1 } ]' : OK => json_patch_apply failed as expected: ERRNO=ENOENT at patch idx 0: Failed to set value at path referenced by 'path' field Testing 'A.1. Adding an Object Member', doc '{ "foo": "bar" }' patch '[ { "op": "add", "path": "\/baz", "value": "qux" } ]' : OK @@ -20,6 +21,7 @@ Testing 'A.14. ~ Escape Ordering', doc '{ "\/": 9, "~1": 10 }' patch '[ { "op": Testing 'A.15. Comparing Strings and Numbers', doc '{ "\/": 9, "~1": 10 }' patch '[ { "op": "test", "path": "\/~01", "value": "10" } ]' : OK => json_patch_apply failed as expected: ERRNO=ENOENT at patch idx 0: Value of element referenced by 'path' field did not match 'value' field Testing 'A.16. Adding an Array Value', doc '{ "foo": [ "bar" ] }' patch '[ { "op": "add", "path": "\/foo\/-", "value": [ "abc", "def" ] } ]' : OK +Testing using file json_patch_tests.json Testing 'empty list, empty docs', doc '{ }' patch '[ ]' : OK Testing 'empty patch list', doc '{ "foo": 1 }' patch '[ ]' : OK Testing 'rearrangements OK?', doc '{ "foo": 1, "bar": 2 }' patch '[ ]' : OK @@ -69,6 +71,15 @@ Testing '(null)', doc '[ "" ]' patch '[ { "op": "replace", "path": "\/0", "value Testing '(null)', doc '[ "" ]' patch '[ { "op": "replace", "path": "\/0", "value": null } ]' : OK Testing 'value in array replace not flattened', doc '[ "foo", "sil" ]' patch '[ { "op": "replace", "path": "\/1", "value": [ "bar", "baz" ] } ]' : OK Testing 'replace whole document', doc '{ "foo": "bar" }' patch '[ { "op": "replace", "path": "", "value": { "baz": "qux" } } ]' : OK +Testing 'add whole document, null', doc '{ }' patch '[ { "op": "remove", "path": "" }, { "op": "add", "path": "", "value": { "baz": "qux" } } ]' : OK +Testing 'replace whole document, null', doc '{ }' patch '[ { "op": "remove", "path": "" }, { "op": "replace", "path": "", "value": { "baz": "qux" } } ]' : OK + => json_patch_apply failed as expected: ERRNO=EINVAL at patch idx 1: Invalid path field +Testing 'remove whole document', doc '{ "foo": "bar" }' patch '[ { "op": "remove", "path": "" } ]' : OK +Testing 'remove whole document', doc '{ "foo": "bar" }' patch '[ { "op": "remove", "path": "" } ]' : OK +Testing 'remove whole document, array', doc '[ "foo", "bar" ]' patch '[ { "op": "remove", "path": "" } ]' : OK +Testing 'remove whole document, string', doc 'foo' patch '[ { "op": "remove", "path": "" } ]' : OK +Testing 'remove whole document, null', doc '{ }' patch '[ { "op": "remove", "path": "" }, { "op": "remove", "path": "" } ]' : OK + => json_patch_apply failed as expected: ERRNO=EINVAL at patch idx 1: Invalid path field Testing 'test replace with missing parent key should fail', doc '{ "bar": "baz" }' patch '[ { "op": "replace", "path": "\/foo\/bar", "value": false } ]' : OK => json_patch_apply failed as expected: ERRNO=ENOENT at patch idx 0: Did not find element referenced by path field Testing 'spurious patch properties', doc '{ "foo": 1 }' patch '[ { "op": "test", "path": "\/foo", "value": 1, "spurious": 1 } ]' : OK @@ -83,7 +94,9 @@ Testing 'test should pass despite (nested) rearrangement', doc '{ "foo": [ { "fo Testing 'test should pass - no error', doc '{ "foo": { "bar": [ 1, 2, 5, 4 ] } }' patch '[ { "op": "test", "path": "\/foo", "value": { "bar": [ 1, 2, 5, 4 ] } } ]' : OK Testing 'test op should fail', doc '{ "foo": { "bar": [ 1, 2, 5, 4 ] } }' patch '[ { "op": "test", "path": "\/foo", "value": [ 1, 2 ] } ]' : OK => json_patch_apply failed as expected: ERRNO=ENOENT at patch idx 0: Value of element referenced by 'path' field did not match 'value' field -Testing 'Whole document', doc '{ "foo": 1 }' patch '[ { "op": "test", "path": "", "value": { "foo": 1 } } ]' : SKIPPING - no expected or error conditions in test +Testing 'Test the whole document', doc '{ "foo": 1 }' patch '[ { "op": "test", "path": "", "value": { "foo": 1 } } ]' : OK +Testing 'Test the whole document, no match', doc '{ "foo": 1 }' patch '[ { "op": "test", "path": "", "value": { "foo": 2 } } ]' : OK + => json_patch_apply failed as expected: ERRNO=ENOENT at patch idx 0: Value of element referenced by 'path' field did not match 'value' field 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 'Move to same location has no effect', doc '{ "foo": 1 }' patch '[ { "op": "move", "from": "\/foo", "path": "\/foo" } ]' : OK @@ -129,7 +142,7 @@ Testing 'missing from parameter to move', doc '{ "foo": 1 }' patch '[ { "op": "m => json_patch_apply failed as expected: ERRNO=EINVAL at patch idx 0: Patch does not contain a 'from' field Testing 'missing from location to move', doc '{ "foo": 1 }' patch '[ { "op": "move", "from": "\/bar", "path": "\/foo" } ]' : OK => json_patch_apply failed as expected: ERRNO=ENOENT at patch idx 0: Did not find element referenced by from field -Testing 'duplicate ops', doc '{ "foo": "bar" }' patch '[ { "op": "move", "path": "\/baz", "value": "qux", "from": "\/foo" } ]' : SKIPPING - disabled in the test spec +Testing 'duplicate ops, json-c parses this as op:move', doc '{ "foo": "bar" }' patch '[ { "op": "move", "path": "\/baz", "value": "qux", "from": "\/foo" } ]' : OK Testing 'unrecognized op should fail', doc '{ "foo": 1 }' patch '[ { "op": "spam", "path": "\/foo", "value": 1 } ]' : OK => json_patch_apply failed as expected: ERRNO=EINVAL at patch idx 0: Patch object has invalid 'op' field Testing 'test with bad array number that has leading zeros', doc '[ "foo", "bar" ]' patch '[ { "op": "test", "path": "\/00", "value": "foo" } ]' : OK