echo icon indicating copy to clipboard operation
echo copied to clipboard

Routing issue with groups

Open andrei-dascalu opened this issue 5 years ago • 6 comments

Issue Description

I have a group with a certain prefix like “/api/root”. Now, I’m adding paths to this group.

myGroup.GET("/", myHandler1) myGroup.GET("/path", myHandler2)

Checklist

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

Expected behaviour

myHandler one handles both ”/api/root/" and “/api/root”

Actual behaviour

myHandler one will only respond to ”/api/root/", but not “/api/root”

This is more of a question (I also asked it in the forums roughly 2 weeks ago), as I'm not sure what's the intended behavior here.

         router := echo.New()
	myGroup := router.Group("/api/root")

	myGroup.GET("/pages", api.FindPages)
	myGroup.GET("/", api.GetIndex)

At the moment I need to add

     myGroup.GET("", api.GetIndex)

To make sure /api/root/ and /api/root behave the same way.

andrei-dascalu avatar May 10 '20 18:05 andrei-dascalu

There are middleware which add and remove trailing slash which will help to achieve your intended behaviour doc So something like

router.Pre(middleware.RemoveTrailingSlash())

should help, and you just handle one without trailing-slash or vice versa.

But personally I'd really like it to be the way you intended by default without injecting extra middleware

andy-shi88 avatar Jun 19 '20 13:06 andy-shi88

For SEO reasons, it's normally good for only one path to be the canonical one, so Gin handles this by optionally (On by default) redirecting in such a case to the other path with/without a trailing slash. Echo seems to not have such a feature, those middlewares are not conditional on whether such a route actually exists or not.

segevfiner avatar Jul 14 '20 17:07 segevfiner

I'm not sure that SEO is a good reason here, as it only depends on your linking for what pathes are actually used. So this is usually only one or the other.

But maybe a RedirectTrailingSlash middleware could be added. I'll leave that for discussion here.

lammel avatar Sep 02 '20 17:09 lammel

You are probably right about SEO. Still, it is a useful behavior sometimes, which is why some frameworks include it.

segevfiner avatar Sep 02 '20 17:09 segevfiner

Agreed. We also have #1533 which also relates to this and would benefit from handling the trailing slash.

When adding the routes to the routing tree we might add an automatic redirect. This could be based on the whether a slash is appended to the route or not upon adding.

e.Static("/static", "static") // Add static route for /static/ and add auto-redirect for /static to /static/
e.Static("/static/", "static") // Only add static route for /static/, do not consider /static in routing tree

@vishr What's your opinion on that? Some magic or adding a middleware to handle that (which would not help the default use-case

lammel avatar Sep 02 '20 18:09 lammel

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[bot] avatar Nov 07 '20 22:11 stale[bot]