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

Bind() panics when the destination is a map with non-interface types #2552

Closed
astromechza opened this issue Dec 1, 2023 · 1 comment
Closed
Labels

Comments

@astromechza
Copy link

Issue Description

When calling context Bind(...) with a map which has non interface or non assignable value type, a panic is triggered. Even though this is a rare case, and can be worked around using a different map type, or using BindBody, we should not have panics in echo. This looks like it has been the behavior since 2019 or so based on git blame.

This comes down to the reflect assignment here:

echo/bind.go

Line 137 in 584cb85

val.SetMapIndex(reflect.ValueOf(k), reflect.ValueOf(v[0]))

func (b *DefaultBinder) bindData(destination interface{}, data map[string][]string, tag string) error {
...
	// Map
	if typ.Kind() == reflect.Map {
		for k, v := range data {
			val.SetMapIndex(reflect.ValueOf(k), reflect.ValueOf(v[0]))
		}
		return nil
	}
...

I couldn't find an issue for this elsewhere.

Expected behaviour

Assignable values are bound. Unassignable values are ignored.

Actual behaviour

It panics!

For example:

panic: reflect.Value.SetMapIndex: value of type string is not assignable to type int

Steps to reproduce

See the code below. Call Bind() with a map[string]int and some string query or path parameter available in the context.

Working code to debug

package main

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

type TestBodyMapInt map[string]int

func main() {
	e := echo.New()

	// test: curl -XPOST --header "Content-Type: application/json" -d '{"module1": 2, "module2": 3}'  http://127.0.0.1:8080/test/1
	e.POST("/test/:id", func(c echo.Context) error {
		p := TestBodyMapInt{}
		if err := c.Bind(&p); err != nil {
			return err
		}
		return c.JSON(http.StatusOK, p)
	})

	e.Start("127.0.0.1:8080")
}

Version/commit

github.com/labstack/echo/v4 v4.11.3

@aldas
Copy link
Contributor

aldas commented Dec 3, 2023

Relates to #988

This seems to be old unresolved problem. I think we can make it work so binding to map[string]string and map[string][]string works. As the source for the values we are binding is Path params, Headers, Query params, Form params and all of these are string values. For all other map cases we should error out or skip in case of source being "param/query/header/form".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants