diff --git a/v5/patch.go b/v5/patch.go index 97aedcf..a2a6f50 100644 --- a/v5/patch.go +++ b/v5/patch.go @@ -454,7 +454,11 @@ func (o Operation) value() *lazyNode { // ValueInterface decodes the operation value into an interface. func (o Operation) ValueInterface() (interface{}, error) { - if obj, ok := o["value"]; ok && obj != nil { + if obj, ok := o["value"]; ok { + if obj == nil { + return nil, nil + } + var v interface{} err := json.Unmarshal(*obj, &v) @@ -816,6 +820,43 @@ func ensurePathExists(pd *container, path string, options *ApplyOptions) error { return nil } +func validateOperation(op Operation) error { + switch op.Kind() { + case "add", "replace": + if _, err := op.ValueInterface(); err != nil { + return errors.Wrapf(err, "failed to decode 'value'") + } + case "move", "copy": + if _, err := op.From(); err != nil { + return errors.Wrapf(err, "failed to decode 'from'") + } + case "remove", "test": + default: + return fmt.Errorf("unsupported operation") + } + + if _, err := op.Path(); err != nil { + return errors.Wrapf(err, "failed to decode 'path'") + } + + return nil +} + +func validatePatch(p Patch) error { + for _, op := range p { + if err := validateOperation(op); err != nil { + opData, infoErr := json.Marshal(op) + if infoErr != nil { + return errors.Wrapf(err, "invalid operation") + } + + return errors.Wrapf(err, "invalid operation %s", opData) + } + } + + return nil +} + func (p Patch) remove(doc *container, op Operation, options *ApplyOptions) error { path, err := op.Path() if err != nil { @@ -1044,6 +1085,10 @@ func DecodePatch(buf []byte) (Patch, error) { return nil, err } + if err := validatePatch(p); err != nil { + return nil, err + } + return p, nil } diff --git a/v5/patch_test.go b/v5/patch_test.go index 3fa079e..91c6413 100644 --- a/v5/patch_test.go +++ b/v5/patch_test.go @@ -31,7 +31,7 @@ func applyPatch(doc, patch string) (string, error) { obj, err := DecodePatch([]byte(patch)) if err != nil { - panic(err) + return "", err } out, err := obj.Apply([]byte(doc)) @@ -47,7 +47,7 @@ func applyPatchIndented(doc, patch string) (string, error) { obj, err := DecodePatch([]byte(patch)) if err != nil { - panic(err) + return "", err } out, err := obj.ApplyIndent([]byte(doc), " ") @@ -63,7 +63,7 @@ func applyPatchWithOptions(doc, patch string, options *ApplyOptions) (string, er obj, err := DecodePatch([]byte(patch)) if err != nil { - panic(err) + return "", err } out, err := obj.ApplyWithOptions([]byte(doc), options) @@ -574,17 +574,26 @@ var Cases = []Case{ } type BadCase struct { - doc, patch string + doc, patch string + failOnDecode bool } var MutationTestCases = []BadCase{ { `{ "foo": "bar", "qux": { "baz": 1, "bar": null } }`, `[ { "op": "remove", "path": "/qux/bar" } ]`, + false, }, { `{ "foo": "bar", "qux": { "baz": 1, "bar": null } }`, `[ { "op": "replace", "path": "/qux/baz", "value": null } ]`, + true, + }, + // malformed value + { + `{ "foo": "bar" }`, + `[ { "op": "add", "path": "/", "value": "{qux" } ]`, + true, }, } @@ -592,79 +601,98 @@ var BadCases = []BadCase{ { `{ "foo": "bar" }`, `[ { "op": "add", "path": "/baz/bat", "value": "qux" } ]`, + false, }, { `{ "a": { "b": { "d": 1 } } }`, `[ { "op": "remove", "path": "/a/b/c" } ]`, + false, }, { `{ "a": { "b": { "d": 1 } } }`, `[ { "op": "move", "from": "/a/b/c", "path": "/a/b/e" } ]`, + false, }, { `{ "a": { "b": [1] } }`, `[ { "op": "remove", "path": "/a/b/1" } ]`, + false, }, { `{ "a": { "b": [1] } }`, `[ { "op": "move", "from": "/a/b/1", "path": "/a/b/2" } ]`, + false, }, { `{ "foo": "bar" }`, `[ { "op": "add", "pathz": "/baz", "value": "qux" } ]`, + true, }, { `{ "foo": "bar" }`, `[ { "op": "add", "path": "", "value": "qux" } ]`, + false, }, { `{ "foo": ["bar","baz"]}`, `[ { "op": "replace", "path": "/foo/2", "value": "bum"}]`, + false, }, { `{ "foo": ["bar","baz"]}`, `[ { "op": "add", "path": "/foo/-4", "value": "bum"}]`, + false, }, { `{ "name":{ "foo": "bat", "qux": "bum"}}`, `[ { "op": "replace", "path": "/foo/bar", "value":"baz"}]`, + false, }, { `{ "foo": ["bar"]}`, `[ {"op": "add", "path": "/foo/2", "value": "bum"}]`, + false, }, { `{ "foo": []}`, `[ {"op": "remove", "path": "/foo/-"}]`, + false, }, { `{ "foo": []}`, `[ {"op": "remove", "path": "/foo/-1"}]`, + false, }, { `{ "foo": ["bar"]}`, `[ {"op": "remove", "path": "/foo/-2"}]`, + false, }, { `{}`, `[ {"op":null,"path":""} ]`, + true, }, { `{}`, `[ {"op":"add","path":null} ]`, + true, }, { `{}`, `[ { "op": "copy", "from": null }]`, + true, }, { `{ "foo": ["bar"]}`, `[{"op": "copy", "path": "/foo/6666666666", "from": "/"}]`, + false, }, // Can't copy into an index greater than the size of the array { `{ "foo": ["bar"]}`, `[{"op": "copy", "path": "/foo/2", "from": "/foo/0"}]`, + false, }, // Accumulated copy size cannot exceed AccumulatedCopySizeLimit. { @@ -673,20 +701,24 @@ var BadCases = []BadCase{ // size, so each copy operation increases the size by 51 bytes. `[ { "op": "copy", "path": "/foo/-", "from": "/foo/1" }, { "op": "copy", "path": "/foo/-", "from": "/foo/1" }]`, + false, }, // Can't move into an index greater than or equal to the size of the array { `{ "foo": [ "all", "grass", "cows", "eat" ] }`, `[ { "op": "move", "from": "/foo/1", "path": "/foo/4" } ]`, + false, }, { `{ "baz": "qux" }`, `[ { "op": "replace", "path": "/foo", "value": "bar" } ]`, + false, }, // Can't copy from non-existent "from" key. { `{ "foo": "bar"}`, `[{"op": "copy", "path": "/qux", "from": "/baz"}]`, + false, }, } @@ -753,10 +785,22 @@ func TestAllCases(t *testing.T) { } for _, c := range BadCases { - _, err := applyPatch(c.doc, c.patch) + p, err := DecodePatch([]byte(c.patch)) + if err == nil && c.failOnDecode { + t.Errorf("Patch %q should have failed decode but did not", c.patch) + } + + if err != nil && !c.failOnDecode { + t.Errorf("Patch %q should have passed decode but failed with %v", c.patch, err) + } + + if err == nil && !c.failOnDecode { + _, err = p.Apply([]byte(c.doc)) + + if err == nil { + t.Errorf("Patch %q should have failed to apply but it did not", c.patch) + } - if err == nil { - t.Errorf("Patch %q should have failed to apply but it did not", c.patch) } } }