go/echo
go/echo
1
0
Fork 0
mirror of https://github.com/labstack/echo.git synced 2025-01-12 01:22:21 +02:00
Code Issues Releases Activity

Merge pull request #1727 from aldas/bind_query_when_get_delete

Browse Source 
Bind query params only for HTTP GET/DELETE methods
This commit is contained in:
Roland Lammel 2020-12-26 00:51:50 +01:00 committed by GitHub
commit b065180250
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 97 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
Makefile
View File

@ -1,3 +1,27 @@
PKG := "github.com/labstack/echo"
PKG_LIST := $(shell go list ${PKG}/...)
tag:
@git tag `grep -P '^\tversion = ' echo.go|cut -f2 -d'"'`
@git tag|grep -v ^v
.DEFAULT_GOAL := check
check: lint vet race ## Check project
init:
@go get -u golang.org/x/lint/golint
lint: ## Lint the files
@golint -set_exit_status ${PKG_LIST}
vet: ## Vet the files
@go vet ${PKG_LIST}
test: ## Run tests
@go test -short ${PKG_LIST}
race: ## Run tests with data race detector
@go test -race ${PKG_LIST}
help: ## Display this help screen
@grep -h -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'

12
bind.go
View File

@ -98,12 +98,20 @@ func (b *DefaultBinder) BindBody(c Context, i interface{}) (err error) {
}
// Bind implements the `Binder#Bind` function.
// Binding is done in following order: 1) path params; 2) query params; 3) request body. Each step COULD override previous
// step binded values. For single source binding use their own methods BindBody, BindQueryParams, BindPathParams.
func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) {
if err := b.BindPathParams(c, i); err != nil {
return err
}
if err = b.BindQueryParams(c, i); err != nil {
return err
// Issue #1670 - Query params are binded only for GET/DELETE and NOT for usual request with body (POST/PUT/PATCH)
// Reasoning here is that parameters in query and bind destination struct could have UNEXPECTED matches and results due that.
// i.e. is `&id=1&lang=en` from URL same as `{"id":100,"lang":"de"}` request body and which one should have priority when binding.
// This HTTP method check restores pre v4.1.11 behavior and avoids different problems when query is mixed with body
if c.Request().Method == http.MethodGet || c.Request().Method == http.MethodDelete {
if err = b.BindQueryParams(c, i); err != nil {
return err
}
}
return b.BindBody(c, i)
}

80
bind_test.go
View File

@ -559,7 +559,7 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
// binding is done in steps and one source could overwrite previous source binded data
// these tests are to document this behaviour and detect further possible regressions when bind implementation is changed
type Node struct {
type Opts struct {
ID int `json:"id"`
Node string `json:"node"`
}
@ -575,41 +575,77 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
expectError string
}{
{
name: "ok, POST bind to struct with: path param + query param + empty body",
name: "ok, POST bind to struct with: path param + query param + body",
givenMethod: http.MethodPost,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`{"id": 1}`),
expect: &Node{ID: 1, Node: "xxx"}, // in current implementation query params has higher priority than path params
expect: &Opts{ID: 1, Node: "node_from_path"}, // query params are not used, node is filled from path
},
{
name: "ok, POST bind to struct with: path param + empty body",
name: "ok, PUT bind to struct with: path param + query param + body",
givenMethod: http.MethodPut,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`{"id": 1}`),
expect: &Opts{ID: 1, Node: "node_from_path"}, // query params are not used
},
{
name: "ok, GET bind to struct with: path param + query param + body",
givenMethod: http.MethodGet,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`{"id": 1}`),
expect: &Opts{ID: 1, Node: "xxx"}, // query overwrites previous path value
},
{
name: "ok, GET bind to struct with: path param + query param + body",
givenMethod: http.MethodGet,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`),
expect: &Opts{ID: 1, Node: "zzz"}, // body is binded last and overwrites previous (path,query) values
},
{
name: "ok, DELETE bind to struct with: path param + query param + body",
givenMethod: http.MethodDelete,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`),
expect: &Opts{ID: 1, Node: "zzz"}, // for DELETE body is binded after query params
},
{
name: "ok, POST bind to struct with: path param + body",
givenMethod: http.MethodPost,
givenURL: "/api/real_node/endpoint",
givenContent: strings.NewReader(`{"id": 1}`),
expect: &Node{ID: 1, Node: "real_node"},
expect: &Opts{ID: 1, Node: "node_from_path"},
},
{
name: "ok, POST bind to struct with path + query + body = body has priority",
givenMethod: http.MethodPost,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`),
expect: &Node{ID: 1, Node: "zzz"}, // field value from content has higher priority
expect: &Opts{ID: 1, Node: "zzz"}, // field value from content has higher priority
},
{
name: "nok, POST body bind failure",
givenMethod: http.MethodPost,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`{`),
expect: &Node{ID: 0, Node: "xxx"}, // query binding has already modified bind target
expect: &Opts{ID: 0, Node: "node_from_path"}, // query binding has already modified bind target
expectError: "code=400, message=unexpected EOF, internal=unexpected EOF",
},
{
name: "nok, GET with body bind failure when types are not convertible",
givenMethod: http.MethodGet,
givenURL: "/api/real_node/endpoint?id=nope",
givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`),
expect: &Opts{ID: 0, Node: "node_from_path"}, // path params binding has already modified bind target
expectError: "code=400, message=strconv.ParseInt: parsing \"nope\": invalid syntax, internal=strconv.ParseInt: parsing \"nope\": invalid syntax",
},
{
name: "nok, GET body bind failure - trying to bind json array to struct",
givenMethod: http.MethodGet,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`[{"id": 1}]`),
expect: &Node{ID: 0, Node: "xxx"}, // query binding has already modified bind target
expectError: "code=400, message=Unmarshal type error: expected=echo.Node, got=array, field=, offset=1, internal=json: cannot unmarshal array into Go value of type echo.Node",
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",
@ -617,17 +653,27 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`[{"id": 1}]`),
whenNoPathParams: true,
whenBindTarget: &[]Node{},
expect: &[]Node{},
whenBindTarget: &[]Opts{},
expect: &[]Opts{},
expectError: "code=400, message=binding element must be a struct, internal=binding element must be a struct",
},
{ // 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",
givenMethod: http.MethodPost,
givenURL: "/api/real_node/endpoint?id=nope&node=xxx",
givenContent: strings.NewReader(`[{"id": 1}]`),
whenNoPathParams: true,
whenBindTarget: &[]Opts{},
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",
givenMethod: http.MethodGet,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`[{"id": 1}]`),
whenBindTarget: &[]Node{},
expect: &[]Node{},
whenBindTarget: &[]Opts{},
expect: &[]Opts{},
expectError: "code=400, message=binding element must be a struct, internal=binding element must be a struct",
},
{
@ -636,8 +682,8 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
givenURL: "/api/real_node/endpoint",
givenContent: strings.NewReader(`[{"id": 1}]`),
whenNoPathParams: true,
whenBindTarget: &[]Node{},
expect: &[]Node{{ID: 1, Node: ""}},
whenBindTarget: &[]Opts{},
expect: &[]Opts{{ID: 1, Node: ""}},
expectError: "",
},
}
@ -653,14 +699,14 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
if !tc.whenNoPathParams {
c.SetParamNames("node")
c.SetParamValues("real_node")
c.SetParamValues("node_from_path")
}
var bindTarget interface{}
if tc.whenBindTarget != nil {
bindTarget = tc.whenBindTarget
} else {
bindTarget = &Node{}
bindTarget = &Opts{}
}
b := new(DefaultBinder)