SetParamValues function throwing index out of range [0] with length 0 error

[yenskillah]

Issue Description

Hi,

I' currently encountering an issue when setting parameter values via SetParamValues function. This is frequently used by my test cases, hence, tests are all failing.

Checklist

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

Expected behaviour

Should be able to set parameter values via SetParamValues function

Actual behaviour

SetParamValues function throws runtime error: index out of range [0] with length 0

Steps to reproduce

1. Create a folder inside your go source path.
2. Copy the codes below into files handler.go and handler_test.go and store them inside that folder.
3. cd yourfolder
4. go test -run ^(TestGetUser)$ -v

It will show this test logs afterwards:

=== RUN TestGetUser
--- FAIL: TestGetUser (0.00s)
panic: runtime error: index out of range [0] with length 0 [recovered]
panic: runtime error: index out of range [0] with length 0

goroutine 20 [running]:
testing.tRunner.func1(0xc00010e100)
/usr/local/Cellar/go/1.13.3/libexec/src/testing/testing.go:874 +0x3a3
panic(0x1387d40, 0xc0000c4380)
/usr/local/Cellar/go/1.13.3/libexec/src/runtime/panic.go:679 +0x1b2
github.com/labstack/echo.(*context).SetParamValues(0xc0000bd180, 0xc00008d2b0, 0x1, 0x1)
/Users/sonnysidramos/go/src/github.com/labstack/echo/context.go:317 +0x93
bitbucket.org/pulseid/echotest.TestGetUser(0xc00010e100)
/Users/sonnysidramos/go/src/bitbucket.org/pulseid/echotest/handler_test.go:27 +0x307
testing.tRunner(0xc00010e100, 0x13ce2a0)
/usr/local/Cellar/go/1.13.3/libexec/src/testing/testing.go:909 +0xc9
created by testing.(*T).Run
/usr/local/Cellar/go/1.13.3/libexec/src/testing/testing.go:960 +0x350
exit status 2

Working code to debug

handler.go

package handler

import (
	"net/http"

	"github.com/labstack/echo"
)

type (
	User struct {
		Name  string `json:"name" form:"name"`
		Email string `json:"email" form:"email"`
	}
	handler struct {
		db map[string]*User
	}
)

func (h *handler) getUser(c echo.Context) error {
	email := c.Param("email")
	user := h.db[email]
	if user == nil {
		return echo.NewHTTPError(http.StatusNotFound, "user not found")
	}
	return c.JSON(http.StatusOK, user)
}

handler_test.go:

package handler

import (
	"net/http"
	"net/http/httptest"
	"testing"

	"github.com/labstack/echo"
	"github.com/stretchr/testify/assert"
)

var (
	mockDB = map[string]*User{
		"jon@labstack.com": &User{"Jon Snow", "jon@labstack.com"},
	}
	userJSON = `{"name":"Jon Snow","email":"jon@labstack.com"}`
)

func TestGetUser(t *testing.T) {
	// Setup
	e := echo.New()
	req := httptest.NewRequest(http.MethodGet, "/", nil)
	rec := httptest.NewRecorder()
	c := e.NewContext(req, rec)
	c.SetPath("/users/:email")
	c.SetParamNames("email")
	c.SetParamValues("jon@labstack.com")
	h := &handler{mockDB}

	// Assertions
	if assert.NoError(t, h.getUser(c)) {
		assert.Equal(t, http.StatusOK, rec.Code)
		assert.Equal(t, userJSON, rec.Body.String())
	}
}

Version/commit

8d7f05e

[i82orbom]

Also facing the same issue here. MaxParams is not settable from an outside package, your tests are passing because in each test case you are manipulating the variable to have the correct array size at all times. This breaks the compatibility

[pratikmallya]

same issue. I believe 8d7f05e#diff-05a3aa0ab9e5687ff167e0ef3ae4c5a6R316 from this PR #1463 may be what is causing the problem

[yenskillah]

yes indeed, maxParams is unsettable outside fo the package.

[noamt]

I'm also experiencing this issue

[yenskillah]

@noamt there's a way to fix this #1463 (comment). It worked like a charm. Thanks @dlowe

[i82orbom]

@yenskillah I think the issue is not solved, the way of "fixing" this cannot be some trick... maxParamValues should be settable from outside or make the array dynamic.

There's even a PR open to address this: #1498

[ginnyedelstein]

i'm experiencing the same issue

[aquincum]

I agree that this is not solved. We have tests that depend on SetParamValues on a context used with a httptest.Recorder and a httptest.Request. We cannot access the maxParams since it's private and there is no way now to set our path parameters.

#1498 would be usable since we could at least manually set maxParam, but I'm not entirely sure why this breaking change was needed in the end.

[i82orbom]

Maybe @dlowe can shed some light. I don't see why the array can't be dynamic... the max param requirement note was introduced by him.

[dlowe]

I did not add the behavior or logic behind maxParam; I just fixed SetParamValues such that it wouldn't cause crashes in other parts of the code.

The comment I added was copied from https://github.com/labstack/echo/blob/master/context.go#L628.

As I said in #1463 (comment): I agree this isn't ideal, but I don't actually understand the point of maxParam, so I can't say what the best solution might be.

[i82orbom]

Maybe @vishr can tell us something

[aquincum]

The author of the issue whose fix broke functionality saying he doesn't "actually understand the point of maxParam" is definitely a red flag to escalate and make sure that the fix actually makes sense and doesn't introduce newer issues.