The parameters in path may be overwritten by the parameters with the same name in query

[alandtsang]

Issue Description

Hi guys, I encountered such a problem while using echo. The parameters in path may be overwritten by the parameters with the same name in query.

Checklist

• Dependencies installed
• No typos
• Searched existing issues and docs

Expected behaviour

Actual behaviour

Steps to reproduce

For example, the following example:

First request:

 curl -X POST http://127.0.0.1:1323/pets/1/names -d '{"name":"Bear"}' -H "Content-Type: application/json"

output is {PetID:1 Name:Bear}, and this is as expected.

If now an attacker passes the path parameter with the same name query parameter pet_id:

curl -X POST http://127.0.0.1:1323/pets/1/names -d '{"name":"Bear", "pet_id":2}' -H "Content-Type: application/json"

The output at this time is not what I expected, pet_id is overwritten:

{PetID:2 Name:Bear}

Is the above situation a known normal situation?

Working code to debug

package main

import (
	"fmt"
	"net/http"

	"github.com/go-playground/validator/v10"
	"github.com/labstack/echo/v4"
	"github.com/labstack/echo/v4/middleware"
)

type setPetNameParams struct {
	PetID int64  `json:"pet_id" param:"pet_id" validate:"required"`
	Name  string `json:"name" query:"name" validate:"required"`
}

type Validator struct {
	validator *validator.Validate
}

func NewValidator() *Validator {
	return &Validator{validator: validator.New()}
}

func (v *Validator) Validate(i interface{}) error {
	return v.validator.Struct(i)
}

func main() {
	e := echo.New()
	e.Validator = NewValidator()

	e.Use(middleware.Logger())
	e.Use(middleware.Recover())

	e.POST("/pets/:pet_id/names", SetPetName)
	e.Logger.Fatal(e.Start(":1323"))
}

func SetPetName(c echo.Context) error {
	var (
		params setPetNameParams
		err    error
	)

	if err = c.Bind(&params); err != nil {
		return c.JSON(http.StatusBadRequest, err.Error())
	}

	if err = c.Validate(&params); err != nil {
		return c.JSON(http.StatusBadRequest, err.Error())
	}

	fmt.Printf("%+v\n", params)
	return c.String(http.StatusOK, params.Name)
}

Version/commit

go version go1.14.11 darwin/amd64

github.com/go-playground/validator/v10 v10.4.1
github.com/labstack/echo/v4 v4.1.17

[aldas]

This is same as #1670 and addressed in multiple ways in different PRs (already in master branch - waiting to be released)

• separate methods to bind only: query params, route params, request body #1681
• Bind query params only for HTTP GET/DELETE methods #1727
• issue #1729 - Binding query/path params and form fields to struct only works for explicit tags #1734
• Fluent Binder for Query/Path/Form binding (#1717) #1736

In current version what you can do - do not mix json binding with param/query binding.

For body/json

	if err := json.NewDecoder(c.Request().Body).Decode(&payload); err != nil {
		return err
	}

For path

	petID, err := strconv.ParseInt(c.Param("pet_id"), 10, 64)
	if err != nil {
		return err
	}

For query

	petID, err := strconv.ParseInt(c.QueryParam("pet_id"), 10, 64)
	if err != nil {
		return err
	}

it is longer but avoids mixing up where data is taken

in next release it will be much easier to handle this stuff

[iambenkay]

This is due to Context.Bind automatically trying to bind the request body, query params and path params into one struct so it would override it if fields from two sources have the same name.

This has been addressed in #1681 where explicit binding is done instead and although hasn't been officially released can be accessed with this version suffix v4.1.18-0.20210107234338-9b0e63046b13

I'd advise to not change to this unreleased version if it's a production app except you really know what you're doing.

I currently use it and it tackles the issue for me.

[alandtsang]

This is same as #1670 and addressed in multiple ways in different PRs (already in master branch - waiting to be released)

• separate methods to bind only: query params, route params, request body #1681
• Bind query params only for HTTP GET/DELETE methods #1727
• issue #1729 - Binding query/path params and form fields to struct only works for explicit tags #1734
• Fluent Binder for Query/Path/Form binding (#1717) #1736

In current version what you can do - do not mix json binding with param/query binding.

Thanks for your reply, I understand, thanks again.

[alandtsang]

This is due to Context.Bind automatically trying to bind the request body, query params and path params into one struct so it would override it if fields from two sources have the same name.

This has been addressed in #1681 where explicit binding is done instead and although hasn't been officially released can be accessed with this version suffix v4.1.18-0.20210107234338-9b0e63046b13

I'd advise to not change to this unreleased version if it's a production app except you really know what you're doing.

I currently use it and it tackles the issue for me.

Thanks for your reply, I will close this issue now.