Skip to content

Commit 02ed3f3

Browse files
authored
Fix #1729 Binding query/path params and form fields to struct only works for explicit tags (#1734)
* Binding query/path params and form fields to struct only works for fields that have explicit TAG defined on struct * remove unnecessary benchmark after change because it is not valid test anymore
1 parent f718079 commit 02ed3f3

File tree

2 files changed

+69
-35
lines changed

2 files changed

+69
-35
lines changed

bind.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,13 @@ func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) {
116116
return b.BindBody(c, i)
117117
}
118118

119-
func (b *DefaultBinder) bindData(ptr interface{}, data map[string][]string, tag string) error {
120-
if ptr == nil || len(data) == 0 {
119+
// bindData will bind data ONLY fields in destination struct that have EXPLICIT tag
120+
func (b *DefaultBinder) bindData(destination interface{}, data map[string][]string, tag string) error {
121+
if destination == nil || len(data) == 0 {
121122
return nil
122123
}
123-
typ := reflect.TypeOf(ptr).Elem()
124-
val := reflect.ValueOf(ptr).Elem()
124+
typ := reflect.TypeOf(destination).Elem()
125+
val := reflect.ValueOf(destination).Elem()
125126

126127
// Map
127128
if typ.Kind() == reflect.Map {
@@ -146,14 +147,15 @@ func (b *DefaultBinder) bindData(ptr interface{}, data map[string][]string, tag
146147
inputFieldName := typeField.Tag.Get(tag)
147148

148149
if inputFieldName == "" {
149-
inputFieldName = typeField.Name
150-
// If tag is nil, we inspect if the field is a struct.
150+
// If tag is nil, we inspect if the field is a not BindUnmarshaler struct and try to bind data into it (might contains fields with tags).
151+
// structs that implement BindUnmarshaler are binded only when they have explicit tag
151152
if _, ok := structField.Addr().Interface().(BindUnmarshaler); !ok && structFieldKind == reflect.Struct {
152153
if err := b.bindData(structField.Addr().Interface(), data, tag); err != nil {
153154
return err
154155
}
155-
continue
156156
}
157+
// does not have explicit tag and is not an ordinary struct - so move to next field
158+
continue
157159
}
158160

159161
inputValue, exists := data[inputFieldName]

bind_test.go

Lines changed: 60 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,31 @@ var values = map[string][]string{
160160
"ST": {"bar"},
161161
}
162162

163+
func TestToMultipleFields(t *testing.T) {
164+
e := New()
165+
req := httptest.NewRequest(http.MethodGet, "/?id=1&ID=2", nil)
166+
rec := httptest.NewRecorder()
167+
c := e.NewContext(req, rec)
168+
169+
type Root struct {
170+
ID int64 `query:"id"`
171+
Child2 struct {
172+
ID int64
173+
}
174+
Child1 struct {
175+
ID int64 `query:"id"`
176+
}
177+
}
178+
179+
u := new(Root)
180+
err := c.Bind(u)
181+
if assert.NoError(t, err) {
182+
assert.Equal(t, int64(1), u.ID) // perfectly reasonable
183+
assert.Equal(t, int64(1), u.Child1.ID) // untagged struct containing tagged field gets filled (by tag)
184+
assert.Equal(t, int64(0), u.Child2.ID) // untagged struct containing untagged field should not be bind
185+
}
186+
}
187+
163188
func TestBindJSON(t *testing.T) {
164189
assert := assert.New(t)
165190
testBindOkay(assert, strings.NewReader(userJSON), MIMEApplicationJSON)
@@ -238,10 +263,13 @@ func TestBindUnmarshalParam(t *testing.T) {
238263
rec := httptest.NewRecorder()
239264
c := e.NewContext(req, rec)
240265
result := struct {
241-
T Timestamp `query:"ts"`
242-
TA []Timestamp `query:"ta"`
243-
SA StringArray `query:"sa"`
244-
ST Struct
266+
T Timestamp `query:"ts"`
267+
TA []Timestamp `query:"ta"`
268+
SA StringArray `query:"sa"`
269+
ST Struct
270+
StWithTag struct {
271+
Foo string `query:"st"`
272+
}
245273
}{}
246274
err := c.Bind(&result)
247275
ts := Timestamp(time.Date(2016, 12, 6, 19, 9, 5, 0, time.UTC))
@@ -252,7 +280,8 @@ func TestBindUnmarshalParam(t *testing.T) {
252280
assert.Equal(ts, result.T)
253281
assert.Equal(StringArray([]string{"one", "two", "three"}), result.SA)
254282
assert.Equal([]Timestamp{ts, ts}, result.TA)
255-
assert.Equal(Struct{"baz"}, result.ST)
283+
assert.Equal(Struct{""}, result.ST) // child struct does not have a field with matching tag
284+
assert.Equal("baz", result.StWithTag.Foo) // child struct has field with matching tag
256285
}
257286
}
258287

@@ -274,7 +303,7 @@ func TestBindUnmarshalText(t *testing.T) {
274303
assert.Equal(t, ts, result.T)
275304
assert.Equal(t, StringArray([]string{"one", "two", "three"}), result.SA)
276305
assert.Equal(t, []time.Time{ts, ts}, result.TA)
277-
assert.Equal(t, Struct{"baz"}, result.ST)
306+
assert.Equal(t, Struct{""}, result.ST) // field in child struct does not have tag
278307
}
279308
}
280309

@@ -323,11 +352,27 @@ func TestBindUnsupportedMediaType(t *testing.T) {
323352
}
324353

325354
func TestBindbindData(t *testing.T) {
326-
assert := assert.New(t)
355+
a := assert.New(t)
327356
ts := new(bindTestStruct)
328357
b := new(DefaultBinder)
329-
b.bindData(ts, values, "form")
330-
assertBindTestStruct(assert, ts)
358+
err := b.bindData(ts, values, "form")
359+
a.NoError(err)
360+
361+
a.Equal(0, ts.I)
362+
a.Equal(int8(0), ts.I8)
363+
a.Equal(int16(0), ts.I16)
364+
a.Equal(int32(0), ts.I32)
365+
a.Equal(int64(0), ts.I64)
366+
a.Equal(uint(0), ts.UI)
367+
a.Equal(uint8(0), ts.UI8)
368+
a.Equal(uint16(0), ts.UI16)
369+
a.Equal(uint32(0), ts.UI32)
370+
a.Equal(uint64(0), ts.UI64)
371+
a.Equal(false, ts.B)
372+
a.Equal(float32(0), ts.F32)
373+
a.Equal(float64(0), ts.F64)
374+
a.Equal("", ts.S)
375+
a.Equal("", ts.cantSet)
331376
}
332377

333378
func TestBindParam(t *testing.T) {
@@ -470,20 +515,6 @@ func TestBindSetFields(t *testing.T) {
470515
}
471516
}
472517

473-
func BenchmarkBindbindData(b *testing.B) {
474-
b.ReportAllocs()
475-
assert := assert.New(b)
476-
ts := new(bindTestStruct)
477-
binder := new(DefaultBinder)
478-
var err error
479-
b.ResetTimer()
480-
for i := 0; i < b.N; i++ {
481-
err = binder.bindData(ts, values, "form")
482-
}
483-
assert.NoError(err)
484-
assertBindTestStruct(assert, ts)
485-
}
486-
487518
func BenchmarkBindbindDataWithTags(b *testing.B) {
488519
b.ReportAllocs()
489520
assert := assert.New(b)
@@ -560,8 +591,9 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
560591
// these tests are to document this behaviour and detect further possible regressions when bind implementation is changed
561592

562593
type Opts struct {
563-
ID int `json:"id"`
564-
Node string `json:"node"`
594+
ID int `json:"id" form:"id" query:"id"`
595+
Node string `json:"node" form:"node" query:"node" param:"node"`
596+
Lang string
565597
}
566598

567599
var testCases = []struct {
@@ -727,8 +759,8 @@ func TestDefaultBinder_BindBody(t *testing.T) {
727759
// these tests are to document this behaviour and detect further possible regressions when bind implementation is changed
728760

729761
type Node struct {
730-
ID int `json:"id" xml:"id"`
731-
Node string `json:"node" xml:"node"`
762+
ID int `json:"id" xml:"id" form:"id" query:"id"`
763+
Node string `json:"node" xml:"node" form:"node" query:"node" param:"node"`
732764
}
733765
type Nodes struct {
734766
Nodes []Node `xml:"node" form:"node"`
@@ -824,7 +856,7 @@ func TestDefaultBinder_BindBody(t *testing.T) {
824856
expectError: "code=400, message=Syntax error: line=1, error=XML syntax error on line 1: unexpected EOF, internal=XML syntax error on line 1: unexpected EOF",
825857
},
826858
{
827-
name: "ok, FORM POST bind to struct with: path + query + empty body",
859+
name: "ok, FORM POST bind to struct with: path + query + body",
828860
givenURL: "/api/real_node/endpoint?node=xxx",
829861
givenMethod: http.MethodPost,
830862
givenContentType: MIMEApplicationForm,

0 commit comments

Comments
 (0)