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

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

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

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

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.

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.

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.

Navigation Menu

Repository navigation

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

Issue Description

Checklist

Expected behaviour

Actual behaviour

Steps to reproduce

Working code to debug

Version/commit

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Issue Description

Checklist

Expected behaviour

Actual behaviour

Steps to reproduce

Working code to debug

Version/commit

Activity

aldas commented on Jan 15, 2021

iambenkay commented on Jan 15, 2021

alandtsang commented on Jan 15, 2021

alandtsang commented on Jan 16, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions