From fc9e957c6aeb042a3a91626e45b72f2a4b40b01c Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 16 May 2023 07:08:41 -0700 Subject: [PATCH 01/11] Update to Go 1.20 Signed-off-by: John Howard --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 54024b9..2ca2f64 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v2 with: - go-version: 1.17 + go-version: 1.20 - name: Build run: | From a81e531fabd348de690367e6393fc0412351c286 Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 16 May 2023 07:09:04 -0700 Subject: [PATCH 02/11] Go.mod for go 1.20 Signed-off-by: John Howard --- v2/go.mod | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/v2/go.mod b/v2/go.mod index 0395a58..df00bf9 100644 --- a/v2/go.mod +++ b/v2/go.mod @@ -1,8 +1,14 @@ module gomodules.xyz/jsonpatch/v2 -go 1.12 +go 1.20 require ( github.com/evanphx/json-patch v0.5.2 github.com/stretchr/testify v1.3.0 ) + +require ( + github.com/davecgh/go-spew v1.1.0 // indirect + github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect +) From 87f8876ad2093b70ea5bd77911c05b566ef26065 Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 16 May 2023 07:20:21 -0700 Subject: [PATCH 03/11] add benchmarks Signed-off-by: John Howard --- v2/bench_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 v2/bench_test.go diff --git a/v2/bench_test.go b/v2/bench_test.go new file mode 100644 index 0000000..e197c21 --- /dev/null +++ b/v2/bench_test.go @@ -0,0 +1,56 @@ +package jsonpatch_test + +import ( + "encoding/json" + "testing" + + "gomodules.xyz/jsonpatch/v2" +) + +func BenchmarkCreatePatch(b *testing.B) { + cases := []struct { + name string + a, b string + }{ + { + "complex", + superComplexBase, + superComplexA, + }, + { + "large array", + largeArray(1000), + largeArray(1000), + }, + { + "simple", + simpleA, + simpleB, + }, + } + + for _, tt := range cases { + b.Run(tt.name, func(b *testing.B) { + at := []byte(tt.a) + bt := []byte(tt.b) + for n := 0; n < b.N; n++ { + _, _ = jsonpatch.CreatePatch(at, bt) + } + }) + } +} + +func largeArray(size int) string { + type nested struct { + A, B string + } + type example struct { + Objects []nested + } + a := example{} + for i := 0; i < size; i++ { + a.Objects = append(a.Objects, nested{A: "a", B: "b"}) + } + res, _ := json.Marshal(a) + return string(res) +} From 4844bbff723c8a71e9ae31df3b9ddc95715ff173 Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 16 May 2023 07:23:31 -0700 Subject: [PATCH 04/11] Add fuzzing test Signed-off-by: John Howard --- v2/fuzz_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 v2/fuzz_test.go diff --git a/v2/fuzz_test.go b/v2/fuzz_test.go new file mode 100644 index 0000000..51eec16 --- /dev/null +++ b/v2/fuzz_test.go @@ -0,0 +1,22 @@ +package jsonpatch_test + +import ( + "testing" + + "gomodules.xyz/jsonpatch/v2" +) + +func FuzzCreatePatch(f *testing.F) { + add := func(a, b string) { + f.Add([]byte(a), []byte(b)) + } + add(simpleA, simpleB) + add(superComplexBase, superComplexA) + add(hyperComplexBase, hyperComplexA) + add(arraySrc, arrayDst) + add(empty, simpleA) + add(point, lineString) + f.Fuzz(func(t *testing.T, a, b []byte) { + _, _ = jsonpatch.CreatePatch(a, b) + }) +} From 471fa305467fa517bc13596be900890d117683a9 Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 16 May 2023 07:25:09 -0700 Subject: [PATCH 05/11] Fix CI Signed-off-by: John Howard --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2ca2f64..e0b10d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v2 with: - go-version: 1.20 + go-version: '1.20' - name: Build run: | From 1f7eb13568cb534c84da3bb3cb826e48cff7ecda Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 16 May 2023 07:58:18 -0700 Subject: [PATCH 06/11] Improve fuzz tests Signed-off-by: John Howard --- v2/fuzz_test.go | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/v2/fuzz_test.go b/v2/fuzz_test.go index 51eec16..a8dc311 100644 --- a/v2/fuzz_test.go +++ b/v2/fuzz_test.go @@ -1,8 +1,11 @@ package jsonpatch_test import ( + "encoding/json" "testing" + jp "github.com/evanphx/json-patch" + "github.com/stretchr/testify/assert" "gomodules.xyz/jsonpatch/v2" ) @@ -17,6 +20,35 @@ func FuzzCreatePatch(f *testing.F) { add(empty, simpleA) add(point, lineString) f.Fuzz(func(t *testing.T, a, b []byte) { - _, _ = jsonpatch.CreatePatch(a, b) + checkFuzz(t, a, b) }) } + +func checkFuzz(t *testing.T, src, dst []byte) { + patch, err := jsonpatch.CreatePatch(src, dst) + if err != nil { + // Ok to error, src or dst may be invalid + t.Skip() + } + + // Applying library only works with arrays and structs, no primitives + // We still do CreatePatch to make sure it doesn't panic + if isPrimitive(src) || isPrimitive(dst) { + return + } + + data, err := json.Marshal(patch) + assert.Nil(t, err) + + p2, err := jp.DecodePatch(data) + assert.Nil(t, err) + + d2, err := p2.Apply(src) + assert.Nil(t, err) + + assert.JSONEq(t, string(dst), string(d2)) +} + +func isPrimitive(data []byte) bool { + return data[0] != '{' && data[0] != '[' +} From 2c05a51447fdd46145aaa02193d3c1a3eb309ec6 Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 16 May 2023 07:59:55 -0700 Subject: [PATCH 07/11] Fix inputs with empty keys Signed-off-by: John Howard --- v2/jsonpatch.go | 3 --- v2/jsonpatch_test.go | 7 +++++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/v2/jsonpatch.go b/v2/jsonpatch.go index 0ffb315..bef26c3 100644 --- a/v2/jsonpatch.go +++ b/v2/jsonpatch.go @@ -149,9 +149,6 @@ func makePath(path string, newPart interface{}) string { if path == "" { return "/" + key } - if strings.HasSuffix(path, "/") { - return path + key - } return path + "/" + key } diff --git a/v2/jsonpatch_test.go b/v2/jsonpatch_test.go index a8ed605..1bc084e 100644 --- a/v2/jsonpatch_test.go +++ b/v2/jsonpatch_test.go @@ -836,6 +836,11 @@ var ( }` ) +var ( + emptyKeyA = `{"":[0]}` + emptyKeyB = `{"":[]}` +) + func TestCreatePatch(t *testing.T) { cases := []struct { name string @@ -881,6 +886,8 @@ func TestCreatePatch(t *testing.T) { {"Array at root", `[{"asdf":"qwerty"}]`, `[{"asdf":"bla"},{"asdf":"zzz"}]`}, {"Empty array at root", `[]`, `[{"asdf":"bla"},{"asdf":"zzz"}]`}, {"Null Key uses replace operation", nullKeyA, nullKeyB}, + // empty key + {"Empty key", emptyKeyA, emptyKeyB}, } for _, c := range cases { From c616a48b06b741ee80e5320a0dd3424576a3c621 Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 16 May 2023 08:06:08 -0700 Subject: [PATCH 08/11] Fix marshaling invalid chars Signed-off-by: John Howard --- v2/jsonpatch.go | 36 +++++++++++++++++++++--------------- v2/jsonpatch_test.go | 6 ++++++ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/v2/jsonpatch.go b/v2/jsonpatch.go index bef26c3..1d61a1f 100644 --- a/v2/jsonpatch.go +++ b/v2/jsonpatch.go @@ -1,7 +1,6 @@ package jsonpatch import ( - "bytes" "encoding/json" "fmt" "reflect" @@ -24,21 +23,28 @@ func (j *Operation) Json() string { } func (j *Operation) MarshalJSON() ([]byte, error) { - var b bytes.Buffer - b.WriteString("{") - b.WriteString(fmt.Sprintf(`"op":"%s"`, j.Operation)) - b.WriteString(fmt.Sprintf(`,"path":"%s"`, j.Path)) - // Consider omitting Value for non-nullable operations. - if j.Value != nil || j.Operation == "replace" || j.Operation == "add" { - v, err := json.Marshal(j.Value) - if err != nil { - return nil, err - } - b.WriteString(`,"value":`) - b.Write(v) + // Ensure for add and replace we emit `value: null` + if j.Value == nil && (j.Operation == "replace" || j.Operation == "add") { + return json.Marshal(struct { + Operation string `json:"op"` + Path string `json:"path"` + Value interface{} `json:"value"` + }{ + Operation: j.Operation, + Path: j.Path, + }) } - b.WriteString("}") - return b.Bytes(), nil + // otherwise just marshal normally. We cannot literally do json.Marshal(j) as it would be recursively + // calling this function. + return json.Marshal(struct { + Operation string `json:"op"` + Path string `json:"path"` + Value interface{} `json:"value,omitempty"` + }{ + Operation: j.Operation, + Path: j.Path, + Value: j.Value, + }) } type ByPath []Operation diff --git a/v2/jsonpatch_test.go b/v2/jsonpatch_test.go index 1bc084e..c311159 100644 --- a/v2/jsonpatch_test.go +++ b/v2/jsonpatch_test.go @@ -841,6 +841,10 @@ var ( emptyKeyB = `{"":[]}` ) +var ( + specialChars = string([]byte{123, 34, 92, 98, 34, 58, 91, 93, 125}) +) + func TestCreatePatch(t *testing.T) { cases := []struct { name string @@ -888,6 +892,8 @@ func TestCreatePatch(t *testing.T) { {"Null Key uses replace operation", nullKeyA, nullKeyB}, // empty key {"Empty key", emptyKeyA, emptyKeyB}, + // special chars + {"Special chars", empty, specialChars}, } for _, c := range cases { From e0f6c248875c8fc66935c34ffcdd2ccde4eee5b7 Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 16 May 2023 08:42:18 -0700 Subject: [PATCH 09/11] Fix known broken case Signed-off-by: John Howard --- v2/fuzz_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/v2/fuzz_test.go b/v2/fuzz_test.go index a8dc311..26e436b 100644 --- a/v2/fuzz_test.go +++ b/v2/fuzz_test.go @@ -25,6 +25,7 @@ func FuzzCreatePatch(f *testing.F) { } func checkFuzz(t *testing.T, src, dst []byte) { + t.Logf("Test: %v -> %v", string(src), string(dst)) patch, err := jsonpatch.CreatePatch(src, dst) if err != nil { // Ok to error, src or dst may be invalid @@ -37,9 +38,17 @@ func checkFuzz(t *testing.T, src, dst []byte) { return } + for _, p := range patch { + if p.Path == "" { + // json-patch doesn't handle this properly, but it is valid + return + } + } + data, err := json.Marshal(patch) assert.Nil(t, err) + t.Logf("Applying patch %v", string(data)) p2, err := jp.DecodePatch(data) assert.Nil(t, err) From 45186ed81b95e176154e133ec4921379fbca6adb Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 16 May 2023 08:46:50 -0700 Subject: [PATCH 10/11] (hackily) remove inefficient optimization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``` name old time/op new time/op delta CreatePatch/complex-48 167µs ± 8% 156µs ± 4% -6.85% (p=0.001 n=10+10) CreatePatch/large_array-48 664ms ± 1% 2ms ± 3% -99.71% (p=0.000 n=10+10) CreatePatch/simple-48 2.95µs ± 2% 2.92µs ± 1% ~ (p=0.447 n=10+10) name old alloc/op new alloc/op delta CreatePatch/complex-48 75.8kB ± 0% 75.0kB ± 0% -0.95% (p=0.000 n=10+10) CreatePatch/large_array-48 153MB ± 0% 1MB ± 0% -99.39% (p=0.000 n=9+10) CreatePatch/simple-48 1.23kB ± 0% 1.23kB ± 0% +0.04% (p=0.033 n=10+10) name old allocs/op new allocs/op delta CreatePatch/complex-48 1.20k ± 0% 1.17k ± 0% -2.41% (p=0.000 n=10+10) CreatePatch/large_array-48 7.01M ± 0% 0.01M ± 0% -99.79% (p=0.000 n=10+10) CreatePatch/simple-48 29.0 ± 0% 29.0 ± 0% ~ (all equal) ``` Signed-off-by: John Howard --- v2/jsonpatch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/v2/jsonpatch.go b/v2/jsonpatch.go index 1d61a1f..d76a65a 100644 --- a/v2/jsonpatch.go +++ b/v2/jsonpatch.go @@ -214,7 +214,7 @@ func handleValues(av, bv interface{}, p string, patch []Operation) ([]Operation, } case []interface{}: bt := bv.([]interface{}) - if isSimpleArray(at) && isSimpleArray(bt) { + if false && isSimpleArray(at) && isSimpleArray(bt) { patch = append(patch, compareEditDistance(at, bt, p)...) } else { n := min(len(at), len(bt)) From 6d5c3df95c022c450a8222ad25ba635814768b19 Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 16 May 2023 08:54:35 -0700 Subject: [PATCH 11/11] Fully remove dead code Signed-off-by: John Howard --- v2/jsonpatch.go | 119 +++++------------------------------------------- 1 file changed, 12 insertions(+), 107 deletions(-) diff --git a/v2/jsonpatch.go b/v2/jsonpatch.go index d76a65a..a411d54 100644 --- a/v2/jsonpatch.go +++ b/v2/jsonpatch.go @@ -214,22 +214,18 @@ func handleValues(av, bv interface{}, p string, patch []Operation) ([]Operation, } case []interface{}: bt := bv.([]interface{}) - if false && isSimpleArray(at) && isSimpleArray(bt) { - patch = append(patch, compareEditDistance(at, bt, p)...) - } else { - n := min(len(at), len(bt)) - for i := len(at) - 1; i >= n; i-- { - patch = append(patch, NewOperation("remove", makePath(p, i), nil)) - } - for i := n; i < len(bt); i++ { - patch = append(patch, NewOperation("add", makePath(p, i), bt[i])) - } - for i := 0; i < n; i++ { - var err error - patch, err = handleValues(at[i], bt[i], makePath(p, i), patch) - if err != nil { - return nil, err - } + n := min(len(at), len(bt)) + for i := len(at) - 1; i >= n; i-- { + patch = append(patch, NewOperation("remove", makePath(p, i), nil)) + } + for i := n; i < len(bt); i++ { + patch = append(patch, NewOperation("add", makePath(p, i), bt[i])) + } + for i := 0; i < n; i++ { + var err error + patch, err = handleValues(at[i], bt[i], makePath(p, i), patch) + if err != nil { + return nil, err } } default: @@ -238,100 +234,9 @@ func handleValues(av, bv interface{}, p string, patch []Operation) ([]Operation, return patch, nil } -func isBasicType(a interface{}) bool { - switch a.(type) { - case string, float64, bool: - default: - return false - } - return true -} - -func isSimpleArray(a []interface{}) bool { - for i := range a { - switch a[i].(type) { - case string, float64, bool: - default: - val := reflect.ValueOf(a[i]) - if val.Kind() == reflect.Map { - for _, k := range val.MapKeys() { - av := val.MapIndex(k) - if av.Kind() == reflect.Ptr || av.Kind() == reflect.Interface { - if av.IsNil() { - continue - } - av = av.Elem() - } - if av.Kind() != reflect.String && av.Kind() != reflect.Float64 && av.Kind() != reflect.Bool { - return false - } - } - return true - } - return false - } - } - return true -} - -// https://en.wikipedia.org/wiki/Wagner%E2%80%93Fischer_algorithm -// Adapted from https://github.com/texttheater/golang-levenshtein -func compareEditDistance(s, t []interface{}, p string) []Operation { - m := len(s) - n := len(t) - - d := make([][]int, m+1) - for i := 0; i <= m; i++ { - d[i] = make([]int, n+1) - d[i][0] = i - } - for j := 0; j <= n; j++ { - d[0][j] = j - } - - for j := 1; j <= n; j++ { - for i := 1; i <= m; i++ { - if reflect.DeepEqual(s[i-1], t[j-1]) { - d[i][j] = d[i-1][j-1] // no op required - } else { - del := d[i-1][j] + 1 - add := d[i][j-1] + 1 - rep := d[i-1][j-1] + 1 - d[i][j] = min(rep, min(add, del)) - } - } - } - - return backtrace(s, t, p, m, n, d) -} - func min(x int, y int) int { if y < x { return y } return x } - -func backtrace(s, t []interface{}, p string, i int, j int, matrix [][]int) []Operation { - if i > 0 && matrix[i-1][j]+1 == matrix[i][j] { - op := NewOperation("remove", makePath(p, i-1), nil) - return append([]Operation{op}, backtrace(s, t, p, i-1, j, matrix)...) - } - if j > 0 && matrix[i][j-1]+1 == matrix[i][j] { - op := NewOperation("add", makePath(p, i), t[j-1]) - return append([]Operation{op}, backtrace(s, t, p, i, j-1, matrix)...) - } - if i > 0 && j > 0 && matrix[i-1][j-1]+1 == matrix[i][j] { - if isBasicType(s[0]) { - op := NewOperation("replace", makePath(p, i-1), t[j-1]) - return append([]Operation{op}, backtrace(s, t, p, i-1, j-1, matrix)...) - } - - p2, _ := handleValues(s[i-1], t[j-1], makePath(p, i-1), []Operation{}) - return append(p2, backtrace(s, t, p, i-1, j-1, matrix)...) - } - if i > 0 && j > 0 && matrix[i-1][j-1] == matrix[i][j] { - return backtrace(s, t, p, i-1, j-1, matrix) - } - return []Operation{} -}