From a5df74aab0efb032ca893c048330abcad0b78e6f Mon Sep 17 00:00:00 2001 From: Brett Buddin Date: Wed, 7 Aug 2019 17:12:57 -0400 Subject: [PATCH 1/4] Conform to RFC6902 replacement semantics. A "replace" patch operation referencing a key that does not exist in the target object are currently being accepted; ultimately becoming "add" operations on the target object. This is incorrect per the specification: From https://tools.ietf.org/html/rfc6902#section-4.3 section about "replace": The target location MUST exist for the operation to be successful. This corrects the behavior by returning an error in this situation. --- patch.go | 10 +++++++--- patch_test.go | 4 ++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/patch.go b/patch.go index 8f75549..c105632 100644 --- a/patch.go +++ b/patch.go @@ -384,13 +384,17 @@ func (d *partialDoc) add(key string, val *lazyNode) error { } func (d *partialDoc) get(key string) (*lazyNode, error) { - return (*d)[key], nil + v, ok := (*d)[key] + if !ok { + return v, errors.Wrapf(ErrMissing, "unable to get nonexistent key: %s", key) + } + return v, nil } func (d *partialDoc) remove(key string) error { _, ok := (*d)[key] if !ok { - return errors.Wrapf(ErrMissing, "Unable to remove nonexistent key: %s", key) + return errors.Wrapf(ErrMissing, "unable to remove nonexistent key: %s", key) } delete(*d, key) @@ -612,7 +616,7 @@ func (p Patch) test(doc *container, op Operation) error { } val, err := con.get(key) - if err != nil { + if err != nil && errors.Cause(err) != ErrMissing { return errors.Wrapf(err, "error in test for path: '%s'", path) } diff --git a/patch_test.go b/patch_test.go index 3a45150..30f6176 100644 --- a/patch_test.go +++ b/patch_test.go @@ -332,6 +332,10 @@ var BadCases = []BadCase{ `{ "foo": [ "all", "grass", "cows", "eat" ] }`, `[ { "op": "move", "from": "/foo/1", "path": "/foo/4" } ]`, }, + { + `{ "baz": "qux" }`, + `[ { "op": "replace", "path": "/foo", "value": "bar" } ]`, + }, } // This is not thread safe, so we cannot run patch tests in parallel. From caa7f266ac971d84049d2a618644979d26114ac8 Mon Sep 17 00:00:00 2001 From: Brett Buddin Date: Mon, 4 Nov 2019 13:16:59 -0500 Subject: [PATCH 2/4] Test for copying non-existent key. --- patch_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/patch_test.go b/patch_test.go index 30f6176..66e5bd3 100644 --- a/patch_test.go +++ b/patch_test.go @@ -336,6 +336,11 @@ var BadCases = []BadCase{ `{ "baz": "qux" }`, `[ { "op": "replace", "path": "/foo", "value": "bar" } ]`, }, + // Can't copy from non-existent "from" key. + { + `{ "foo": "bar"}`, + `[{"op": "copy", "path": "/qux", "from": "/baz"}]`, + }, } // This is not thread safe, so we cannot run patch tests in parallel. From c235ce89e1e02a76eac0be921e6e8fe12d9c6120 Mon Sep 17 00:00:00 2001 From: Brett Buddin Date: Mon, 4 Nov 2019 13:17:14 -0500 Subject: [PATCH 3/4] Test for testing non-existent and null-value keys. --- patch_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/patch_test.go b/patch_test.go index 66e5bd3..69851bc 100644 --- a/patch_test.go +++ b/patch_test.go @@ -468,6 +468,18 @@ var TestCases = []TestCase{ false, "/foo", }, + { + `{ "foo": "bar" }`, + `[ { "op": "test", "path": "/baz", "value": "bar" } ]`, + false, + "/baz", + }, + { + `{ "foo": "bar" }`, + `[ { "op": "test", "path": "/baz", "value": null } ]`, + true, + "/baz", + }, } func TestAllTest(t *testing.T) { From 266303115221f6a79e03731280f3676b22168d2c Mon Sep 17 00:00:00 2001 From: Brett Buddin Date: Mon, 4 Nov 2019 13:20:41 -0500 Subject: [PATCH 4/4] Test for copying null-value key. --- patch_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/patch_test.go b/patch_test.go index 69851bc..8e20cf3 100644 --- a/patch_test.go +++ b/patch_test.go @@ -186,6 +186,11 @@ var Cases = []Case{ `[{"op": "copy", "path": "/foo/0", "from": "/foo"}]`, `{ "foo": [["bar"], "bar"]}`, }, + { + `{ "foo": null}`, + `[{"op": "copy", "path": "/bar", "from": "/foo"}]`, + `{ "foo": null, "bar": null}`, + }, { `{ "foo": ["bar","qux","baz"]}`, `[ { "op": "remove", "path": "/foo/-2"}]`,