Binding confusion when params and request body have the same named field

[waterandair]

Issue Description

in this PR #1165 added feature to map url params to a struct with the default binder, now if the url params and request body have same named field but different variable type, the default binder will throw an type parse error.

Checklist

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

Expected behaviour

Maybe binder should bind url params just when struct really contains params tag.

Actual behaviour

If struct have a field named id with json tag, and the url have a param named id, the default binder will throw an error

Steps to reproduce

1. run this code with echo v4

func main() {
	app := echo.New()

	app.POST("/user/:id", handler)
	app.Logger.Fatal(app.Start(":1323"))
}

type Body struct {
	ID   uint64 `json:"id"`
	Type string `json:"type"`
}

func handler(ctx echo.Context) error {
	id := ctx.Param("id")
	doSomething(id)

	body := new(Body)
	if err := ctx.Bind(body); err != nil {
		return ctx.JSON(http.StatusInternalServerError, err)
	}

	return ctx.JSON(http.StatusOK, body)
}

func doSomething(id string) {
}

2. use curl make a request

$ curl -H "Content-type: application/json" -X POST -d '{"id":1,"type":"any"}' http://localhost:1323/user/a
  {"message":"strconv.ParseUint: parsing \"a\": invalid syntax"}

[junedev]

We were just about to upgrade to version 4. Out of coincidence one of our test cases for the application caught this bug. It was introduced in version 4.1.7. We will try to downgrade below that for now.

[PrakharSrivastav]

Are there any workarounds for this.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[junedev]

I am very concerned that the bot marked this as "wontfix" because it is quite an obvious and serious bug that is quite hard to debug in production once you run into it. Happy to help with fixing this but I would need some guidance for that.

As for workarounds, the only thing I can think of is either downgrading like we did or making sure there is no name collision between the path parameters and the JSON fields which is probably not very practical.

[patrikeh]

IMO the decision to merge #1165, not even behind a custom setting or anything is very questionable. It basically breaks about half of our paths in some obscure way due to path parameters randomly matching with struct fields. That's a pretty big assumption that everyone would want query and path parameters inserted into structs.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[junedev]

Addressed in #1681