Skip to content

Commit

Permalink
Remove maxparam dependence from Context (#2611)
Browse files Browse the repository at this point in the history
  • Loading branch information
aldas committed Mar 21, 2024
1 parent 011acb4 commit d549290
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 38 deletions.
40 changes: 25 additions & 15 deletions context.go
Expand Up @@ -202,15 +202,29 @@ type Context interface {
type context struct {
request *http.Request
response *Response
path string
pnames []string
pvalues []string
query url.Values
handler HandlerFunc
store Map
echo *Echo
logger Logger
lock sync.RWMutex

store Map
lock sync.RWMutex

// following fields are set by Router

// path is route path that Router matched. It is empty string where there is no route match.
// Route registered with RouteNotFound is considered as a match and path therefore is not empty.
path string

// pnames length is tied to param count for the matched route
pnames []string

// Usually echo.Echo is sizing pvalues but there could be user created middlewares that decide to
// overwrite parameter by calling SetParamNames + SetParamValues.
// When echo.Echo allocated that slice it length/capacity is tied to echo.Echo.maxParam value.
//
// It is important that pvalues size is always equal or bigger to pnames length.
pvalues []string
handler HandlerFunc
}

const (
Expand Down Expand Up @@ -330,10 +344,6 @@ func (c *context) SetParamNames(names ...string) {
c.pnames = 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 overridden in a Context#SetParamValues
Expand All @@ -348,11 +358,11 @@ func (c *context) ParamValues() []string {
}

func (c *context) SetParamValues(values ...string) {
// NOTE: Don't just set c.pvalues = values, because it has to have length c.echo.maxParam at all times
// NOTE: Don't just set c.pvalues = values, because it has to have length c.echo.maxParam (or bigger) at all times
// It will brake the Router#Find code
limit := len(values)
if limit > *c.echo.maxParam {
limit = *c.echo.maxParam
if limit > len(c.pvalues) {
c.pvalues = make([]string, limit)
}
for i := 0; i < limit; i++ {
c.pvalues[i] = values[i]
Expand Down Expand Up @@ -643,8 +653,8 @@ func (c *context) Reset(r *http.Request, w http.ResponseWriter) {
c.path = ""
c.pnames = nil
c.logger = nil
// NOTE: Don't reset because it has to have length c.echo.maxParam at all times
for i := 0; i < *c.echo.maxParam; i++ {
// NOTE: Don't reset because it has to have length c.echo.maxParam (or bigger) at all times
for i := 0; i < len(c.pvalues); i++ {
c.pvalues[i] = ""
}
}
60 changes: 37 additions & 23 deletions context_test.go
Expand Up @@ -653,36 +653,50 @@ func TestContextGetAndSetParam(t *testing.T) {
})
}

// Issue #1655
func TestContextSetParamNamesShouldUpdateEchoMaxParam(t *testing.T) {
func TestContextSetParamNamesEchoMaxParam(t *testing.T) {
e := New()
assert.Equal(t, 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(t, 2, *e.maxParam)
assert.EqualValues(t, expectedTwoParams, c.ParamValues())

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

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

c.SetParamValues("A", "B", "C", "D")
assert.Equal(t, 3, *e.maxParam)
// Here D shouldn't be returned
assert.EqualValues(t, expectedABCParams, c.ParamValues())
{
c := e.AcquireContext()
c.SetParamNames("1", "2")
c.SetParamValues(expectedTwoParams...)
assert.Equal(t, 0, *e.maxParam) // has not been changed
assert.EqualValues(t, expectedTwoParams, c.ParamValues())
e.ReleaseContext(c)
}

{
c := e.AcquireContext()
c.SetParamNames("1", "2", "3")
c.SetParamValues(expectedThreeParams...)
assert.Equal(t, 0, *e.maxParam) // has not been changed
assert.EqualValues(t, expectedThreeParams, c.ParamValues())
e.ReleaseContext(c)
}

{ // values is always same size as names length
c := e.NewContext(nil, nil)
c.SetParamValues([]string{"one", "two"}...) // more values than names should be ok
c.SetParamNames("1")
assert.Equal(t, 0, *e.maxParam) // has not been changed
assert.EqualValues(t, expectedOneParam, c.ParamValues())
}

e.GET("/:id", handlerFunc)
assert.Equal(t, 1, *e.maxParam) // has not been changed

{
c := e.NewContext(nil, nil)
c.SetParamValues([]string{"one", "two"}...)
c.SetParamNames("1")
assert.Equal(t, 1, *e.maxParam) // has not been changed
assert.EqualValues(t, expectedOneParam, c.ParamValues())
}
}

func TestContextFormValue(t *testing.T) {
Expand Down

0 comments on commit d549290

Please sign in to comment.