From 4a9dce77425227f84ead852dfbb9cef51a7702da Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 11 Jan 2024 17:11:52 -0800 Subject: [PATCH 1/3] Handle null correctly when introduced by replace. Fixes #171 --- v5/patch.go | 35 ++++++++++++++++++++++++++++++++--- v5/patch_test.go | 5 +++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/v5/patch.go b/v5/patch.go index 73ff2c5..a5cd4df 100644 --- a/v5/patch.go +++ b/v5/patch.go @@ -254,6 +254,10 @@ func (n *lazyNode) intoDoc() (*partialDoc, error) { err := json.Unmarshal(*n.raw, &n.doc) + if n.doc == nil { + return nil, ErrInvalid + } + if err != nil { return nil, err } @@ -308,6 +312,10 @@ func (n *lazyNode) tryDoc() bool { return false } + if n.doc == nil { + return false + } + n.which = eDoc return true } @@ -327,6 +335,18 @@ func (n *lazyNode) tryAry() bool { return true } +func (n *lazyNode) isNull() bool { + if n == nil { + return true + } + + if n.raw == nil { + return true + } + + return bytes.Equal(n.compact(), rawJSONNull) +} + func (n *lazyNode) equal(o *lazyNode) bool { if n.which == eRaw { if !n.tryDoc() && !n.tryAry() { @@ -446,6 +466,10 @@ func (o Operation) From() (string, error) { func (o Operation) value() *lazyNode { if obj, ok := o["value"]; ok { + // A `null` gets decoded as a nil RawMessage, so let's fix it up here. + if obj == nil { + return newLazyNode(newRawMessage(rawJSONNull)) + } return newLazyNode(obj) } @@ -798,7 +822,10 @@ func ensurePathExists(pd *container, path string, options *ApplyOptions) error { newNode := newLazyNode(newRawMessage(rawJSONObject)) doc.add(part, newNode, options) - doc, _ = newNode.intoDoc() + doc, err = newNode.intoDoc() + if err != nil { + return err + } } } else { if isArray(*target.raw) { @@ -1005,12 +1032,14 @@ func (p Patch) test(doc *container, op Operation, options *ApplyOptions) error { return errors.Wrapf(err, "error in test for path: '%s'", path) } + ov := op.value() + if val == nil { - if op.value() == nil || op.value().raw == nil { + if ov.isNull() { return nil } return errors.Wrapf(ErrTestFailed, "testing value %s failed", path) - } else if op.value() == nil { + } else if ov.isNull() { return errors.Wrapf(ErrTestFailed, "testing value %s failed", path) } diff --git a/v5/patch_test.go b/v5/patch_test.go index 58bb7f1..fbcb5ae 100644 --- a/v5/patch_test.go +++ b/v5/patch_test.go @@ -727,6 +727,11 @@ var BadCases = []BadCase{ `[{"op": "copy", "path": "/qux", "from": "/baz"}]`, false, }, + { + `{ "foo": {"bar": []}}`, + `[{"op": "replace", "path": "/foo/bar", "value": null}, {"op": "add", "path": "/foo/bar/0", "value": "blah"}]`, + false, + }, } // This is not thread safe, so we cannot run patch tests in parallel. From 742691bceb3947164a888d5a71eef6678ceab845 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 11 Jan 2024 17:16:27 -0800 Subject: [PATCH 2/3] Additional null tests --- v5/patch_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/v5/patch_test.go b/v5/patch_test.go index fbcb5ae..20d90cd 100644 --- a/v5/patch_test.go +++ b/v5/patch_test.go @@ -578,6 +578,13 @@ var Cases = []Case{ false, false, }, + { + `{}`, + `[{"op": "replace", "path": "", "value": null}]`, + `null`, + false, + false, + }, } type BadCase struct { @@ -732,6 +739,11 @@ var BadCases = []BadCase{ `[{"op": "replace", "path": "/foo/bar", "value": null}, {"op": "add", "path": "/foo/bar/0", "value": "blah"}]`, false, }, + { + `{}`, + `[{"op": "replace", "path": ""}]`, + true, + }, } // This is not thread safe, so we cannot run patch tests in parallel. From a82b43d7d0443cd7a7ecf00860352f301a4526c0 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 11 Jan 2024 17:31:01 -0800 Subject: [PATCH 3/3] Handle null and other literals correctly in merge. Fixes #160 --- v5/merge.go | 8 ++++++++ v5/merge_test.go | 34 +++------------------------------- 2 files changed, 11 insertions(+), 31 deletions(-) diff --git a/v5/merge.go b/v5/merge.go index a7c4573..8394e90 100644 --- a/v5/merge.go +++ b/v5/merge.go @@ -130,6 +130,10 @@ func doMergePatch(docData, patchData []byte, mergeMerge bool) ([]byte, error) { } if isSyntaxError(patchErr) { + if json.Valid(patchData) { + return patchData, nil + } + return nil, errBadJSONPatch } @@ -154,6 +158,10 @@ func doMergePatch(docData, patchData []byte, mergeMerge bool) ([]byte, error) { patchErr = json.Unmarshal(patchData, patchAry) if patchErr != nil { + // Not an array either, a literal is the result directly. + if json.Valid(patchData) { + return patchData, nil + } return nil, errBadJSONPatch } diff --git a/v5/merge_test.go b/v5/merge_test.go index 72e3ea8..67f8d74 100644 --- a/v5/merge_test.go +++ b/v5/merge_test.go @@ -2,7 +2,6 @@ package jsonpatch import ( "fmt" - "strings" "testing" ) @@ -10,7 +9,7 @@ func mergePatch(doc, patch string) string { out, err := MergePatch([]byte(doc), []byte(patch)) if err != nil { - panic(err) + panic(fmt.Sprintf("%s: %s", err, patch)) } return string(out) @@ -166,8 +165,8 @@ var rfcTests = []struct { {target: `{"a":[{"b":"c"}]}`, patch: `{"a":[1]}`, expected: `{"a":[1]}`}, {target: `["a","b"]`, patch: `["c","d"]`, expected: `["c","d"]`}, {target: `{"a":"b"}`, patch: `["c"]`, expected: `["c"]`}, - // {target: `{"a":"foo"}`, patch: `null`, expected: `null`}, - // {target: `{"a":"foo"}`, patch: `"bar"`, expected: `"bar"`}, + {target: `{"a":"foo"}`, patch: `null`, expected: `null`}, + {target: `{"a":"foo"}`, patch: `"bar"`, expected: `"bar"`}, {target: `{"e":null}`, patch: `{"a":1}`, expected: `{"a":1,"e":null}`}, {target: `[1,2]`, patch: `{"a":"b","c":null}`, expected: `{"a":"b"}`}, {target: `{}`, patch: `{"a":{"bb":{"ccc":null}}}`, expected: `{"a":{"bb":{}}}`}, @@ -183,33 +182,6 @@ func TestMergePatchRFCCases(t *testing.T) { } } -var rfcFailTests = ` - {"a":"foo"} | null - {"a":"foo"} | "bar" -` - -func TestMergePatchFailRFCCases(t *testing.T) { - tests := strings.Split(rfcFailTests, "\n") - - for _, c := range tests { - if strings.TrimSpace(c) == "" { - continue - } - - parts := strings.SplitN(c, "|", 2) - - doc := strings.TrimSpace(parts[0]) - pat := strings.TrimSpace(parts[1]) - - out, err := MergePatch([]byte(doc), []byte(pat)) - - if err != errBadJSONPatch { - t.Errorf("error not returned properly: %s, %s", err, string(out)) - } - } - -} - func TestResembleJSONArray(t *testing.T) { testCases := []struct { input []byte