From 15e9387ca00c99ba2d69ffacaba548e0e1be0b76 Mon Sep 17 00:00:00 2001 From: guonaihong Date: Sat, 13 Jul 2019 21:05:23 +0800 Subject: [PATCH 1/2] Improve the problem described in #1978 In the go, the stack burst will occupy 1G memory, the error message is as follows ```runtime: goroutine stack exceeds 1000000000-byte limit``` If it is a user of gin, the structure of the circular reference is defined on the server side, and the server memory may be used up as long as 100 go processes, and even the server may be down. So I suggest that the mapping function should add a limit on the number of calls, and the number of times can be configured. Of course, I tried several other modifications when I solved the stackoverflow problem described in #1978, but it was not compatible with the existing API, so I did not adopt the solution I tried. --- binding/form_mapping.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/binding/form_mapping.go b/binding/form_mapping.go index 80b1d15a..c4543f9a 100644 --- a/binding/form_mapping.go +++ b/binding/form_mapping.go @@ -15,6 +15,8 @@ import ( "github.com/gin-gonic/gin/internal/json" ) +var LimitMappingCallNumber = 10000 + var errUnknownType = errors.New("Unknown type") func mapUri(ptr interface{}, m map[string][]string) error { @@ -46,11 +48,11 @@ func (form formSource) TrySet(value reflect.Value, field reflect.StructField, ta } func mappingByPtr(ptr interface{}, setter setter, tag string) error { - _, err := mapping(reflect.ValueOf(ptr), emptyField, setter, tag) + _, err := mapping(reflect.ValueOf(ptr), emptyField, setter, tag, 0) return err } -func mapping(value reflect.Value, field reflect.StructField, setter setter, tag string) (bool, error) { +func mapping(value reflect.Value, field reflect.StructField, setter setter, tag string, callNumber int) (bool, error) { var vKind = value.Kind() if vKind == reflect.Ptr { @@ -60,7 +62,7 @@ func mapping(value reflect.Value, field reflect.StructField, setter setter, tag isNew = true vPtr = reflect.New(value.Type().Elem()) } - isSetted, err := mapping(vPtr.Elem(), field, setter, tag) + isSetted, err := mapping(vPtr.Elem(), field, setter, tag, callNumber+1) if err != nil { return false, err } @@ -70,6 +72,10 @@ func mapping(value reflect.Value, field reflect.StructField, setter setter, tag return isSetted, nil } + if callNumber > LimitMappingCallNumber { + return false, errors.New("Maybe entering a circular reference") + } + if vKind != reflect.Struct || !field.Anonymous { ok, err := tryToSetValue(value, field, setter, tag) if err != nil { @@ -89,7 +95,7 @@ func mapping(value reflect.Value, field reflect.StructField, setter setter, tag if sf.PkgPath != "" && !sf.Anonymous { // unexported continue } - ok, err := mapping(value.Field(i), tValue.Field(i), setter, tag) + ok, err := mapping(value.Field(i), tValue.Field(i), setter, tag, callNumber+1) if err != nil { return false, err } From 36f6c493d431dbe4ddad3ab179c44257e7307f26 Mon Sep 17 00:00:00 2001 From: guonaihong Date: Mon, 15 Jul 2019 22:58:16 +0800 Subject: [PATCH 2/2] add test function --- binding/binding_test.go | 14 ++++++++++++++ binding/form_mapping.go | 3 ++- binding/form_mapping_test.go | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/binding/binding_test.go b/binding/binding_test.go index 806f3ac9..cfafb83a 100644 --- a/binding/binding_test.go +++ b/binding/binding_test.go @@ -1177,6 +1177,20 @@ func testBodyBindingFail(t *testing.T, b Binding, name, path, badPath, body, bad assert.Error(t, err) } +type Identifier struct { + Assigner *Reference `json:"assigner,omitempty"` +} + +type Reference struct { + Identifier *Identifier `json:"identifier,omitempty"` +} + +func TestLoopReference(t *testing.T) { + r := Reference{} + err := mapForm(&r, map[string][]string{}) + assert.Error(t, err) +} + func testProtoBodyBinding(t *testing.T, b Binding, name, path, badPath, body, badBody string) { assert.Equal(t, name, b.Name()) diff --git a/binding/form_mapping.go b/binding/form_mapping.go index c4543f9a..48db75c2 100644 --- a/binding/form_mapping.go +++ b/binding/form_mapping.go @@ -18,6 +18,7 @@ import ( var LimitMappingCallNumber = 10000 var errUnknownType = errors.New("Unknown type") +var ErrMaybeCircularReference = errors.New("Maybe entering a circular reference") func mapUri(ptr interface{}, m map[string][]string) error { return mapFormByTag(ptr, m, "uri") @@ -73,7 +74,7 @@ func mapping(value reflect.Value, field reflect.StructField, setter setter, tag } if callNumber > LimitMappingCallNumber { - return false, errors.New("Maybe entering a circular reference") + return false, ErrMaybeCircularReference } if vKind != reflect.Struct || !field.Anonymous { diff --git a/binding/form_mapping_test.go b/binding/form_mapping_test.go index c9d6111b..04145c21 100644 --- a/binding/form_mapping_test.go +++ b/binding/form_mapping_test.go @@ -52,7 +52,7 @@ func TestMappingBaseTypes(t *testing.T) { field := val.Elem().Type().Field(0) - _, err := mapping(val, emptyField, formSource{field.Name: {tt.form}}, "form") + _, err := mapping(val, emptyField, formSource{field.Name: {tt.form}}, "form", 0) assert.NoError(t, err, testName) actual := val.Elem().Field(0).Interface()