echo icon indicating copy to clipboard operation
echo copied to clipboard

Wrong parameter behaviour with specific path

Open ghost opened this issue 4 years ago • 3 comments

Issue Description

Route router.DELETE takes parameters from router.GET instead of his own parameters.

Checklist

  • [x] Dependencies installed
  • [X] No typos
  • [X] Searched existing issues and docs

Expected behaviour

DeleteTranslation handler receives echo.Context with id parameter without lang parameter

Actual behaviour

DeleteTranslation handler receives lang parameter instead of id parameter

Steps to reproduce

  1. Run server
  2. Request DELETE http://localhost:3333/translation/123

Working code to debug

package main

import (
	"github.com/labstack/echo/v4"
	"net/http"
)

var router = echo.New()

type resp struct {
	Name string
	Lang string
	Id string
}

func GetTranslation (c echo.Context) error {
	return c.JSON(http.StatusOK, resp{Name: "Get", Id: c.Param("id"), Lang: c.Param("lang")})
}

func DeleteTranslation (c echo.Context) error {
	return c.JSON(http.StatusOK, resp{Name: "Delete", Id: c.Param("id"), Lang: c.Param("lang")})
}

func main()  {
	router.GET("/translation/:lang", GetTranslation)
	router.DELETE("/translation/:id", DeleteTranslation)

	router.Start(":8000")
}

Version/commit

go 1.15 github.com/labstack/echo/v4 v4.1.17

ghost avatar Dec 31 '20 17:12 ghost

Seems to be same as #479

Problem is that routes with same path (with path variables at same location) but with different method are grouped by path. Parameter name comes from firstly declared route that created routing node. Subsequent route declarations just add their method on that node but dont register their parameter names for the same place. So route for path /translation/: always sets parameter lang value.

https://github.com/labstack/echo/blob/6119aecb16b39eae121ef620d665ca0ff00b21ba/router.go#L204

so probably when method handler is added to Node we could add pNames also for that method (pull pnames down from node to near method handlers https://github.com/labstack/echo/blob/6119aecb16b39eae121ef620d665ca0ff00b21ba/router.go#L32

methodHandler struct {
...
  delete  struct {
    handler HandlerFunc
    pnames []string
  }
...
...

then every method on same router Node can have different name for parameter name at same place when match is made and we populate context with pnames/pvalues. Anyway @lammel and @pafuent should know that subject better.

aldas avatar Jan 01 '21 07:01 aldas

This is currently not supported. You have to use consistent parameter naming for routes. There were similar requests in the past though, so I guess it is time to tackle this issue.

The solution suggested by @aldas is one of the options, also a refactoring of the router to decouple routes and method handlers is an option.

lammel avatar Jan 01 '21 17:01 lammel

Duplicate of #1726

lammel avatar Jan 01 '21 17:01 lammel