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

Function looping router.go#Find #1493

Closed
art-frela opened this issue Jan 30, 2020 · 5 comments
Closed

Function looping router.go#Find #1493

art-frela opened this issue Jan 30, 2020 · 5 comments

Comments

@art-frela
Copy link

Issue Description

Example code has the one registered route

e := echo.New()
// Routes
e.GET("/assets/:id", assetID)
...
// assetID mock of the real handler
func assetID(c echo.Context) error {
	return c.String(http.StatusOK, "assetID")
}

when request uri is /assets/tree/free application is looping...

if application is simple then another requests are processing, but in complex application it's don't answer to any requests and need kill -9 pid
current request deadlocking...

Checklist

  • [ x] Dependencies installed
  • [ x] No typos
  • [ x] Searched existing issues and docs
    very similar issue#1461

Expected behaviour

404 {"message":"Not Found"}

Actual behaviour

curl  http://localhost:1323/assets/3/e
{"message":"Not Found"}

curl  http://localhost:1323/assets/tree/free
# looping and stop answer...

Steps to reproduce

  • run code below
  • make curl request curl http://localhost:1323/assets/tree/free

Working code to debug

package main

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

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

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

	// Routes
	e.GET("/assets/:id", assetID)
	// Start server
	e.Logger.Fatal(e.Start(":1323"))
}

// assetID mock of the real handler
func assetID(c echo.Context) error {
	return c.String(http.StatusOK, "assetID")
}

Version/commit

4.1.14

@art-frela
Copy link
Author

in 4.1.13 doesn't has this problem

@rdluffy
Copy link

rdluffy commented Jan 30, 2020

I have the same issue, it seems the problem comes with the addition of the following code in router.go (line 417):

else if cn != nil && strings.IndexByte(ns, '/') != 1 { // If slash is present, it means that this is a parameterized route. cn = cn.parent goto Param }

@lammel
Copy link
Contributor

lammel commented Jan 30, 2020

I hope to look into that this week, this is probably an unintentional loop caused by the backtracking in the path for parametrized routes as noted by @rdluffy

Our example should allow to reproduce that in the unit tests and fix the issue. Let's see if that is the case.

@lammel
Copy link
Contributor

lammel commented Jan 31, 2020

I tried to reproduce the issue in the router unit tests, but it is working there.
So back at debugging your example...

lammel added a commit to neotel-at/echo that referenced this issue Feb 1, 2020
lammel added a commit to neotel-at/echo that referenced this issue Feb 8, 2020
@lammel
Copy link
Contributor

lammel commented Feb 8, 2020

The issue was reproducable after all and a test was added for it.
A potential fix is provided with pull request #1501

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

No branches or pull requests

3 participants