Bug: wrong context ParamNames

[mrLSD]

Go vesion: latest
Echo version: latest

Code:

package main

import (
	"fmt"
	"github.com/labstack/echo/v4"
	"github.com/labstack/echo/v4/middleware"
	"github.com/labstack/gommon/log"
	"net/http"
)

func main() {
	e := echo.New()
	e.Use(middleware.Logger())
	e.Use(middleware.Recover())

	e.Logger.SetLevel(log.INFO)

	e.GET("/test", func(c echo.Context) error {
		return c.String(http.StatusOK, "GET /test")
	}).Name = "get-test"
	e.GET("/test/:test", func(c echo.Context) error {
		fmt.Printf("%#v\n", c.ParamValues())
		fmt.Printf("%#v\n", c.ParamNames()) // return []string{"test"}
		return c.String(http.StatusOK, "GET /test/:test=" + c.Param("test"))
	}).Name = "get-list-test"
	e.PUT("/test/:id", func(c echo.Context) error {
		fmt.Printf("%#v\n", c.ParamValues())
		fmt.Printf("%#v\n", c.ParamNames()) // return []string{"test"}
		return c.String(http.StatusOK, "PUT /test/:id=" + c.Param("id"))
	}).Name = "put-test"

	e.Logger.Fatal(e.Start(":3000"))
}

And requests:

$ curl  -X GET   http://localhost:3000/test/test
GET /test/:test=test

$ curl  -X PUT   http://localhost:3000/test/test
PUT /test/:id=

[nilsmagnus]

It seems like the first added path decides what the paramter-name is. If you add another handler to the same path, the parameter-names are re-used from the first handler.

from context.go: (about line 130)

cn.addHandler(method, h)
cn.ppath = ppath
cn.pnames = pnames

To me it seems that the parameter names are bound to the node across all http-mehtods. And this might just be correct behaviour. Do you really want different parameter-names for different http-methods?

To support different parameter-names for different http-methods, the node struct could have a map of method-params.

[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.

[mrLSD]

Any updates?

[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.

[supa-freak]

Any idea when this will be fixed? I feel this is a huge one, basically it makes it impossible to have the same routes for different methods and I end up having to create a different route for every method...

[jerrdasur]

As I understand the problem summary is something like this:

1. Param names and param values are arrays
2. Param names are bound to route, not to route+method
3. Param values are getting by finding their position in ctx.pnames position.
4. The problem can be solved by converting echo.Node.pnames to map or structure like echo.methodHandler and filling ctx.pnames by route+method. So, it'll be not like ctx.pnames = node.pnames, but something like ctx.pnames = node.GetPnames(methodName)

I'll try to provide draft PR in a few days

[mrLSD]

Thanks @jerrdasur

[jerrdasur]

@mrLSD

You can test your case against PR above. As for me, it solves the issue with a minimum of changes.

[mrLSD]

@jerrdasur Thanks for support!
For now I can't test it because can't get to my code base.
Thanks!

[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.

[slaveofcode]

So how the progress about this fix? It seems this PR request still unfinished and closed #1430

[skast96]

This is still a problem for us..

[ddibiasi]

So this issue will be fixed in the v5 release?

[hansbogert]

this is so stupid, and i hit it every couple of months. I have to google for it for 5 mins, and then the "oooh this one" moment comes. This violates the "don't do unexpected things" rule so hard.

[aldas]

It will be addressed in v5, currently this is hard limitation and you need to work around it i.e. do not name params differently for same path.

Naming param differently for same path seems to be situation where you are thinking/using that param conceptionally differently in another method for same path - which seems really odd to me personally. I can not think situation where different names make sense. If it is conceptionally different thing - why do you use same path? Is it not confusing for API users? For example GET /users/:id POST /users/:name feels really really strange thing to do.

[mrLSD]

@aldas right answer: why it doesn't work properly so long time.