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

Regression: binding a map is not allowed anymore #988

Closed
3 tasks done
jinroh opened this issue Aug 10, 2017 · 20 comments
Closed
3 tasks done

Regression: binding a map is not allowed anymore #988

jinroh opened this issue Aug 10, 2017 · 20 comments
Assignees
Labels

Comments

@jinroh
Copy link

jinroh commented Aug 10, 2017

Issue Description

I think a valid use case of binding to a map[string]interface{} now causes the following error: Binding element must be a struct. I think the regression has been caused by #972.

Is this use case still valid ?

Checklist

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

Expected behaviour

Binding a map[string]interface{} is valid.

Actual behaviour

Binding a map[string]interface{} causes an error.

Code to debug

package main

func handler(c echo.Context) {
  var m map[string]interface{}
  if err := c.Bind(&m); err != nil {
    // err is "Binding element must be a struct"
  }
  return nil
}

Version/commit

1e9845a

@jinroh jinroh changed the title Regression: Regression: binding a map is not allowed anymore Aug 10, 2017
@vishr vishr self-assigned this Aug 14, 2017
@vishr vishr added the bug label Aug 14, 2017
@vishr vishr closed this as completed in 70c1806 Aug 15, 2017
@narrowizard
Copy link

narrowizard commented Jul 12, 2019

Has this issue been solved? I meet it today. my code as following:

maybe you need to repear it locally since go playground cannot listen port.
@vishr

@thanhps42
Copy link

thanhps42 commented Jul 13, 2019

Has this issue been solved? I meet it today. my code as following:

maybe you need to repear it locally since go playground cannot listen port.
@vishr

me too

@vishr
Copy link
Member

vishr commented Jul 13, 2019

I am not sure what is the issue but the following code works

package main

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

func main() {
	e := echo.New()
	e.GET("/test", func(c echo.Context) error {
		p := map[string]interface{}{}
		if err := c.Bind(&p); err != nil {
			return err
		}
		return c.JSON(http.StatusOK, p)
	})

	e.Start("127.0.0.1:8080")
}
curl -X GET \
  http://localhost:8080/test \
  -H 'Content-Type: application/json' \
  -d '{
	"a": 1,
	"b": 2
}'

@narrowizard
Copy link

thanks for your reply,
and your eg works fine.
it seems that bind to map only works when content-type is application/json, does it?

@vishr
Copy link
Member

vishr commented Jul 15, 2019

How are you sending the data?

@narrowizard
Copy link

narrowizard commented Jul 15, 2019

i've tested both following format:

  • curl -X GET -H 'Content-Type: application/x-www-form-urlencoded' http://localhost:8080/test?a=1&b=2
  • curl -X GET http://localhost:8080/test -H 'Content-Type: application/x-www-form-urlencoded' -d 'a=1&b=2'

and echo returns binding element must be a struct error.

@narrowizard
Copy link

