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

Binding json array fails when there is any parm in url #1565

Closed
3 tasks done
blxzfb307 opened this issue May 7, 2020 · 6 comments · Fixed by #1835
Closed
3 tasks done

Binding json array fails when there is any parm in url #1565

blxzfb307 opened this issue May 7, 2020 · 6 comments · Fixed by #1835
Labels

Comments

@blxzfb307
Copy link

Issue Description

I want to bind a json array in the handler function but I can not do it when there is parm in url.
What is the right way to bind json array in the handler function?

Checklist

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

Expected behaviour

curl -X POST -H "Content-Type: application/json" -d '[1,2,3]' http://127.0.0.1:1323/1
[
  1,
  2,
  3
]

Actual behaviour

curl -X POST -H "Content-Type: application/json" -d '[1,2,3]' http://127.0.0.1:1323/1
"code=400, message=binding element must be a struct, internal=binding element must be a struct"

Steps to reproduce

First build and run the code.
Then use curl to create http requests.
It will returan the arrya in first url but not in second url.

curl -X POST -H "Content-Type: application/json" -d '[1,2,3]' http://127.0.0.1:1323
[
  1,
  2,
  3
]
curl -X POST -H "Content-Type: application/json" -d '[1,2,3]' http://127.0.0.1:1323/1
"code=400, message=binding element must be a struct, internal=binding element must be a struct"

Working code to debug

package main

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

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

	e.POST("", func(ctx echo.Context) error {
		data := make([]uint, 0)
		if err := ctx.Bind(&data); err != nil {
			return err
		}
		return ctx.JSON(200, data)
	})
	e.POST("/:id", func(ctx echo.Context) error {
		data := make([]uint, 0)
		if err := ctx.Bind(&data); err != nil {
			return err
		}
		return ctx.JSON(200, data)
	})

	e.Logger.Fatal(e.Start(":1323"))
}

Version/commit

@Tobbeman
Copy link

Tobbeman commented May 13, 2020

I have the same issue
Issue originates here
https://github.com/labstack/echo/blob/master/bind.go#L43

Introduction of a write-able params would mean we can remove them if we call bind, that could be a solution

Edit:
Thinking more on the issue I would investigate creating a BindParam and BindBody, separating use case. I think Bind is mostly used to bind the http body, having that as a specific use case is probably easier that having a single function that need to juggle both and their respective error handling

Creating new functions would also allow the old to remain the same, thus keeping backwards compatibility.

I can create a PR if maintainers are interested

@Tobbeman
Copy link

My current workaround includes a custom binder

func (cb *CustomBinder) Bind(i interface{}, c echo.Context) (err error) {
	req := c.Request()
	ctype := req.Header.Get(echo.HeaderContentType)
	switch {
	case strings.HasPrefix(ctype, echo.MIMEApplicationJSON):
		if err = json.NewDecoder(req.Body).Decode(i); err == nil {
			return
		}
	}

	db := new(echo.DefaultBinder)
	if err = db.Bind(i, c); err != echo.ErrUnsupportedMediaType {
		return
	}
	return
}

@arigon
Copy link

arigon commented May 14, 2020

I have the same problem. The description is misleading, because it binds the param as well, not only the request body...

// Bind binds the request body into provided type `i`. The default binder
// does it based on Content-Type header.
Bind(i interface{}) error

My workaround: reset the params to nil by calling
c.SetParamNames()

Unfortunately it doesn't work with existing QueryParams

Of course reading the c.Request().Body is always possible.

benoitmasson added a commit to benoitmasson/echo that referenced this issue May 19, 2020
- for JSON, XML and Multipart payloads
- without and with extra (dummy) query parameters, which make the tests fail
  (see e.g. labstack#1565)
benoitmasson added a commit to benoitmasson/echo that referenced this issue May 19, 2020
Do not throw an error when binding to a slice, if binding occurs
on path- or query-parameters
=> data is very likely to be found in the body, which will be bound later

Should fix labstack#1356, labstack#1448, labstack#1495 (second case), labstack#1565
@stale
Copy link

stale bot commented Jul 13, 2020

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.

@stale stale bot added the wontfix label Jul 13, 2020
@stale stale bot closed this as completed Jul 25, 2020
benoitmasson added a commit to benoitmasson/echo that referenced this issue Aug 17, 2020
- for JSON, XML and Multipart payloads
- without and with extra (dummy) query parameters, which make the tests fail
  (see e.g. labstack#1565)
benoitmasson added a commit to benoitmasson/echo that referenced this issue Aug 17, 2020
Do not throw an error when binding to a slice, if binding occurs
on path- or query-parameters
=> data is very likely to be found in the body, which will be bound later

Should fix labstack#1356, labstack#1448, labstack#1495 (second case), labstack#1565
@jamelt
Copy link

jamelt commented Apr 5, 2021

This is not a good practice, closing issues like this, that are valid but "stale"?

@aldas
Copy link
Contributor

aldas commented Apr 5, 2021

This is current limitation for c.Bind() when target is slice it does not know what to do with query or path params.

Details of possible methods to use can be found in documentation: https://echo.labstack.com/guide/binding/

In case you want to bind json body to slice you should use

		data := make([]uint, 0)
		if err := (&echo.DefaultBinder{}).BindBody(ctx, &data); err != nil {
			return err
		}

or go plain standard library

		data := make([]uint, 0)
		if err := json.NewDecoder(ctx.Request().Body).Decode(data); err != nil {
			return err
		}

If you want to bind both then you need have two separate bind calls:

for example accessing path param as ctx.Param("id")

	e.POST("/:id", func(ctx echo.Context) error {
		var data struct{
			ID string
			Data []uint
		}
		if err := (&echo.DefaultBinder{}).BindBody(ctx, &data.Data); err != nil {
			return err
		}
		data.ID = ctx.Param("id")
		return ctx.JSON(200, data)
	})

or binding to struct

	e.POST("/:id", func(ctx echo.Context) error {
		var data struct{
			ID string `param:"id"`
			Data []uint
		}
		if err := (&echo.DefaultBinder{}).BindBody(ctx, &data.Data); err != nil {
			return err
		}
		if err := (&echo.DefaultBinder{}).BindPathParams(ctx, &data); err != nil {
			return err
		}
		return ctx.JSON(200, data)
	})

or fluent api:

	e.POST("/:id", func(ctx echo.Context) error {
		var data struct {
			ID   string
			Data []uint
		}
		if err := json.NewDecoder(ctx.Request().Body).Decode(&data.Data); err != nil {
			return err
		}
		if err := echo.PathParamsBinder(ctx).String("id", &data.ID).BindError(); err != nil {
			return err
		}
		return ctx.JSON(200, data)
	})

aldas added a commit to aldas/echo that referenced this issue Apr 6, 2021
… not being struct.

For path/query params binding we do not try (silently return) to bind when target is not struct.
Recreates PR labstack#1574 and fixes labstack#1565
lammel pushed a commit that referenced this issue Apr 6, 2021
…target not being struct (#1835)

For path/query params binding we do not try (silently return) to bind when target is not struct.
Recreates PR #1574 and fixes #1565
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants