From e63fa8516f71cdf8be6db3386a7fac0ed6435269 Mon Sep 17 00:00:00 2001 From: Pablo Andres Fuente Date: Tue, 10 Nov 2020 03:10:15 +0000 Subject: [PATCH 1/3] Adding a new config to DefaultBinder Now the DefaultBinder could be configured to avoid binding struct fields by name. This is particularly useful when the user don't want to bind certain struct fields (with this config in true, only the tagged fields will be binded) Fixes #1620, fixes #1631, partially fixes #1670 --- bind.go | 9 +++++++-- bind_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/bind.go b/bind.go index f89147435..07a85add2 100644 --- a/bind.go +++ b/bind.go @@ -19,7 +19,11 @@ type ( } // DefaultBinder is the default implementation of the Binder interface. - DefaultBinder struct{} + DefaultBinder struct { + // AvoidBindByFieldName avoid binding struct fields by name automatically. If it's set to true, the binding is + // only performed when a valid tag is pressent + AvoidBindByFieldName bool + } // BindUnmarshaler is the interface used to wrap the UnmarshalParam method. // Types that don't implement this, but do implement encoding.TextUnmarshaler @@ -113,13 +117,14 @@ func (b *DefaultBinder) bindData(ptr interface{}, data map[string][]string, tag inputFieldName := typeField.Tag.Get(tag) if inputFieldName == "" { - inputFieldName = typeField.Name // If tag is nil, we inspect if the field is a struct. if _, ok := structField.Addr().Interface().(BindUnmarshaler); !ok && structFieldKind == reflect.Struct { if err := b.bindData(structField.Addr().Interface(), data, tag); err != nil { return err } continue + } else if b.AvoidBindByFieldName == false { + inputFieldName = typeField.Name } } diff --git a/bind_test.go b/bind_test.go index b9fb9de3c..4d23e5072 100644 --- a/bind_test.go +++ b/bind_test.go @@ -330,6 +330,23 @@ func TestBindbindData(t *testing.T) { assertBindTestStruct(assert, ts) } +func TestBindbindDataByTags(t *testing.T) { + assert := assert.New(t) + ts := new(bindTestStructWithTags) + b := new(DefaultBinder) + b.bindData(ts, values, "form") + assertBindTestStruct(assert, (*bindTestStruct)(ts)) +} + +func TestBindbindDataAvoidBindByFieldName(t *testing.T) { + assert := assert.New(t) + ts := new(bindTestStruct) + b := new(DefaultBinder) + b.AvoidBindByFieldName = true + b.bindData(ts, values, "form") + assertBindTestStructDefaultValues(assert, ts) +} + func TestBindParam(t *testing.T) { e := New() req := httptest.NewRequest(GET, "/", nil) @@ -516,6 +533,24 @@ func assertBindTestStruct(a *assert.Assertions, ts *bindTestStruct) { a.Equal("", ts.GetCantSet()) } +func assertBindTestStructDefaultValues(a *assert.Assertions, ts *bindTestStruct) { + a.Equal(0, ts.I) + a.Equal(int8(0), ts.I8) + a.Equal(int16(0), ts.I16) + a.Equal(int32(0), ts.I32) + a.Equal(int64(0), ts.I64) + a.Equal(uint(0), ts.UI) + a.Equal(uint8(0), ts.UI8) + a.Equal(uint16(0), ts.UI16) + a.Equal(uint32(0), ts.UI32) + a.Equal(uint64(0), ts.UI64) + a.Equal(false, ts.B) + a.Equal(float32(0), ts.F32) + a.Equal(float64(0), ts.F64) + a.Equal("", ts.S) + a.Equal("", ts.GetCantSet()) +} + func testBindOkay(assert *assert.Assertions, r io.Reader, ctype string) { e := New() req := httptest.NewRequest(http.MethodPost, "/", r) From 9ba1225cc12fa78135c76383c4463b78aa5a53a8 Mon Sep 17 00:00:00 2001 From: Pablo Andres Fuente Date: Wed, 11 Nov 2020 03:11:40 +0000 Subject: [PATCH 2/3] Rewriting DefaultBinder#AvoidBindByFieldName doc Adding a JSON Bind new unit test --- bind.go | 4 +++- bind_test.go | 23 +++++++++++++++++++++++ echo_test.go | 1 + 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/bind.go b/bind.go index 07a85add2..873d8b00a 100644 --- a/bind.go +++ b/bind.go @@ -21,7 +21,9 @@ type ( // DefaultBinder is the default implementation of the Binder interface. DefaultBinder struct { // AvoidBindByFieldName avoid binding struct fields by name automatically. If it's set to true, the binding is - // only performed when a valid tag is pressent + // only performed when a valid tag is pressent. + // This doesn't apply for json/xml encoding. In this case you should use the features provided in the Go stdlib + // to achieve this. e.g. If you want to ignore a struct field during binding, you should add the tag `json:"-"` AvoidBindByFieldName bool } diff --git a/bind_test.go b/bind_test.go index 4d23e5072..e307c7a33 100644 --- a/bind_test.go +++ b/bind_test.go @@ -347,6 +347,29 @@ func TestBindbindDataAvoidBindByFieldName(t *testing.T) { assertBindTestStructDefaultValues(assert, ts) } +func TestBindAvoidBindingJsonStructField(t *testing.T) { + type User struct { + ID int `json:"id"` + IsAdmin bool `json:"-"` + } + + assert := assert.New(t) + + e := New() + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(userAvoidBindJSONField)) + req.Header.Set(HeaderContentType, MIMEApplicationJSON) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + defaultUser := &User{} + b := new(DefaultBinder) + b.Bind(defaultUser, c) + + assert.Equal(1, defaultUser.ID) + // This should be false because the Zero value of a bool is false and the JSON have it in true + assert.Equal(false, defaultUser.IsAdmin) +} + func TestBindParam(t *testing.T) { e := New() req := httptest.NewRequest(GET, "/", nil) diff --git a/echo_test.go b/echo_test.go index 0368dbd7a..995f76007 100644 --- a/echo_test.go +++ b/echo_test.go @@ -32,6 +32,7 @@ const ( userJSONInvalidType = `{"id":"1","name":"Jon Snow"}` userXMLConvertNumberError = `Number oneJon Snow` userXMLUnsupportedTypeError = `<>Number oneJon Snow` + userAvoidBindJSONField = `{"id": 1, "isAdmin": true}` ) const userJSONPretty = `{ From f6bd7bd5b6d4d82637b41eb1260943ab6e40af5a Mon Sep 17 00:00:00 2001 From: Pablo Andres Fuente Date: Fri, 13 Nov 2020 03:43:32 +0000 Subject: [PATCH 3/3] Adding DefaultBinder flags to prevent methods of binding Adding to the DefaultBinder one flag per binding method to prevent it if the flag is set to true. --- bind.go | 20 ++++-- bind_test.go | 171 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+), 5 deletions(-) diff --git a/bind.go b/bind.go index 873d8b00a..b3d58e4ab 100644 --- a/bind.go +++ b/bind.go @@ -25,6 +25,12 @@ type ( // This doesn't apply for json/xml encoding. In this case you should use the features provided in the Go stdlib // to achieve this. e.g. If you want to ignore a struct field during binding, you should add the tag `json:"-"` AvoidBindByFieldName bool + + // This flags are aimed to fully disable each one of the three binding methods supported by DefaultBinder + // Setting this flags to true, when you don't need that kind of binding, will result in performace gains. + DisableRouteParamsBinding bool + DisableQueryParamsBinding bool + DisableBodyBinding bool } // BindUnmarshaler is the interface used to wrap the UnmarshalParam method. @@ -46,13 +52,17 @@ func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) { for i, name := range names { params[name] = []string{values[i]} } - if err := b.bindData(i, params, "param"); err != nil { - return NewHTTPError(http.StatusBadRequest, err.Error()).SetInternal(err) + if !b.DisableRouteParamsBinding { + if err := b.bindData(i, params, "param"); err != nil { + return NewHTTPError(http.StatusBadRequest, err.Error()).SetInternal(err) + } } - if err = b.bindData(i, c.QueryParams(), "query"); err != nil { - return NewHTTPError(http.StatusBadRequest, err.Error()).SetInternal(err) + if !b.DisableQueryParamsBinding { + if err = b.bindData(i, c.QueryParams(), "query"); err != nil { + return NewHTTPError(http.StatusBadRequest, err.Error()).SetInternal(err) + } } - if req.ContentLength == 0 { + if b.DisableBodyBinding || req.ContentLength == 0 { return } ctype := req.Header.Get(HeaderContentType) diff --git a/bind_test.go b/bind_test.go index e307c7a33..48f702061 100644 --- a/bind_test.go +++ b/bind_test.go @@ -99,6 +99,22 @@ type ( Struct struct { Foo string } + jsonBenchmark struct { + One int `json:"One" form:"One" query:"One"` + Two int `json:"Two" form:"Two" query:"Two"` + Three int `json:"Three" form:"Three" query:"Three"` + Four int `json:"Four" form:"Four" query:"Four"` + Five int `json:"Five" form:"Five" query:"Five"` + Six int `json:"Six" form:"Six" query:"Six"` + Seven int `json:"Seven" form:"Seven" query:"Seven"` + Eigth int `json:"Eigth" form:"Eigth" query:"Eigth"` + Nine int `json:"Nine" form:"Nine" query:"Nine"` + Ten int `json:"Ten" form:"Ten" query:"Ten"` + } +) + +const ( + jsonBenchmarkJSON = `{"One":1,"Two":2,"Three":3,"Four":4,"Five":5,"Six":6,"Seven":7,"Eigth":8,"Nine":9,"Ten":10}` ) func (t *Timestamp) UnmarshalParam(src string) error { @@ -510,6 +526,95 @@ func TestBindSetFields(t *testing.T) { } } +func TestDefaultBinderConfiguration(t *testing.T) { + type Node struct { + ID int `json:"id"` + Node string `json:"node" param:"node"` + Next string `json:"next" param:"next"` + } + + var testCases = []struct { + name string + givenURL string + givenContent io.Reader + givenMethod string + givenBinder DefaultBinder + expectNode string + expectNext string + }{ + { + name: "bind to struct with route param + query param and binding by struct field name", + givenMethod: http.MethodPost, + givenURL: "/api/real_node/endpoint?node=xxx&next=real_next", + givenContent: strings.NewReader(`{"id": 1}`), + expectNode: "xxx", + expectNext: "real_next", + }, + { + name: "bind to struct with route param + query param and avoid binding by struct field name", + givenMethod: http.MethodPost, + givenURL: "/api/real_node/endpoint?node=xxx", + givenContent: strings.NewReader(`{"id": 1}`), + givenBinder: DefaultBinder{AvoidBindByFieldName: true}, + expectNode: "real_node", + }, + { + name: "bind to struct with route param + query param and disabling param binding", + givenMethod: http.MethodPost, + givenURL: "/api/real_node/endpoint?node=xxx", + givenContent: strings.NewReader(`{"id": 1}`), + givenBinder: DefaultBinder{DisableRouteParamsBinding: true}, + expectNode: "xxx", + }, + { + name: "bind to struct with route param + query param and disabling query binding", + givenMethod: http.MethodPost, + givenURL: "/api/real_node/endpoint?node=xxx&next=yyy", + givenContent: strings.NewReader(`{"id": 1}`), + givenBinder: DefaultBinder{DisableQueryParamsBinding: true}, + expectNode: "real_node", + expectNext: "", // Node.Next shouldn't be binded + }, + { + name: "bind to struct with route param + query param and disabling body binding", + givenMethod: http.MethodPost, + givenURL: "/api/real_node/endpoint", + givenContent: strings.NewReader(`{"id": 1, "node": yyy}`), + givenBinder: DefaultBinder{DisableBodyBinding: true}, + expectNode: "real_node", + }, + { + name: "bind to struct with route + query + body = body has priority", + givenMethod: http.MethodPost, + givenURL: "/api/real_node/endpoint?node=xxx", + givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`), + expectNode: "zzz", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + e := New() + // assume route we are testing is "/api/:node/endpoint" + req := httptest.NewRequest(tc.givenMethod, tc.givenURL, tc.givenContent) + req.Header.Set(HeaderContentType, MIMEApplicationJSON) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + c.SetParamNames("node") + c.SetParamValues("real_node") + + defaultUser := &Node{} + err := tc.givenBinder.Bind(defaultUser, c) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, tc.expectNode, defaultUser.Node) + assert.Equal(t, tc.expectNext, defaultUser.Next) + }) + } +} + func BenchmarkBindbindData(b *testing.B) { b.ReportAllocs() assert := assert.New(b) @@ -538,6 +643,72 @@ func BenchmarkBindbindDataWithTags(b *testing.B) { assertBindTestStruct(assert, (*bindTestStruct)(ts)) } +func BenchmarkBindByJsonBody(b *testing.B) { + b.ReportAllocs() + assert := assert.New(b) + + e := New() + var err error + var jb *jsonBenchmark + + b.ResetTimer() + for i := 0; i < b.N; i++ { + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(jsonBenchmarkJSON)) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + req.Header.Set(HeaderContentType, MIMEApplicationJSON) + jb = new(jsonBenchmark) + err = c.Bind(jb) + } + if assert.NoError(err) { + assert.Equal(1, jb.One) + assert.Equal(2, jb.Two) + assert.Equal(3, jb.Three) + assert.Equal(4, jb.Four) + assert.Equal(5, jb.Five) + assert.Equal(6, jb.Six) + assert.Equal(7, jb.Seven) + assert.Equal(8, jb.Eigth) + assert.Equal(9, jb.Nine) + assert.Equal(10, jb.Ten) + } +} + +func BenchmarkBindByJsonBodyDisablingRouteParamQueryParamBinding(b *testing.B) { + b.ReportAllocs() + assert := assert.New(b) + + e := New() + e.Binder = &DefaultBinder{ + DisableQueryParamsBinding: true, + DisableRouteParamsBinding: true, + } + var err error + var jb *jsonBenchmark + + b.ResetTimer() + for i := 0; i < b.N; i++ { + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(jsonBenchmarkJSON)) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + req.Header.Set(HeaderContentType, MIMEApplicationJSON) + jb = new(jsonBenchmark) + err = c.Bind(jb) + } + if assert.NoError(err) { + assert.Equal(1, jb.One) + assert.Equal(2, jb.Two) + assert.Equal(3, jb.Three) + assert.Equal(4, jb.Four) + assert.Equal(5, jb.Five) + assert.Equal(6, jb.Six) + assert.Equal(7, jb.Seven) + assert.Equal(8, jb.Eigth) + assert.Equal(9, jb.Nine) + assert.Equal(10, jb.Ten) + } +} + func assertBindTestStruct(a *assert.Assertions, ts *bindTestStruct) { a.Equal(0, ts.I) a.Equal(int8(8), ts.I8)