after a deep dive to echo.
I find that ctx.Bind returns err in some situation with a map destination.

  • In a GET or DELETE request without request body.

    echo/bind.go

    Lines 47 to 49 in 01229ec

    if req.ContentLength == 0 {
    if req.Method == http.MethodGet || req.Method == http.MethodDelete {
    if err = b.bindData(i, c.QueryParams(), "query"); err != nil {
  • In a request with Content-Type: application/form-data or application/x-www-form-urlencoded.

    echo/bind.go

    Lines 76 to 81 in 01229ec

    case strings.HasPrefix(ctype, MIMEApplicationForm), strings.HasPrefix(ctype, MIMEMultipartForm):
    params, err := c.FormParams()
    if err != nil {
    return NewHTTPError(http.StatusBadRequest, err.Error()).SetInternal(err)
    }
    if err = b.bindData(i, params, "form"); err != nil {

    echo/bind.go

    Lines 97 to 99 in 01229ec

    if typ.Kind() != reflect.Struct {
    return errors.New("binding element must be a struct")
    }

Do it possible to support Map in DefaultBinder? or this will cause some other issues? @vishr

vishr added a commit that referenced this issue Jul 15, 2019
Signed-off-by: Vishal Rana <vr@labstack.com>
@vishr
Copy link
Member

vishr commented Jul 15, 2019

@narrowizard Just push a fix, let me know how it works out for you.

@narrowizard
Copy link

it works now except that it causes a panic if ptr is of type map[string]interface{} with nil value. it's better to add an nil check before set value.

echo/bind.go

Lines 97 to 98 in da45981

if m, ok := ptr.(*map[string]interface{}); ok {
for k, v := range data {

var p map[string]interface
ctx.Bind(&p) // here panic `assignment to entry in nil map`

anyway, thanks for your impl. @vishr

vishr added a commit that referenced this issue Jul 15, 2019
Signed-off-by: Vishal Rana <vr@labstack.com>
@vishr
Copy link
Member

vishr commented Jul 15, 2019 via email

@nono
Copy link

nono commented Aug 19, 2019

Well, it looks like binding a map can still panic with echo v4.1.10: https://travis-ci.org/cozy/cozy-stack/jobs/573823462#L696-L703. The code is in this PR: cozy/cozy-stack#2015.

@vishr
Copy link
Member

vishr commented Aug 19, 2019

@nono I will look into this today.

@KanybekMomukeyev
Copy link

@vishr
I also encountered this kind of problem.
If i send Post request with same json file, everything is ok.
But if i send Put request with same json file, error on parsing

m := echo.Map{}
fmt.Printf("\n\n confirmOrCancelPaymentOrder ENTERED")
if err := c.Bind(&m); err != nil {
	fmt.Printf("\n PARSING ERROROROROOR \n")
	return err
}

@vishr
Copy link
Member

vishr commented Sep 13, 2019

Folks, I need help here. Let me know if you are willing to contribute.

@KanybekMomukeyev
Copy link

KanybekMomukeyev commented Sep 13, 2019

@vishr

  1. this happen in latest version of Echo
    s.PUT("/users/:id", func(c echo.Context) error

  2. Also this happen when add :id param in url.

RESPONSE BODY: {"message":"binding element must be a struct"}
RESPONSE STATUS: 400 Bad Request

@KanybekMomukeyev
Copy link

KanybekMomukeyev commented Sep 13, 2019

Solved, this is working for me:

order struct {
  ID    int    `json:"id" param:"id"`
  State string `json:"state"`
}

e := echo.New()
e.Use(middleware.Logger())
e.PUT("/orders/:id", confirmOrder)

//JSON:
// PUT /orders/{id}
// {"state": "CONFIRM"}

ord := order{}
if err := c.Bind(&ord); err != nil {
  log.Fatal(err)
}
log.Print(ord)
return nil

@arigon
Copy link

arigon commented Oct 9, 2019

I have the same problem with using the :id param and PUT/PATCH/POST and Bind(). It only works with version 4.1.6 or older..
I am sending a JSON Body with Content-Type application/json to a route: /api/path/:id

@JesseObrien
Copy link

JesseObrien commented Oct 23, 2019

I'm having this problem with echo v4.1.11. When I have this code:

callbackURL := ctx.QueryParam("callback")

payload := echo.Map{}

err := ctx.Bind(&payload)

This produces the error:

binding element must be a struct

If I remove the callbackURL line, it binds just fine. Anyone have any clues as to what's going on here?

@vishr vishr reopened this Oct 24, 2019
@vishr vishr closed this as completed in 7c5e9ab Oct 24, 2019
@arigon
Copy link

arigon commented Oct 28, 2019

Thank you, @vishr. It works for me.

@toorop
Copy link

toorop commented Nov 25, 2019

Same problem here with Echo v4.1.11

month := c.Param("month")
var lines []models.CLinesToUpdate
if err := c.Bind(&lines); err != nil {

=> binding element must be a struct

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

9 participants