Skip to content

issue #1729 - Binding query/path params and form fields to struct only works for explicit tags #1734

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 5, 2021
Merged

issue #1729 - Binding query/path params and form fields to struct only works for explicit tags #1734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7efda20
Binding query/path params and form fields to struct only works for fi…
aldas Dec 20, 2020
254d08c
remove unnecessary benchmark after change because it is not valid tes…
aldas Jan 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions bind.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,13 @@ func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) {
return b.BindBody(c, i)
}

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

// Map
if typ.Kind() == reflect.Map {
Expand All @@ -146,14 +147,15 @@ 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 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).
// structs that implement BindUnmarshaler are binded only when they have explicit tag
if _, ok := structField.Addr().Interface().(BindUnmarshaler); !ok && structFieldKind == reflect.Struct {
if err := b.bindData(structField.Addr().Interface(), data, tag); err != nil {
return err
}
continue
}
// does not have explicit tag and is not an ordinary struct - so move to next field
continue
}

inputValue, exists := data[inputFieldName]
Expand Down
88 changes: 60 additions & 28 deletions bind_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,31 @@ var values = map[string][]string{
"ST": {"bar"},
}

func TestToMultipleFields(t *testing.T) {
e := New()
req := httptest.NewRequest(http.MethodGet, "/?id=1&ID=2", nil)
rec := httptest.NewRecorder()
c := e.NewContext(req, rec)

type Root struct {
ID int64 `query:"id"`
Child2 struct {
ID int64
}
Child1 struct {
ID int64 `query:"id"`
}
}

u := new(Root)
err := c.Bind(u)
if assert.NoError(t, err) {
assert.Equal(t, int64(1), u.ID) // perfectly reasonable
assert.Equal(t, int64(1), u.Child1.ID) // untagged struct containing tagged field gets filled (by tag)
assert.Equal(t, int64(0), u.Child2.ID) // untagged struct containing untagged field should not be bind
}
}

func TestBindJSON(t *testing.T) {
assert := assert.New(t)
testBindOkay(assert, strings.NewReader(userJSON), MIMEApplicationJSON)
Expand Down Expand Up @@ -238,10 +263,13 @@ func TestBindUnmarshalParam(t *testing.T) {
rec := httptest.NewRecorder()
c := e.NewContext(req, rec)
result := struct {
T Timestamp `query:"ts"`
TA []Timestamp `query:"ta"`
SA StringArray `query:"sa"`
ST Struct
T Timestamp `query:"ts"`
TA []Timestamp `query:"ta"`
SA StringArray `query:"sa"`
ST Struct
StWithTag struct {
Foo string `query:"st"`
}
}{}
err := c.Bind(&result)
ts := Timestamp(time.Date(2016, 12, 6, 19, 9, 5, 0, time.UTC))
Expand All @@ -252,7 +280,8 @@ func TestBindUnmarshalParam(t *testing.T) {
assert.Equal(ts, result.T)
assert.Equal(StringArray([]string{"one", "two", "three"}), result.SA)
assert.Equal([]Timestamp{ts, ts}, result.TA)
assert.Equal(Struct{"baz"}, result.ST)
assert.Equal(Struct{""}, result.ST) // child struct does not have a field with matching tag
assert.Equal("baz", result.StWithTag.Foo) // child struct has field with matching tag
}
}

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

Expand Down Expand Up @@ -323,11 +352,27 @@ func TestBindUnsupportedMediaType(t *testing.T) {
}

func TestBindbindData(t *testing.T) {
assert := assert.New(t)
a := assert.New(t)
ts := new(bindTestStruct)
b := new(DefaultBinder)
b.bindData(ts, values, "form")
assertBindTestStruct(assert, ts)
err := b.bindData(ts, values, "form")
a.NoError(err)

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.cantSet)
}

func TestBindParam(t *testing.T) {
Expand Down Expand Up @@ -470,20 +515,6 @@ func TestBindSetFields(t *testing.T) {
}
}

func BenchmarkBindbindData(b *testing.B) {
b.ReportAllocs()
assert := assert.New(b)
ts := new(bindTestStruct)
binder := new(DefaultBinder)
var err error
b.ResetTimer()
for i := 0; i < b.N; i++ {
err = binder.bindData(ts, values, "form")
}
assert.NoError(err)
assertBindTestStruct(assert, ts)
}

func BenchmarkBindbindDataWithTags(b *testing.B) {
b.ReportAllocs()
assert := assert.New(b)
Expand Down Expand Up @@ -560,8 +591,9 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
// these tests are to document this behaviour and detect further possible regressions when bind implementation is changed

type Opts struct {
ID int `json:"id"`
Node string `json:"node"`
ID int `json:"id" form:"id" query:"id"`
Node string `json:"node" form:"node" query:"node" param:"node"`
Lang string
}

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

type Node struct {
ID int `json:"id" xml:"id"`
Node string `json:"node" xml:"node"`
ID int `json:"id" xml:"id" form:"id" query:"id"`
Node string `json:"node" xml:"node" form:"node" query:"node" param:"node"`
}
type Nodes struct {
Nodes []Node `xml:"node" form:"node"`
Expand Down Expand Up @@ -824,7 +856,7 @@ func TestDefaultBinder_BindBody(t *testing.T) {
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",
},
{
name: "ok, FORM POST bind to struct with: path + query + empty body",
name: "ok, FORM POST bind to struct with: path + query + body",
givenURL: "/api/real_node/endpoint?node=xxx",
givenMethod: http.MethodPost,
givenContentType: MIMEApplicationForm,
Expand Down