From a5df74aab0efb032ca893c048330abcad0b78e6f Mon Sep 17 00:00:00 2001 From: Brett Buddin Date: Wed, 7 Aug 2019 17:12:57 -0400 Subject: [PATCH] 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.