From 5ea40da5f44614a910b370fc223d9517d0a28ea9 Mon Sep 17 00:00:00 2001 From: lujjjh Date: Thu, 13 Aug 2020 21:46:12 +0800 Subject: [PATCH 1/5] encoding/json: detect cyclic maps and slices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The documentation says: JSON cannot represent cyclic data structures and Marshal does not handle them. Passing cyclic structures to Marshal will result in an error. However, passing cyclic maps or slices still results in an infinite recursion. Cycle detection is now added into mapEncoder and sliceEncoder. name old time/op new time/op delta CodeEncoder-16 933µs ± 0% 1324µs ± 0% ~ (p=1.000 n=1+1) CodeMarshal-16 940µs ± 0% 1436µs ± 0% ~ (p=1.000 n=1+1) name old speed new speed delta CodeEncoder-16 2.08GB/s ± 0% 1.47GB/s ± 0% ~ (p=1.000 n=1+1) CodeMarshal-16 2.06GB/s ± 0% 1.35GB/s ± 0% ~ (p=1.000 n=1+1) name old alloc/op new alloc/op delta CodeEncoder-16 7.00B ± 0% 94933.00B ± 0% ~ (p=1.000 n=1+1) CodeMarshal-16 1.98MB ± 0% 2.04MB ± 0% ~ (p=1.000 n=1+1) name old allocs/op new allocs/op delta CodeEncoder-16 0.00 11816.00 ± 0% ~ (p=1.000 n=1+1) CodeMarshal-16 1.00 ± 0% 11816.00 ± 0% ~ (p=1.000 n=1+1) Fixes #40745. --- src/encoding/json/encode.go | 22 ++++++++++++++++++++++ src/encoding/json/encode_test.go | 11 ++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/encoding/json/encode.go b/src/encoding/json/encode.go index 578d551102de47..51d76bf236ec36 100644 --- a/src/encoding/json/encode.go +++ b/src/encoding/json/encode.go @@ -779,6 +779,16 @@ func (me mapEncoder) encode(e *encodeState, v reflect.Value, opts encOpts) { e.WriteString("null") return } + if e.ptrLevel++; e.ptrLevel > startDetectingCyclesAfter { + // We're a large number of nested ptrEncoder.encode calls deep; + // start checking if we've run into a pointer cycle. + ptr := v.Pointer() + if _, ok := e.ptrSeen[ptr]; ok { + e.error(&UnsupportedValueError{v, fmt.Sprintf("encountered a cycle via %s", v.Type())}) + } + e.ptrSeen[ptr] = struct{}{} + defer delete(e.ptrSeen, ptr) + } e.WriteByte('{') // Extract and sort the keys. @@ -801,6 +811,7 @@ func (me mapEncoder) encode(e *encodeState, v reflect.Value, opts encOpts) { me.elemEnc(e, v.MapIndex(kv.v), opts) } e.WriteByte('}') + e.ptrLevel-- } func newMapEncoder(t reflect.Type) encoderFunc { @@ -857,7 +868,18 @@ func (se sliceEncoder) encode(e *encodeState, v reflect.Value, opts encOpts) { e.WriteString("null") return } + if e.ptrLevel++; e.ptrLevel > startDetectingCyclesAfter { + // We're a large number of nested ptrEncoder.encode calls deep; + // start checking if we've run into a pointer cycle. + ptr := v.Pointer() + if _, ok := e.ptrSeen[ptr]; ok { + e.error(&UnsupportedValueError{v, fmt.Sprintf("encountered a cycle via %s", v.Type())}) + } + e.ptrSeen[ptr] = struct{}{} + defer delete(e.ptrSeen, ptr) + } se.arrayEnc(e, v, opts) + e.ptrLevel-- } func newSliceEncoder(t reflect.Type) encoderFunc { diff --git a/src/encoding/json/encode_test.go b/src/encoding/json/encode_test.go index 7290eca06f0707..5e6dceb0ef2293 100644 --- a/src/encoding/json/encode_test.go +++ b/src/encoding/json/encode_test.go @@ -183,7 +183,11 @@ type PointerCycleIndirect struct { Ptrs []interface{} } -var pointerCycleIndirect = &PointerCycleIndirect{} +var ( + pointerCycleIndirect = &PointerCycleIndirect{} + mapCycle = make(map[string]interface{}) + sliceCycle = []interface{}{nil} +) func init() { ptr := &SamePointerNoCycle{} @@ -192,6 +196,9 @@ func init() { pointerCycle.Ptr = pointerCycle pointerCycleIndirect.Ptrs = []interface{}{pointerCycleIndirect} + + mapCycle["x"] = mapCycle + sliceCycle[0] = sliceCycle } func TestSamePointerNoCycle(t *testing.T) { @@ -206,6 +213,8 @@ var unsupportedValues = []interface{}{ math.Inf(1), pointerCycle, pointerCycleIndirect, + mapCycle, + sliceCycle, } func TestUnsupportedValues(t *testing.T) { From b69943d455bb8227bfde3f8529642b48e6051b0b Mon Sep 17 00:00:00 2001 From: lujjjh Date: Thu, 13 Aug 2020 23:12:46 +0800 Subject: [PATCH 2/5] Fix slice pointer --- src/encoding/json/encode.go | 4 +++- src/encoding/json/encode_test.go | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/encoding/json/encode.go b/src/encoding/json/encode.go index 51d76bf236ec36..7f059f7dd89488 100644 --- a/src/encoding/json/encode.go +++ b/src/encoding/json/encode.go @@ -871,7 +871,9 @@ func (se sliceEncoder) encode(e *encodeState, v reflect.Value, opts encOpts) { if e.ptrLevel++; e.ptrLevel > startDetectingCyclesAfter { // We're a large number of nested ptrEncoder.encode calls deep; // start checking if we've run into a pointer cycle. - ptr := v.Pointer() + p := reflect.New(reflect.TypeOf(v)) + p.Elem().Set(reflect.ValueOf(v)) + ptr := p.Pointer() if _, ok := e.ptrSeen[ptr]; ok { e.error(&UnsupportedValueError{v, fmt.Sprintf("encountered a cycle via %s", v.Type())}) } diff --git a/src/encoding/json/encode_test.go b/src/encoding/json/encode_test.go index 5e6dceb0ef2293..57e93ab54d81b3 100644 --- a/src/encoding/json/encode_test.go +++ b/src/encoding/json/encode_test.go @@ -187,6 +187,7 @@ var ( pointerCycleIndirect = &PointerCycleIndirect{} mapCycle = make(map[string]interface{}) sliceCycle = []interface{}{nil} + sliceNoCycle = []interface{}{nil, nil} ) func init() { @@ -199,6 +200,10 @@ func init() { mapCycle["x"] = mapCycle sliceCycle[0] = sliceCycle + sliceNoCycle[1] = sliceNoCycle[:1] + for i := startDetectingCyclesAfter; i > 0; i-- { + sliceNoCycle = []interface{}{sliceNoCycle} + } } func TestSamePointerNoCycle(t *testing.T) { @@ -207,6 +212,12 @@ func TestSamePointerNoCycle(t *testing.T) { } } +func TestSliceNoCycle(t *testing.T) { + if _, err := Marshal(sliceNoCycle); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + var unsupportedValues = []interface{}{ math.NaN(), math.Inf(-1), From 409d9161bf3db5d812316a8494e5a4b45d59b275 Mon Sep 17 00:00:00 2001 From: lujjjh Date: Tue, 25 Aug 2020 23:03:28 +0800 Subject: [PATCH 3/5] Use a struct to record slice --- src/encoding/json/encode.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/encoding/json/encode.go b/src/encoding/json/encode.go index 7f059f7dd89488..81d018bf249229 100644 --- a/src/encoding/json/encode.go +++ b/src/encoding/json/encode.go @@ -871,9 +871,11 @@ func (se sliceEncoder) encode(e *encodeState, v reflect.Value, opts encOpts) { if e.ptrLevel++; e.ptrLevel > startDetectingCyclesAfter { // We're a large number of nested ptrEncoder.encode calls deep; // start checking if we've run into a pointer cycle. - p := reflect.New(reflect.TypeOf(v)) - p.Elem().Set(reflect.ValueOf(v)) - ptr := p.Pointer() + // Here we use a struct to record the pointer of the first element and the length. + ptr := struct { + ptr uintptr + len int + }{v.Pointer(), v.Len()} if _, ok := e.ptrSeen[ptr]; ok { e.error(&UnsupportedValueError{v, fmt.Sprintf("encountered a cycle via %s", v.Type())}) } From d80d86b4c3762370d5148e3f78478376eedd5fbb Mon Sep 17 00:00:00 2001 From: lujjjh Date: Tue, 25 Aug 2020 23:12:44 +0800 Subject: [PATCH 4/5] Fix typo in the comment --- src/encoding/json/encode.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/encoding/json/encode.go b/src/encoding/json/encode.go index 81d018bf249229..ee2728be11c1bd 100644 --- a/src/encoding/json/encode.go +++ b/src/encoding/json/encode.go @@ -871,7 +871,8 @@ func (se sliceEncoder) encode(e *encodeState, v reflect.Value, opts encOpts) { if e.ptrLevel++; e.ptrLevel > startDetectingCyclesAfter { // We're a large number of nested ptrEncoder.encode calls deep; // start checking if we've run into a pointer cycle. - // Here we use a struct to record the pointer of the first element and the length. + // Here we use a struct to memorize the pointer to the first element of the slice + // and its length. ptr := struct { ptr uintptr len int From 6f874944f4065b5237babbb0fdce14c1c74a3c97 Mon Sep 17 00:00:00 2001 From: lujjjh Date: Thu, 17 Sep 2020 22:36:52 +0800 Subject: [PATCH 5/5] Add recursiveSliceCycle case --- src/encoding/json/encode_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/encoding/json/encode_test.go b/src/encoding/json/encode_test.go index 57e93ab54d81b3..42bb09d5cded0b 100644 --- a/src/encoding/json/encode_test.go +++ b/src/encoding/json/encode_test.go @@ -183,11 +183,14 @@ type PointerCycleIndirect struct { Ptrs []interface{} } +type RecursiveSlice []RecursiveSlice + var ( pointerCycleIndirect = &PointerCycleIndirect{} mapCycle = make(map[string]interface{}) sliceCycle = []interface{}{nil} sliceNoCycle = []interface{}{nil, nil} + recursiveSliceCycle = []RecursiveSlice{nil} ) func init() { @@ -204,6 +207,7 @@ func init() { for i := startDetectingCyclesAfter; i > 0; i-- { sliceNoCycle = []interface{}{sliceNoCycle} } + recursiveSliceCycle[0] = recursiveSliceCycle } func TestSamePointerNoCycle(t *testing.T) { @@ -226,6 +230,7 @@ var unsupportedValues = []interface{}{ pointerCycleIndirect, mapCycle, sliceCycle, + recursiveSliceCycle, } func TestUnsupportedValues(t *testing.T) {