Skip to content

Fixed Router#Find panic an infinite loop #1661

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions context.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,15 +314,35 @@ func (c *context) ParamNames() []string {

func (c *context) SetParamNames(names ...string) {
c.pnames = names
*c.echo.maxParam = len(names)

l := len(names)
if *c.echo.maxParam < l {
*c.echo.maxParam = l
}

if len(c.pvalues) < l {
// Keeping the old pvalues just for backward compatibility, but it sounds that doesn't make sense to keep them,
// probably those values will be overriden in a Context#SetParamValues
newPvalues := make([]string, l)
copy(newPvalues, c.pvalues)
c.pvalues = newPvalues
}
}

func (c *context) ParamValues() []string {
return c.pvalues[:len(c.pnames)]
}

func (c *context) SetParamValues(values ...string) {
c.pvalues = values
// NOTE: Don't just set c.pvalues = values, because it has to have length c.echo.maxParam at all times
// It will brake the Router#Find code
limit := len(values)
if limit > *c.echo.maxParam {
limit = *c.echo.maxParam
}
for i := 0; i < limit; i++ {
c.pvalues[i] = values[i]
}
}

func (c *context) QueryParam(name string) string {
Expand Down
34 changes: 34 additions & 0 deletions context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,40 @@ func TestContextGetAndSetParam(t *testing.T) {
})
}

// Issue #1655
func TestContextSetParamNamesShouldUpdateEchoMaxParam(t *testing.T) {
assert := testify.New(t)

e := New()
assert.Equal(0, *e.maxParam)

expectedOneParam := []string{"one"}
expectedTwoParams := []string{"one", "two"}
expectedThreeParams := []string{"one", "two", ""}
expectedABCParams := []string{"A", "B", "C"}

c := e.NewContext(nil, nil)
c.SetParamNames("1", "2")
c.SetParamValues(expectedTwoParams...)
assert.Equal(2, *e.maxParam)
assert.EqualValues(expectedTwoParams, c.ParamValues())

c.SetParamNames("1")
assert.Equal(2, *e.maxParam)
// Here for backward compatibility the ParamValues remains as they are
assert.EqualValues(expectedOneParam, c.ParamValues())

c.SetParamNames("1", "2", "3")
assert.Equal(3, *e.maxParam)
// Here for backward compatibility the ParamValues remains as they are, but the len is extended to e.maxParam
assert.EqualValues(expectedThreeParams, c.ParamValues())

c.SetParamValues("A", "B", "C", "D")
assert.Equal(3, *e.maxParam)
// Here D shouldn't be returned
assert.EqualValues(expectedABCParams, c.ParamValues())
}

func TestContextFormValue(t *testing.T) {
f := make(url.Values)
f.Set("name", "Jon Snow")
Expand Down
37 changes: 37 additions & 0 deletions router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1298,6 +1298,43 @@ func TestRouterParam1466(t *testing.T) {
assert.Equal(t, 0, c.response.Status)
}

// Issue #1655
func TestRouterFindNotPanicOrLoopsWhenContextSetParamValuesIsCalledWithLessValuesThanEchoMaxParam(t *testing.T) {
e := New()
r := e.router

v0 := e.Group("/:version")
v0.GET("/admin", func(c Context) error {
c.SetParamNames("version")
c.SetParamValues("v1")
return nil
})

v0.GET("/images/view/:id", handlerHelper("iv", 1))
v0.GET("/images/:id", handlerHelper("i", 1))
v0.GET("/view/*", handlerHelper("v", 1))

//If this API is called before the next two one panic the other loops ( of course without my fix ;) )
c := e.NewContext(nil, nil)
r.Find(http.MethodGet, "/v1/admin", c)
c.Handler()(c)
assert.Equal(t, "v1", c.Param("version"))

//panic
c = e.NewContext(nil, nil)
r.Find(http.MethodGet, "/v1/view/same-data", c)
c.Handler()(c)
assert.Equal(t, "same-data", c.Param("*"))
assert.Equal(t, 1, c.Get("v"))

//looping
c = e.NewContext(nil, nil)
r.Find(http.MethodGet, "/v1/images/view", c)
c.Handler()(c)
assert.Equal(t, "view", c.Param("id"))
assert.Equal(t, 1, c.Get("i"))
}

// Issue #1653
func TestRouterPanicWhenParamNoRootOnlyChildsFailsFind(t *testing.T) {
e := New()
Expand Down