Skip to content

Commit

Permalink
Fix Bind() when target is array/slice and path/query params complain …
Browse files Browse the repository at this point in the history
…target not being struct (#1835)

For path/query params binding we do not try (silently return) to bind when target is not struct.
Recreates PR #1574 and fixes #1565
  • Loading branch information
aldas committed Apr 6, 2021
1 parent dec96f0 commit 67f6346
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 17 deletions.
4 changes: 4 additions & 0 deletions bind.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,10 @@ func (b *DefaultBinder) bindData(destination interface{}, data map[string][]stri

// !struct
if typ.Kind() != reflect.Struct {
if tag == "param" || tag == "query" {
// incompatible type, data is probably to be found in the body
return nil
}
return errors.New("binding element must be a struct")
}

Expand Down
70 changes: 53 additions & 17 deletions bind_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"mime/multipart"
"net/http"
"net/http/httptest"
"net/url"
"reflect"
"strconv"
"strings"
Expand Down Expand Up @@ -187,19 +188,26 @@ func TestToMultipleFields(t *testing.T) {

func TestBindJSON(t *testing.T) {
assert := assert.New(t)
testBindOkay(assert, strings.NewReader(userJSON), MIMEApplicationJSON)
testBindOkay(assert, strings.NewReader(userJSON), nil, MIMEApplicationJSON)
testBindOkay(assert, strings.NewReader(userJSON), dummyQuery, MIMEApplicationJSON)
testBindArrayOkay(assert, strings.NewReader(usersJSON), nil, MIMEApplicationJSON)
testBindArrayOkay(assert, strings.NewReader(usersJSON), dummyQuery, MIMEApplicationJSON)
testBindError(assert, strings.NewReader(invalidContent), MIMEApplicationJSON, &json.SyntaxError{})
testBindError(assert, strings.NewReader(userJSONInvalidType), MIMEApplicationJSON, &json.UnmarshalTypeError{})
}

func TestBindXML(t *testing.T) {
assert := assert.New(t)

testBindOkay(assert, strings.NewReader(userXML), MIMEApplicationXML)
testBindOkay(assert, strings.NewReader(userXML), nil, MIMEApplicationXML)
testBindOkay(assert, strings.NewReader(userXML), dummyQuery, MIMEApplicationXML)
testBindArrayOkay(assert, strings.NewReader(userXML), nil, MIMEApplicationXML)
testBindArrayOkay(assert, strings.NewReader(userXML), dummyQuery, MIMEApplicationXML)
testBindError(assert, strings.NewReader(invalidContent), MIMEApplicationXML, errors.New(""))
testBindError(assert, strings.NewReader(userXMLConvertNumberError), MIMEApplicationXML, &strconv.NumError{})
testBindError(assert, strings.NewReader(userXMLUnsupportedTypeError), MIMEApplicationXML, &xml.SyntaxError{})
testBindOkay(assert, strings.NewReader(userXML), MIMETextXML)
testBindOkay(assert, strings.NewReader(userXML), nil, MIMETextXML)
testBindOkay(assert, strings.NewReader(userXML), dummyQuery, MIMETextXML)
testBindError(assert, strings.NewReader(invalidContent), MIMETextXML, errors.New(""))
testBindError(assert, strings.NewReader(userXMLConvertNumberError), MIMETextXML, &strconv.NumError{})
testBindError(assert, strings.NewReader(userXMLUnsupportedTypeError), MIMETextXML, &xml.SyntaxError{})
Expand All @@ -208,7 +216,8 @@ func TestBindXML(t *testing.T) {
func TestBindForm(t *testing.T) {
assert := assert.New(t)

testBindOkay(assert, strings.NewReader(userForm), MIMEApplicationForm)
testBindOkay(assert, strings.NewReader(userForm), nil, MIMEApplicationForm)
testBindOkay(assert, strings.NewReader(userForm), dummyQuery, MIMEApplicationForm)
e := New()
req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(userForm))
rec := httptest.NewRecorder()
Expand Down Expand Up @@ -336,14 +345,16 @@ func TestBindUnmarshalTextPtr(t *testing.T) {
}

func TestBindMultipartForm(t *testing.T) {
body := new(bytes.Buffer)
mw := multipart.NewWriter(body)
bodyBuffer := new(bytes.Buffer)
mw := multipart.NewWriter(bodyBuffer)
mw.WriteField("id", "1")
mw.WriteField("name", "Jon Snow")
mw.Close()
body := bodyBuffer.Bytes()

assert := assert.New(t)
testBindOkay(assert, body, mw.FormDataContentType())
testBindOkay(assert, bytes.NewReader(body), nil, mw.FormDataContentType())
testBindOkay(assert, bytes.NewReader(body), dummyQuery, mw.FormDataContentType())
}

func TestBindUnsupportedMediaType(t *testing.T) {
Expand Down Expand Up @@ -547,9 +558,13 @@ func assertBindTestStruct(a *assert.Assertions, ts *bindTestStruct) {
a.Equal("", ts.GetCantSet())
}

func testBindOkay(assert *assert.Assertions, r io.Reader, ctype string) {
func testBindOkay(assert *assert.Assertions, r io.Reader, query url.Values, ctype string) {
e := New()
req := httptest.NewRequest(http.MethodPost, "/", r)
path := "/"
if len(query) > 0 {
path += "?" + query.Encode()
}
req := httptest.NewRequest(http.MethodPost, path, r)
rec := httptest.NewRecorder()
c := e.NewContext(req, rec)
req.Header.Set(HeaderContentType, ctype)
Expand All @@ -561,6 +576,25 @@ func testBindOkay(assert *assert.Assertions, r io.Reader, ctype string) {
}
}

func testBindArrayOkay(assert *assert.Assertions, r io.Reader, query url.Values, ctype string) {
e := New()
path := "/"
if len(query) > 0 {
path += "?" + query.Encode()
}
req := httptest.NewRequest(http.MethodPost, path, r)
rec := httptest.NewRecorder()
c := e.NewContext(req, rec)
req.Header.Set(HeaderContentType, ctype)
u := []user{}
err := c.Bind(&u)
if assert.NoError(err) {
assert.Equal(1, len(u))
assert.Equal(1, u[0].ID)
assert.Equal("Jon Snow", u[0].Name)
}
}

func testBindError(assert *assert.Assertions, r io.Reader, ctype string, expectedInternal error) {
e := New()
req := httptest.NewRequest(http.MethodPost, "/", r)
Expand Down Expand Up @@ -679,15 +713,16 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
expect: &Opts{ID: 0, Node: "xxx"}, // query binding has already modified bind target
expectError: "code=400, message=Unmarshal type error: expected=echo.Opts, got=array, field=, offset=1, internal=json: cannot unmarshal array into Go value of type echo.Opts",
},
{ // binding query params interferes with body. b.BindBody() should be used to bind only body to slice
name: "nok, GET query params bind failure - trying to bind json array to slice",
{ // query param is ignored as we do not know where exactly to bind it in slice
name: "ok, GET bind to struct slice, ignore query param",
givenMethod: http.MethodGet,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`[{"id": 1}]`),
whenNoPathParams: true,
whenBindTarget: &[]Opts{},
expect: &[]Opts{},
expectError: "code=400, message=binding element must be a struct, internal=binding element must be a struct",
expect: &[]Opts{
{ID: 1, Node: ""},
},
},
{ // binding query params interferes with body. b.BindBody() should be used to bind only body to slice
name: "ok, POST binding to slice should not be affected query params types",
Expand All @@ -699,14 +734,15 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
expect: &[]Opts{{ID: 1}},
expectError: "",
},
{ // binding path params interferes with body. b.BindBody() should be used to bind only body to slice
name: "nok, GET path params bind failure - trying to bind json array to slice",
{ // path param is ignored as we do not know where exactly to bind it in slice
name: "ok, GET bind to struct slice, ignore path param",
givenMethod: http.MethodGet,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`[{"id": 1}]`),
whenBindTarget: &[]Opts{},
expect: &[]Opts{},
expectError: "code=400, message=binding element must be a struct, internal=binding element must be a struct",
expect: &[]Opts{
{ID: 1, Node: ""},
},
},
{
name: "ok, GET body bind json array to slice",
Expand Down
4 changes: 4 additions & 0 deletions echo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"net"
"net/http"
"net/http/httptest"
"net/url"
"os"
"reflect"
"strings"
Expand All @@ -30,6 +31,7 @@ type (

const (
userJSON = `{"id":1,"name":"Jon Snow"}`
usersJSON = `[{"id":1,"name":"Jon Snow"}]`
userXML = `<user><id>1</id><name>Jon Snow</name></user>`
userForm = `id=1&name=Jon Snow`
invalidContent = "invalid content"
Expand All @@ -48,6 +50,8 @@ const userXMLPretty = `<user>
<name>Jon Snow</name>
</user>`

var dummyQuery = url.Values{"dummy": []string{"useless"}}

func TestEcho(t *testing.T) {
e := New()
req := httptest.NewRequest(http.MethodGet, "/", nil)
Expand Down

0 comments on commit 67f6346

Please sign in to comment.