Skip to content

Dont return 400 from .Bind() in POST / PUT with no body #1409

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

Closed
3 tasks done
cousinbenson opened this issue Sep 30, 2019 · 3 comments · Fixed by #1410
Closed
3 tasks done

Dont return 400 from .Bind() in POST / PUT with no body #1409

cousinbenson opened this issue Sep 30, 2019 · 3 comments · Fixed by #1410

Comments

@cousinbenson
Copy link
Contributor

cousinbenson commented Sep 30, 2019

Issue Description

The default binder currently returns 400 if req.ContentLength == 0 during methods that aren't GET or DELETE. It is generally NOT regarded bad practice to make POST / PUT requests with an empty body. It should be up the resource to determine whether or not this should error and not the HTTP protocol.

#1214 was created with regards to this issue, but eventually went stale after no action was taken.

Some discussion on the topic can be found here:

Checklist

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

Expected behaviour

c.Bind() should not return 400 for POST / PUT with empty bodies

Actual behaviour

c.Bind() returns 400 for POST / PUT with empty bodies

Steps to reproduce

call c.Bind() in a POST handler with an empty body

Working code to debug

package main

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

func main() {
	srv := echo.New()
	srv.POST("/test", testPostBody)
	srv.Start(":1234")
}

type Params struct {
	Foo string `param:"foo"`
	Bar string `query:"bar"`
}

func testPostBody(c echo.Context) error {
	var params Params
	if err := c.Bind(&params); err != nil {
		return echo.NewHTTPError(http.StatusBadRequest, err)
	}

	return c.String(200, "OK")
}

Version/commit

@beaufosheau
Copy link

beaufosheau commented Oct 7, 2019

I have a request that binds to an array of structs and it currently does not work with v4.1.11 (works fine in previous versions). Is it bad practice to bind to an array or is this just a minor bug? I am getting the error, "Binding element must be a struct."

g.POST("/CallDownloadRequest", func(c echo.Context) error {
	notes := c.QueryParam("notes")

	var calls []downloadCallRequest
	err := c.Bind(&calls)
	if err != nil {
		fmt.Println(err.Error())
		return c.String(http.StatusBadRequest, "Unable to parse the request")
	}

       // logic
}

Sorry if this is in the wrong place, just seemed related to your changes. I can make a PR if needed.

@vishr
Copy link
Member

vishr commented Oct 7, 2019

package main

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

type (
	Book struct {
		Name string `json:"name"`
	}
)

func main() {
	e := echo.New()
	e.POST("/", func(c echo.Context) error {
		var books []*Book
		if err := c.Bind(&books); err != nil {
			return err
		}
		return c.JSON(200, books)
	})

	e.Logger.Fatal(e.Start(":1323"))
}
curl -X POST \
  http://localhost:1323 \
  -H 'Content-Type: application/json' \
  -d '[{"name": "book1"}, {"name": "book2"}]'

Above works for me.

@beaufosheau
Copy link

Thanks for the quick response! It must be something with my local machine. I have tried your code and it works fine on my laptop and with Postman but curl returns the same error as before. This is all with rolling back to v4.1.10 as well, so definitely unrelated to this issue. Thanks for the help anyway!

package main

import "github.com/labstack/echo"

type (
	Book struct {
		Name string `json:"name"`
	}
)

func main() {
	e := echo.New()
	e.POST("/", func(c echo.Context) error {
		var books []*Book
		if err := c.Bind(&books); err != nil {
			return err
		}
		return c.JSON(200, books)
	})

	e.Logger.Fatal(e.Start(":1323"))
}
C:\<User>\go\src\books>curl -X POST   http://localhost:1323   -H 'Content-Type:application/json'   -d "[{\"name\" : \"book1\" }, {\"name\" : \"book2\" }]"
{"message":"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
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants