From b5e20d456f0e423d412d09f4801b0560ec1eb9a3 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 11 Jan 2024 13:50:37 -0800 Subject: [PATCH 1/3] Handle from="" more properly. Fixes #192 --- v5/merge.go | 8 ++-- v5/merge_test.go | 1 + v5/patch.go | 105 +++++++++++++++++++++++++++++++---------------- v5/patch_test.go | 27 +++++++++--- 4 files changed, 95 insertions(+), 46 deletions(-) diff --git a/v5/merge.go b/v5/merge.go index 6c5f541..2aee31d 100644 --- a/v5/merge.go +++ b/v5/merge.go @@ -88,14 +88,14 @@ func pruneDocNulls(doc *partialDoc) *partialDoc { func pruneAryNulls(ary *partialArray) *partialArray { newAry := []*lazyNode{} - for _, v := range *ary { + for _, v := range ary.nodes { if v != nil { pruneNulls(v) } newAry = append(newAry, v) } - *ary = newAry + ary.nodes = newAry return ary } @@ -155,7 +155,7 @@ func doMergePatch(docData, patchData []byte, mergeMerge bool) ([]byte, error) { } } else { patchAry := &partialArray{} - patchErr = unmarshal(patchData, patchAry) + patchErr = unmarshal(patchData, &patchAry.nodes) if patchErr != nil { // Not an array either, a literal is the result directly. @@ -167,7 +167,7 @@ func doMergePatch(docData, patchData []byte, mergeMerge bool) ([]byte, error) { pruneAryNulls(patchAry) - out, patchErr := json.Marshal(patchAry) + out, patchErr := json.Marshal(patchAry.nodes) if patchErr != nil { return nil, errBadJSONPatch diff --git a/v5/merge_test.go b/v5/merge_test.go index 67f8d74..0077a71 100644 --- a/v5/merge_test.go +++ b/v5/merge_test.go @@ -69,6 +69,7 @@ func TestMergePatchNilArray(t *testing.T) { } for _, c := range cases { + t.Log(c.original) act := mergePatch(c.original, c.patch) if !compareJSON(c.res, act) { diff --git a/v5/patch.go b/v5/patch.go index a9e750d..7069f9a 100644 --- a/v5/patch.go +++ b/v5/patch.go @@ -45,7 +45,7 @@ var ( type lazyNode struct { raw *json.RawMessage doc *partialDoc - ary partialArray + ary *partialArray which int } @@ -56,11 +56,15 @@ type Operation map[string]*json.RawMessage type Patch []Operation type partialDoc struct { + self *lazyNode keys []string obj map[string]*lazyNode } -type partialArray []*lazyNode +type partialArray struct { + self *lazyNode + nodes []*lazyNode +} type container interface { get(key string, options *ApplyOptions) (*lazyNode, error) @@ -114,7 +118,7 @@ func (n *lazyNode) MarshalJSON() ([]byte, error) { case eDoc: return json.Marshal(n.doc) case eAry: - return json.Marshal(n.ary) + return json.Marshal(n.ary.nodes) default: return nil, ErrUnknownType } @@ -199,6 +203,14 @@ func (n *partialDoc) UnmarshalJSON(data []byte) error { return nil } +func (n *partialArray) UnmarshalJSON(data []byte) error { + return json.Unmarshal(data, &n.nodes) +} + +func (n *partialArray) MarshalJSON() ([]byte, error) { + return json.Marshal(n.nodes) +} + func skipValue(d *json.Decoder) error { t, err := d.Token() if err != nil { @@ -268,7 +280,7 @@ func (n *lazyNode) intoDoc() (*partialDoc, error) { func (n *lazyNode) intoAry() (*partialArray, error) { if n.which == eAry { - return &n.ary, nil + return n.ary, nil } if n.raw == nil { @@ -282,7 +294,7 @@ func (n *lazyNode) intoAry() (*partialArray, error) { } n.which = eAry - return &n.ary, nil + return n.ary, nil } func (n *lazyNode) compact() []byte { @@ -420,12 +432,12 @@ func (n *lazyNode) equal(o *lazyNode) bool { return false } - if len(n.ary) != len(o.ary) { + if len(n.ary.nodes) != len(o.ary.nodes) { return false } - for idx, val := range n.ary { - if !val.equal(o.ary[idx]) { + for idx, val := range n.ary.nodes { + if !val.equal(o.ary.nodes[idx]) { return false } } @@ -541,6 +553,9 @@ func findObject(pd *container, path string, options *ApplyOptions) (container, s split := strings.Split(path, "/") if len(split) < 2 { + if path == "" { + return doc, "" + } return nil, "" } @@ -596,6 +611,9 @@ func (d *partialDoc) add(key string, val *lazyNode, options *ApplyOptions) error } func (d *partialDoc) get(key string, options *ApplyOptions) (*lazyNode, error) { + if key == "" { + return d.self, nil + } v, ok := d.obj[key] if !ok { return v, errors.Wrapf(ErrMissing, "unable to get nonexistent key: %s", key) @@ -635,19 +653,19 @@ func (d *partialArray) set(key string, val *lazyNode, options *ApplyOptions) err if !options.SupportNegativeIndices { return errors.Wrapf(ErrInvalidIndex, "Unable to access invalid index: %d", idx) } - if idx < -len(*d) { + if idx < -len(d.nodes) { return errors.Wrapf(ErrInvalidIndex, "Unable to access invalid index: %d", idx) } - idx += len(*d) + idx += len(d.nodes) } - (*d)[idx] = val + d.nodes[idx] = val return nil } func (d *partialArray) add(key string, val *lazyNode, options *ApplyOptions) error { if key == "-" { - *d = append(*d, val) + d.nodes = append(d.nodes, val) return nil } @@ -656,11 +674,11 @@ func (d *partialArray) add(key string, val *lazyNode, options *ApplyOptions) err return errors.Wrapf(err, "value was not a proper array index: '%s'", key) } - sz := len(*d) + 1 + sz := len(d.nodes) + 1 ary := make([]*lazyNode, sz) - cur := *d + cur := d if idx >= len(ary) { return errors.Wrapf(ErrInvalidIndex, "Unable to access invalid index: %d", idx) @@ -676,15 +694,19 @@ func (d *partialArray) add(key string, val *lazyNode, options *ApplyOptions) err idx += len(ary) } - copy(ary[0:idx], cur[0:idx]) + copy(ary[0:idx], cur.nodes[0:idx]) ary[idx] = val - copy(ary[idx+1:], cur[idx:]) + copy(ary[idx+1:], cur.nodes[idx:]) - *d = ary + d.nodes = ary return nil } func (d *partialArray) get(key string, options *ApplyOptions) (*lazyNode, error) { + if key == "" { + return d.self, nil + } + idx, err := strconv.Atoi(key) if err != nil { @@ -695,17 +717,17 @@ func (d *partialArray) get(key string, options *ApplyOptions) (*lazyNode, error) if !options.SupportNegativeIndices { return nil, errors.Wrapf(ErrInvalidIndex, "Unable to access invalid index: %d", idx) } - if idx < -len(*d) { + if idx < -len(d.nodes) { return nil, errors.Wrapf(ErrInvalidIndex, "Unable to access invalid index: %d", idx) } - idx += len(*d) + idx += len(d.nodes) } - if idx >= len(*d) { + if idx >= len(d.nodes) { return nil, errors.Wrapf(ErrInvalidIndex, "Unable to access invalid index: %d", idx) } - return (*d)[idx], nil + return d.nodes[idx], nil } func (d *partialArray) remove(key string, options *ApplyOptions) error { @@ -714,9 +736,9 @@ func (d *partialArray) remove(key string, options *ApplyOptions) error { return err } - cur := *d + cur := d - if idx >= len(cur) { + if idx >= len(cur.nodes) { if options.AllowMissingPathOnRemove { return nil } @@ -727,21 +749,21 @@ func (d *partialArray) remove(key string, options *ApplyOptions) error { if !options.SupportNegativeIndices { return errors.Wrapf(ErrInvalidIndex, "Unable to access invalid index: %d", idx) } - if idx < -len(cur) { + if idx < -len(cur.nodes) { if options.AllowMissingPathOnRemove { return nil } return errors.Wrapf(ErrInvalidIndex, "Unable to access invalid index: %d", idx) } - idx += len(cur) + idx += len(cur.nodes) } - ary := make([]*lazyNode, len(cur)-1) + ary := make([]*lazyNode, len(cur.nodes)-1) - copy(ary[0:idx], cur[0:idx]) - copy(ary[idx:], cur[idx+1:]) + copy(ary[0:idx], cur.nodes[0:idx]) + copy(ary[idx:], cur.nodes[idx+1:]) - *d = ary + d.nodes = ary return nil } @@ -806,9 +828,9 @@ func ensurePathExists(pd *container, path string, options *ApplyOptions) error { if arrIndex, err = strconv.Atoi(part); err == nil { pa, ok := doc.(*partialArray) - if ok && arrIndex >= len(*pa)+1 { + if ok && arrIndex >= len(pa.nodes)+1 { // Pad the array with null values up to the required index. - for i := len(*pa); i <= arrIndex-1; i++ { + for i := len(pa.nodes); i <= arrIndex-1; i++ { doc.add(strconv.Itoa(i), newLazyNode(newRawMessage(rawJSONNull)), options) } } @@ -946,7 +968,7 @@ func (p Patch) replace(doc *container, op Operation, options *ApplyOptions) erro switch val.which { case eAry: - *doc = &val.ary + *doc = val.ary case eDoc: *doc = val.doc case eRaw: @@ -981,6 +1003,10 @@ func (p Patch) move(doc *container, op Operation, options *ApplyOptions) error { return errors.Wrapf(err, "move operation failed to decode from") } + if from == "" { + return errors.Wrapf(ErrInvalid, "unable to move entire document to another path") + } + con, key := findObject(doc, from, options) if con == nil { @@ -1030,7 +1056,7 @@ func (p Patch) test(doc *container, op Operation, options *ApplyOptions) error { self.doc = sv self.which = eDoc case *partialArray: - self.ary = *sv + self.ary = sv self.which = eAry } @@ -1079,7 +1105,7 @@ func (p Patch) copy(doc *container, op Operation, accumulatedCopySize *int64, op con, key := findObject(doc, from, options) if con == nil { - return errors.Wrapf(ErrMissing, "copy operation does not apply: doc is missing from path: %s", from) + return errors.Wrapf(ErrMissing, "copy operation does not apply: doc is missing from path: \"%s\"", from) } val, err := con.get(key, options) @@ -1166,11 +1192,18 @@ func (p Patch) ApplyIndentWithOptions(doc []byte, indent string, options *ApplyO return doc, nil } + raw := json.RawMessage(doc) + self := newLazyNode(&raw) + var pd container if doc[0] == '[' { - pd = &partialArray{} + pd = &partialArray{ + self: self, + } } else { - pd = &partialDoc{} + pd = &partialDoc{ + self: self, + } } err := unmarshal(doc, pd) diff --git a/v5/patch_test.go b/v5/patch_test.go index 18b4511..1fb3068 100644 --- a/v5/patch_test.go +++ b/v5/patch_test.go @@ -585,6 +585,20 @@ var Cases = []Case{ false, false, }, + { + `{"foo": 1}`, + `[ { "op": "copy", "from": "", "path": "/bar"}]`, + `{"foo": 1, "bar": {"foo": 1}}`, + false, + false, + }, + { + `[{"foo": 1}]`, + `[ { "op": "copy", "from": "", "path": "/1"}]`, + `[{"foo": 1}, [{"foo": 1}]]`, + false, + false, + }, } type BadCase struct { @@ -642,11 +656,6 @@ var BadCases = []BadCase{ `[ { "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"}]`, @@ -744,6 +753,11 @@ var BadCases = []BadCase{ `[{"op": "replace", "path": ""}]`, true, }, + { + `{ "foo": "bar"}`, + `[{"op": "move", "path": "/qux", "from": ""}]`, + false, + }, } // This is not thread safe, so we cannot run patch tests in parallel. @@ -819,9 +833,10 @@ func TestAllCases(t *testing.T) { } if err == nil && !c.failOnDecode { - _, err = p.Apply([]byte(c.doc)) + out, err := p.Apply([]byte(c.doc)) if err == nil { + t.Log(string(out)) t.Errorf("Patch %q should have failed to apply but it did not", c.patch) } From c645ff441b0958be0c723b43e1eb95f2746d3a0e Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 11 Jan 2024 14:22:19 -0800 Subject: [PATCH 2/3] Handle `add` with path="". Fixes #188 --- v5/patch.go | 26 ++++++++++++++++++++++++++ v5/patch_test.go | 14 ++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/v5/patch.go b/v5/patch.go index 7069f9a..2cd66a1 100644 --- a/v5/patch.go +++ b/v5/patch.go @@ -773,6 +773,32 @@ func (p Patch) add(doc *container, op Operation, options *ApplyOptions) error { return errors.Wrapf(ErrMissing, "add operation failed to decode path") } + // special case, adding to empty means replacing the container with the value given + if path == "" { + val := op.value() + + var pd container + if (*val.raw)[0] == '[' { + pd = &partialArray{ + self: val, + } + } else { + pd = &partialDoc{ + self: val, + } + } + + err := json.Unmarshal(*val.raw, pd) + + if err != nil { + return err + } + + *doc = pd + + return nil + } + if options.EnsurePathExistsOnAdd { err = ensurePathExists(doc, path, options) diff --git a/v5/patch_test.go b/v5/patch_test.go index 1fb3068..b9d5e46 100644 --- a/v5/patch_test.go +++ b/v5/patch_test.go @@ -599,6 +599,20 @@ var Cases = []Case{ false, false, }, + { + `{}`, + `[{"op":"add","path":"","value":{"foo":"bar"}}]`, + `{"foo": "bar"}`, + false, + false, + }, + { + `[]`, + `[{"op":"add","path":"","value":{"foo":"bar"}}, {"op": "add", "path": "/qux", "value": 1}]`, + `{"foo": "bar", "qux": 1}`, + false, + false, + }, } type BadCase struct { From e8ab61f4a0e6205bbed116ee7c172f22554ecb2e Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 11 Jan 2024 17:59:57 -0800 Subject: [PATCH 3/3] Catch IO advancement errors as well --- v5/merge.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/v5/merge.go b/v5/merge.go index 2aee31d..369144e 100644 --- a/v5/merge.go +++ b/v5/merge.go @@ -3,7 +3,9 @@ package jsonpatch import ( "bytes" "encoding/json" + "errors" "fmt" + "io" "reflect" ) @@ -183,6 +185,12 @@ func doMergePatch(docData, patchData []byte, mergeMerge bool) ([]byte, error) { } func isSyntaxError(err error) bool { + if errors.Is(err, io.EOF) { + return true + } + if errors.Is(err, io.ErrUnexpectedEOF) { + return true + } if _, ok := err.(*json.SyntaxError); ok { return true }