diff --git a/patch.go b/patch.go index 9513668..f65f8d2 100644 --- a/patch.go +++ b/patch.go @@ -570,6 +570,10 @@ func (p Patch) replace(doc *container, op Operation) error { if path == "" { val := op.value() + if val == nil { + return fmt.Errorf("replace operation missing value field: %w", ErrMissing) + } + if val.which == eRaw { if !val.tryDoc() { if !val.tryAry() { @@ -668,6 +672,12 @@ func (p Patch) test(doc *container, op Operation) error { self.which = eAry } + // A missing value is treated as null, and the whole document is + // never null. + if op.value() == nil { + return fmt.Errorf("testing value %s failed: %w", path, ErrTestFailed) + } + if self.equal(op.value()) { return nil } diff --git a/patch_test.go b/patch_test.go index 98c14ea..a269a77 100644 --- a/patch_test.go +++ b/patch_test.go @@ -369,6 +369,15 @@ var BadCases = []BadCase{ `{ "foo": [ "all", "grass", "cows", "eat" ] }`, `[ { "op": "move", "from": "/foo/1", "path": "/foo/4" } ]`, }, + // Missing value when targeting the whole document + { + `{ "foo": "bar" }`, + `[ { "op": "replace", "path": "" } ]`, + }, + { + `{ "foo": "bar" }`, + `[ { "op": "add", "path": "" } ]`, + }, } // This is not thread safe, so we cannot run patch tests in parallel. @@ -502,6 +511,18 @@ var TestCases = []TestCase{ true, "/foo", }, + { + `{ "baz": [] }`, + `[ { "op": "test", "path": ""} ]`, + false, + "", + }, + { + `[ "baz" ]`, + `[ { "op": "test", "path": ""} ]`, + false, + "", + }, } func TestAllTest(t *testing.T) { diff --git a/v5/patch.go b/v5/patch.go index 83102e5..bb841f3 100644 --- a/v5/patch.go +++ b/v5/patch.go @@ -775,6 +775,10 @@ func (p Patch) add(doc *container, op Operation, options *ApplyOptions) error { if path == "" { val := op.value() + if val == nil { + return fmt.Errorf("add operation missing value field: %w", ErrMissing) + } + var pd container if (*val.raw)[0] == '[' { pd = &partialArray{ @@ -983,6 +987,10 @@ func (p Patch) replace(doc *container, op Operation, options *ApplyOptions) erro if path == "" { val := op.value() + if val == nil { + return fmt.Errorf("replace operation missing value field: %w", ErrMissing) + } + if val.which == eRaw { if !val.tryDoc() { if !val.tryAry() { @@ -1087,6 +1095,12 @@ func (p Patch) test(doc *container, op Operation, options *ApplyOptions) error { self.which = eAry } + // A missing value is treated as null, and the whole document is + // never null. + if op.value() == nil { + return fmt.Errorf("testing value %s failed: %w", path, ErrTestFailed) + } + if self.equal(op.value()) { return nil } diff --git a/v5/patch_test.go b/v5/patch_test.go index 1f807f5..768f63e 100644 --- a/v5/patch_test.go +++ b/v5/patch_test.go @@ -3,6 +3,7 @@ package jsonpatch import ( "bytes" "encoding/json" + "errors" "fmt" "reflect" "testing" @@ -772,6 +773,17 @@ var BadCases = []BadCase{ `[{"op": "move", "path": "/qux", "from": ""}]`, false, }, + // Missing value when targeting the whole document + { + `{ "foo": "bar" }`, + `[ { "op": "replace", "path": "" } ]`, + true, + }, + { + `{ "foo": "bar" }`, + `[ { "op": "add", "path": "" } ]`, + true, + }, } // This is not thread safe, so we cannot run patch tests in parallel. @@ -949,6 +961,18 @@ var TestCases = []TestCase{ true, "/baz", }, + { + `{ "foo": "bar" }`, + `[ { "op": "test", "path": "" } ]`, + false, + "", + }, + { + `[ "foo" ]`, + `[ { "op": "test", "path": "" } ]`, + false, + "", + }, } func TestAllTest(t *testing.T) { @@ -1245,3 +1269,28 @@ func init() { "foo": &msg, } } + +func TestMissingValueOnWholeDocument(t *testing.T) { + // DecodePatch rejects add and replace operations without a value, so + // build the patch directly to exercise the checks in Apply. + cases := []struct { + patch string + err error + }{ + {`[ { "op": "add", "path": "" } ]`, ErrMissing}, + {`[ { "op": "replace", "path": "" } ]`, ErrMissing}, + {`[ { "op": "test", "path": "" } ]`, ErrTestFailed}, + } + + for _, c := range cases { + var p Patch + if err := json.Unmarshal([]byte(c.patch), &p); err != nil { + t.Fatalf("Unable to unmarshal patch %q: %s", c.patch, err) + } + + _, err := p.Apply([]byte(`{ "foo": "bar" }`)) + if !errors.Is(err, c.err) { + t.Errorf("Patch %q: expected error %v, got %v", c.patch, c.err, err) + } + } +}