From ad1fa5da9821472dbf47098667c2b7160ff2628f Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Mon, 28 Jan 2019 17:36:17 -0800 Subject: [PATCH 1/3] According to the RFC, the move operation should be functionally identical to a remove followed by an add. --- patch.go | 2 +- patch_test.go | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/patch.go b/patch.go index a708469..a2b12e2 100644 --- a/patch.go +++ b/patch.go @@ -565,7 +565,7 @@ func (p Patch) move(doc *container, op operation) error { return fmt.Errorf("jsonpatch move operation does not apply: doc is missing destination path: %s", path) } - return con.set(key, val) + return con.add(key, val) } func (p Patch) test(doc *container, op operation) error { diff --git a/patch_test.go b/patch_test.go index 85062c9..63ca766 100644 --- a/patch_test.go +++ b/patch_test.go @@ -121,6 +121,11 @@ var Cases = []Case{ `[ { "op": "move", "from": "/foo/1", "path": "/foo/3" } ]`, `{ "foo": [ "all", "cows", "eat", "grass" ] }`, }, + { + `{ "foo": [ "all", "grass", "cows", "eat" ] }`, + `[ { "op": "move", "from": "/foo/1", "path": "/foo/2" } ]`, + `{ "foo": [ "all", "cows", "grass", "eat" ] }`, + }, { `{ "foo": "bar" }`, `[ { "op": "add", "path": "/child", "value": { "grandchild": { } } } ]`, @@ -322,6 +327,11 @@ var BadCases = []BadCase{ `[ { "op": "copy", "path": "/foo/-", "from": "/foo/1" }, { "op": "copy", "path": "/foo/-", "from": "/foo/1" }]`, }, + // 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" } ]`, + }, } // This is not thread safe, so we cannot run patch tests in parallel. From 500d67a63e4062af846743a3c58b8d7622fe2401 Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Fri, 1 Feb 2019 14:39:51 -0800 Subject: [PATCH 2/3] Since that the "set" method is now only called to implement the "replace" operation, and the "path" of "replace" must refer to an existing location, we can greatly simplify the sanity check in "set". Also, we removed the ArraySizeAdditionLimit, because according to the RFC, no operation can increase the length of an existing array by more than 1. --- README.md | 3 +++ patch.go | 37 +++---------------------------------- patch_test.go | 7 ++----- 3 files changed, 8 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index 06b7efe..7778fdb 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,7 @@ go get -u github.com/evanphx/json-patch limits the length of any array the patched object can have. It defaults to 0, which means there is no limit. +<<<<<<< HEAD * There is a global configuration variable `jsonpatch.ArraySizeAdditionLimit`, which limits the increase of array length caused by each operation. It defaults to 0, which means there is no limit. @@ -46,6 +47,8 @@ go get -u github.com/evanphx/json-patch which limits the total size increase in bytes caused by "copy" operations in a patch. It defaults to 0, which means there is no limit. +======= +>>>>>>> Since that the "set" method is now only called to implement the ## Create and apply a merge patch Given both an original JSON document and a modified JSON document, you can create a [Merge Patch](https://tools.ietf.org/html/rfc7396) document. diff --git a/patch.go b/patch.go index a2b12e2..9a5a1db 100644 --- a/patch.go +++ b/patch.go @@ -20,7 +20,6 @@ var ( // Default to true. SupportNegativeIndices bool = true ArraySizeLimit int = 0 - ArraySizeAdditionLimit int = 0 // AccumulatedCopySizeLimit limits the total size increase in bytes caused by // "copy" operations in a patch. AccumulatedCopySizeLimit int64 = 0 @@ -368,44 +367,14 @@ func (d *partialDoc) remove(key string) error { return nil } +// set should only be used to implement the "replace" operation, so "key" must +// be an already existing index in "d". func (d *partialArray) set(key string, val *lazyNode) error { - if key == "-" { - *d = append(*d, val) - return nil - } - idx, err := strconv.Atoi(key) if err != nil { return err } - - sz := len(*d) - - if diff := idx + 1 - sz; ArraySizeAdditionLimit > 0 && diff > ArraySizeAdditionLimit { - return fmt.Errorf("Unable to increase the array size by %d, the limit is %d", diff, ArraySizeAdditionLimit) - } - - if idx+1 > sz { - sz = idx + 1 - } - - if ArraySizeLimit > 0 && sz > ArraySizeLimit { - return NewArraySizeError(ArraySizeLimit, sz) - } - - ary := make([]*lazyNode, sz) - - cur := *d - - copy(ary, cur) - - if idx >= len(ary) { - return fmt.Errorf("Unable to access invalid index: %d", idx) - } - - ary[idx] = val - - *d = ary + (*d)[idx] = val return nil } diff --git a/patch_test.go b/patch_test.go index 63ca766..120d8e7 100644 --- a/patch_test.go +++ b/patch_test.go @@ -335,22 +335,19 @@ var BadCases = []BadCase{ } // This is not thread safe, so we cannot run patch tests in parallel. -func configureGlobals(arraySizeLimit, arraySizeAdditionLimit int, accumulatedCopySizeLimit int64) func() { +func configureGlobals(arraySizeLimit int, accumulatedCopySizeLimit int64) func() { oldArraySizeLimit := ArraySizeLimit - oldArraySizeAdditionLimit := ArraySizeAdditionLimit oldAccumulatedCopySizeLimit := AccumulatedCopySizeLimit ArraySizeLimit = arraySizeLimit - ArraySizeAdditionLimit = arraySizeAdditionLimit AccumulatedCopySizeLimit = accumulatedCopySizeLimit return func() { ArraySizeLimit = oldArraySizeLimit - ArraySizeAdditionLimit = oldArraySizeAdditionLimit AccumulatedCopySizeLimit = oldAccumulatedCopySizeLimit } } func TestAllCases(t *testing.T) { - defer configureGlobals(1000, 10, int64(100))() + defer configureGlobals(1000, int64(100))() for _, c := range Cases { out, err := applyPatch(c.doc, c.patch) From bbf30d6397377bd990f39adc3bc3446cb26478e4 Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Fri, 1 Feb 2019 15:20:51 -0800 Subject: [PATCH 3/3] Removed the ArraySizeLimit. It's not useful anymore because: 1. There used to be bugs in the library that allowed the "path" field of the "move" and "copy" operations to refer to arbitrarily large index of an existing array. Now the bugs are fixed, the two operations can at most increase the length of any _existing_ array by 1, by appending to the end. 2. The "add" and "replace" operation can create large new arrays. However, for these two operations, the patch MUST contain the "value", thus the caller of this library can cap the size by limiting the size of the patch. 3. The "copy" operations can copy existing large array, but users can control the size increase by setting AccumulatedCopySizeLimit. --- README.md | 11 ----------- patch.go | 4 ---- patch_test.go | 7 ++----- 3 files changed, 2 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 7778fdb..9c7f87f 100644 --- a/README.md +++ b/README.md @@ -34,21 +34,10 @@ go get -u github.com/evanphx/json-patch functionality can be disabled by setting `jsonpatch.SupportNegativeIndices = false`. -* There is a global configuration variable `jsonpatch.ArraySizeLimit`, which - limits the length of any array the patched object can have. It defaults to 0, - which means there is no limit. - -<<<<<<< HEAD -* There is a global configuration variable `jsonpatch.ArraySizeAdditionLimit`, - which limits the increase of array length caused by each operation. It - defaults to 0, which means there is no limit. - * There is a global configuration variable `jsonpatch.AccumulatedCopySizeLimit`, which limits the total size increase in bytes caused by "copy" operations in a patch. It defaults to 0, which means there is no limit. -======= ->>>>>>> Since that the "set" method is now only called to implement the ## Create and apply a merge patch Given both an original JSON document and a modified JSON document, you can create a [Merge Patch](https://tools.ietf.org/html/rfc7396) document. diff --git a/patch.go b/patch.go index 9a5a1db..c9cf590 100644 --- a/patch.go +++ b/patch.go @@ -19,7 +19,6 @@ var ( // allowing negative indices to mean indices starting at the end of an array. // Default to true. SupportNegativeIndices bool = true - ArraySizeLimit int = 0 // AccumulatedCopySizeLimit limits the total size increase in bytes caused by // "copy" operations in a patch. AccumulatedCopySizeLimit int64 = 0 @@ -390,9 +389,6 @@ func (d *partialArray) add(key string, val *lazyNode) error { } sz := len(*d) + 1 - if ArraySizeLimit > 0 && sz > ArraySizeLimit { - return fmt.Errorf("Unable to create array of size %d, limit is %d", sz, ArraySizeLimit) - } ary := make([]*lazyNode, sz) diff --git a/patch_test.go b/patch_test.go index 120d8e7..96bcef4 100644 --- a/patch_test.go +++ b/patch_test.go @@ -335,19 +335,16 @@ var BadCases = []BadCase{ } // This is not thread safe, so we cannot run patch tests in parallel. -func configureGlobals(arraySizeLimit int, accumulatedCopySizeLimit int64) func() { - oldArraySizeLimit := ArraySizeLimit +func configureGlobals(accumulatedCopySizeLimit int64) func() { oldAccumulatedCopySizeLimit := AccumulatedCopySizeLimit - ArraySizeLimit = arraySizeLimit AccumulatedCopySizeLimit = accumulatedCopySizeLimit return func() { - ArraySizeLimit = oldArraySizeLimit AccumulatedCopySizeLimit = oldAccumulatedCopySizeLimit } } func TestAllCases(t *testing.T) { - defer configureGlobals(1000, int64(100))() + defer configureGlobals(int64(100))() for _, c := range Cases { out, err := applyPatch(c.doc, c.patch)