Skip to content

Commit

Permalink
Conform to RFC6902 replacement semantics.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
brettbuddin committed Nov 4, 2019
1 parent fab786d commit a5df74a
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
10 changes: 7 additions & 3 deletions patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}

Expand Down
4 changes: 4 additions & 0 deletions patch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit a5df74a

Please sign in to comment.