Echo: 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[bot]]

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[bot]]

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[bot]]

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 https://github.com/labstack/echo/pull/1430