Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

static routing and dynamic routing conflicts problems. #1406

Closed
3 tasks
pxlh007 opened this issue Sep 23, 2019 · 6 comments · Fixed by #1413
Closed
3 tasks

static routing and dynamic routing conflicts problems. #1406

pxlh007 opened this issue Sep 23, 2019 · 6 comments · Fixed by #1413

Comments

@pxlh007
Copy link

pxlh007 commented Sep 23, 2019

Issue Description

If static routing and dynamic routing have only the last or several characters different, there will be routing conflicts.

Checklist

e.GET("/ups/person",hello)
e.GET("/users/new", hello)
e.GET("users/:name", hello)

curl localhost:1323/users/new --- ok
curl localhost:1323/users/new2 --- 404

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

Expected behaviour

curl localhost:1323/users/new --- ok
curl localhost:1323/users/new2 --- ok

Actual behaviour

curl localhost:1323/users/new --- ok
curl localhost:1323/users/new2 --- 404

Steps to reproduce

Working code to debug

package main
  
import (
  "net/http"
  "github.com/labstack/echo"
  "github.com/labstack/echo/middleware"
)

func main() {
  // Echo instance
  e := echo.New()

  // Middleware
  e.Use(middleware.Logger())
  e.Use(middleware.Recover())

  // Routes
  e.GET("/", hello)
  e.GET("/ups/person",hello)
  e.GET("/users/new", hello)
  e.GET("/users/:name", hello)

  // Start server
  e.Logger.Fatal(e.Start(":1323"))
}

// Handler
func hello(c echo.Context) error {
  return c.String(http.StatusOK, "Hello, World!")
}

Version/commit

@nikolay-turpitko
Copy link

nikolay-turpitko commented Sep 26, 2019

I can confirm this. I have the same issue and created some tests to reproduce it. See patch attached. I created test for route and it passes, but test for group doesn't pass (routes are the same). I have no time to properly format those tests with project's requirements to code formatting, etc (to make PR). So, who is interested, may start from applying attached patch.

I printed a tree (image attached), you may see, that /v1/city and /v1/config-dump started from the same latter c somehow cause conflict with /v1/:vert/init and /v1/:vert/redirect, when cars is passed as :vert path param. That is for /v1/cars/init and /v1/cars/redirect. If I replace /v1/config-dump with /v1/dump-config (which I'm going to do as a workaround), than issue disappear and test pass.

tests.zip

image

@denniselite
Copy link

Yes, the same problem. In my case, it happened when after echo -> echo/v4 update:

v1.PUT("/customers/access-token", func(...))
v1.POST("/customers/:id/identities", func(...))

POST /customers/ab68cd44-6503-41a8-a0e5-44ef7d4976f7/identities -> 404

because of intersections of static and dynamic routes in /customers/a... pattern

Even though the methods are different (POST/PUT)

@waterandair
Copy link

May be the similar problem in this example:

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

	app.GET("/user/:id", handler)
	app.PUT("/user/:group_id", handler)

	app.Logger.Fatal(app.Start(":1323"))
}

func handler(ctx echo.Context) error {
	return ctx.String(http.StatusOK, fmt.Sprintf("%v %v %v %v", ctx.Request().Method, ctx.Path(),ctx.ParamNames(), ctx.ParamValues()))
}

Test with curl:

$ curl -X GET localhost:1323/user/1
   GET /user/:group_id [id] [1]                                                                                                                                                                                                                                         $ curl -X PUT localhost:1323/user/1
   PUT /user/:group_id [id] [1]  

Result:
The first request wants GET /user/:id [id] [1] but GET /user/:group_id [id] [1],
The Second request wants PUT /user/:group_id [group_id] [1] but PUT /user/:group_id [id] [1]

@lammel
Copy link
Contributor

lammel commented Oct 22, 2019

The main issue is that the router does not consider the request method when resolving routes.
So one of the paths (could by any of the dynamic routes) is chosen and and a handler is searched for afterwards.

The pull request #1413 fixes detection of the longest matching any route, but does not (at least not intentional) help for overlaying methods of a route.

I'm working on that issue too, as we are facing a similar problem with routing.

@stale
Copy link

stale bot commented Dec 21, 2019

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.

@stale stale bot added the wontfix label Dec 21, 2019
@nikolay-turpitko
Copy link

Please don't close this issue. This is a bug and it must be fixed and verified by human being.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants