Bind causes panic or false-positive error if path has :param

[daniel-nichter]

Issue Description

In v4.1.14 and v4.1.16, Bind is still broken (similar to issues #1448 and #1356) when the path has a :param and the bind variable is not initialized. Without a path :param, both work as expected.

Checklist

• [*] Dependencies installed
• [*] No typos
• [*] Searched existing issues and docs

Expected behaviour

Bind should work for both paths with params in the example code below, no panic or error.

Actual behaviour

Panic on bind to a single var.
False-positive error on bind to slice of vars.

Steps to reproduce

See working code below.

Working code to debug

package main

import (
    "github.com/labstack/echo/v4"
    "log"
)

type T map[string]interface{}

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

    s.PUT("/one/:foo", func(c echo.Context) error {
        var t T
        if err := c.Bind(&t); err != nil {
            log.Fatal(err)
        }
        log.Printf("OK: %+v", t)
        return nil
    })

    s.PUT("/many/:foo", func(c echo.Context) error {
        var t []T
        if err := c.Bind(&t); err != nil {
            log.Fatal(err)
        }
        log.Printf("OK: %+v", t)
        return nil
    })

    s.Start(":8811")
}

For single var (/one/:foo):

$ curl -X PUT -H "Content-Type: application/json" -d '{"name":"John"}' localhost:8811/one/foo

   ____    __
  / __/___/ /  ___
 / _// __/ _ \/ _ \
/___/\__/_//_/\___/ v4.1.14
High performance, minimalist Go web framework
https://echo.labstack.com
____________________________________O/_______
                                    O\
⇨ http server started on [::]:8811
echo: http: panic serving 127.0.0.1:62092: assignment to entry in nil map
goroutine 21 [running]:
net/http.(*conn).serve.func1(0xc0000ccaa0)
	/usr/local/Cellar/go/1.13.3/libexec/src/net/http/server.go:1767 +0x139
panic(0x1310540, 0x13df5b0)
	/usr/local/Cellar/go/1.13.3/libexec/src/runtime/panic.go:679 +0x1b2
reflect.mapassign(0x1310240, 0x0, 0xc00009f0e0, 0xc00009f100)
	/usr/local/Cellar/go/1.13.3/libexec/src/runtime/map.go:1329 +0x3f
reflect.Value.SetMapIndex(0x1310240, 0xc0000b8060, 0x195, 0x12f5e20, 0xc00009f0e0, 0x98, 0x1306240, 0xc00009f100, 0x94)
	/usr/local/Cellar/go/1.13.3/libexec/src/reflect/value.go:1679 +0x268
github.com/labstack/echo/v4.(*DefaultBinder).bindData(0x1623b80, 0x12e7f80, 0xc0000b8060, 0xc000119918, 0x1374844, 0x5, 0x104b9a6, 0x105d1f0)
	/Users/dn/Development/go/pkg/mod/github.com/labstack/echo/v4@v4.1.14/bind.go:96 +0x23b
github.com/labstack/echo/v4.(*DefaultBinder).Bind(0x1623b80, 0x12e7f80, 0xc0000b8060, 0x13f1520, 0xc0000ccb40, 0xc000119b01, 0xc0000b8060)
	/Users/dn/Development/go/pkg/mod/github.com/labstack/echo/v4@v4.1.14/bind.go:43 +0x2a5
github.com/labstack/echo/v4.(*context).Bind(0xc0000ccb40, 0x12e7f80, 0xc0000b8060, 0xc000092540, 0xc000108070)
	/Users/dn/Development/go/pkg/mod/github.com/labstack/echo/v4@v4.1.14/context.go:396 +0x63
main.main.func1(0x13f1520, 0xc0000ccb40, 0x0, 0x0)
	/Users/dn/Development/go/src/github.com/daniel-nichter/lab/echov4/main.go:15 +0x60
github.com/labstack/echo/v4.(*Echo).add.func1(0x13f1520, 0xc0000ccb40, 0x0, 0x0)
	/Users/dn/Development/go/pkg/mod/github.com/labstack/echo/v4@v4.1.14/echo.go:509 +0x8a
github.com/labstack/echo/v4.(*Echo).ServeHTTP(0xc000106000, 0x13ea420, 0xc0001041c0, 0xc00011c000)
	/Users/dn/Development/go/pkg/mod/github.com/labstack/echo/v4@v4.1.14/echo.go:620 +0x16c
net/http.serverHandler.ServeHTTP(0xc000104000, 0x13ea420, 0xc0001041c0, 0xc00011c000)
	/usr/local/Cellar/go/1.13.3/libexec/src/net/http/server.go:2802 +0xa4
net/http.(*conn).serve(0xc0000ccaa0, 0x13eab20, 0xc0000905c0)
	/usr/local/Cellar/go/1.13.3/libexec/src/net/http/server.go:1890 +0x875
created by net/http.(*Server).Serve
	/usr/local/Cellar/go/1.13.3/libexec/src/net/http/server.go:2927 +0x38e

For a slice of vars (/many/:foo)

$ curl -X PUT -H "Content-Type: application/json" -d '[{"name":"John"}]' localhost:8811/many/foo

2020/02/01 15:05:57 code=400, message=binding element must be a struct, internal=binding element must be a struct

Remove :foo param from both paths and both will work as expected.

Version/commit

v4.1.14 and v4.1.16

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[daniel-nichter]

Tested again with v4.1.16: same results. Someone emailed me and said this isn't a bug (I don't have the email off hand), but since there's been no reply to this issue, I still consider it a bug.

The server should definitely not panic (first case with single var), and it always worked before in the second case (slice of vars).

[leefernandes]

.Bind attempts to apply more than just request bodies on the bind target.

If your struct has a field which matches a url :param such as :id echo appears to assign the value of the url :id param on the bind target.
https://github.com/labstack/echo/blob/master/bind.go#L34

It caught me by surprise, and if you're not aware of this, there's a lot of potential for unwanted side-effects.

Given this, for your scenario I can't imagine how it would apply :foo into a slice of type T. I'd suggest looking at a custom binder: https://echo.labstack.com/guide/request#bind-data

imo the mega Bind implementation is neat, but somewhat overloaded, and maybe it's preferable to specify the type of request content wanted to bind, instead of choosing all the things else roll your own binder.

Perhaps composable bind implementations?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.