From 0fa70ab211a416d681086b0ab8a2f4a3b86f01c0 Mon Sep 17 00:00:00 2001 From: ItsMeSamey <92444995+ItsMeSamey@users.noreply.github.com> Date: Fri, 18 Oct 2024 02:16:48 +0530 Subject: [PATCH 1/6] Fix nil pointer dereference with Must Bind binding error if err is nil err.Error() panics (eg. c.Bind().Must().JSON(...) successfully binds but panics --- bind.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bind.go b/bind.go index e202cd85e0..b66d0a3da2 100644 --- a/bind.go +++ b/bind.go @@ -41,7 +41,7 @@ func (b *Bind) Must() *Bind { // Check Should/Must errors and return it by usage. func (b *Bind) returnErr(err error) error { - if !b.should { + if !b.should && err != nil { b.ctx.Status(StatusBadRequest) return NewError(StatusBadRequest, "Bad request: "+err.Error()) } From 791f4bae86af51d91d0f3a9bc565671288688884 Mon Sep 17 00:00:00 2001 From: ItsMeSamey <92444995+ItsMeSamey@users.noreply.github.com> Date: Fri, 18 Oct 2024 13:58:19 +0530 Subject: [PATCH 2/6] Added returnErr test make sure returnErr works with nil error --- bind_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/bind_test.go b/bind_test.go index aa00e191ca..9cbb51c46c 100644 --- a/bind_test.go +++ b/bind_test.go @@ -19,6 +19,15 @@ import ( const helloWorld = "hello world" +// go test -run Test_returnErr -v +func Test_returnErr(t *testing.T) { + app := New() + c := app.AcquireCtx(&fasthttp.RequestCtx{}) + + err := c.Bind().Must().returnErr(nil) + require.Equal(t, nil, err) +} + // go test -run Test_Bind_Query -v func Test_Bind_Query(t *testing.T) { t.Parallel() From dde670f7204d840869c3c77c47faffc1b069fe47 Mon Sep 17 00:00:00 2001 From: ItsMeSamey <92444995+ItsMeSamey@users.noreply.github.com> Date: Fri, 18 Oct 2024 14:00:30 +0530 Subject: [PATCH 3/6] Reordered returnErr nil check as in majority of cases we expect err to be nil, this should provide better short-cutting --- bind.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bind.go b/bind.go index b66d0a3da2..a55b91cb38 100644 --- a/bind.go +++ b/bind.go @@ -41,12 +41,12 @@ func (b *Bind) Must() *Bind { // Check Should/Must errors and return it by usage. func (b *Bind) returnErr(err error) error { - if !b.should && err != nil { - b.ctx.Status(StatusBadRequest) - return NewError(StatusBadRequest, "Bad request: "+err.Error()) + if err == nil || b.should { + return err } - return err + b.ctx.Status(StatusBadRequest) + return NewError(StatusBadRequest, "Bad request: "+err.Error()) } // Struct validation. From 94d08adcd049bef9705eef88923e0c2db39e7042 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Fri, 18 Oct 2024 08:37:47 -0400 Subject: [PATCH 4/6] Use require.NoError --- bind_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bind_test.go b/bind_test.go index 9cbb51c46c..4f4fb71ec6 100644 --- a/bind_test.go +++ b/bind_test.go @@ -25,7 +25,7 @@ func Test_returnErr(t *testing.T) { c := app.AcquireCtx(&fasthttp.RequestCtx{}) err := c.Bind().Must().returnErr(nil) - require.Equal(t, nil, err) + require.NoError(t, nil, err) } // go test -run Test_Bind_Query -v From b15eda0891646a072b1773a8003cfaeec116b7b0 Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Fri, 18 Oct 2024 08:41:19 -0400 Subject: [PATCH 5/6] Update bind_test.go --- bind_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bind_test.go b/bind_test.go index 4f4fb71ec6..710c7aec8c 100644 --- a/bind_test.go +++ b/bind_test.go @@ -25,7 +25,7 @@ func Test_returnErr(t *testing.T) { c := app.AcquireCtx(&fasthttp.RequestCtx{}) err := c.Bind().Must().returnErr(nil) - require.NoError(t, nil, err) + require.NoError(t, err) } // go test -run Test_Bind_Query -v From dde4c8185fd2f16a46e5110bf50ee6d37c4f78e9 Mon Sep 17 00:00:00 2001 From: ItsMeSamey Date: Wed, 23 Oct 2024 23:00:56 +0530 Subject: [PATCH 6/6] removed Must --- bind.go | 53 +++++++++++++--------------------------------------- bind_test.go | 24 ------------------------ ctx.go | 1 - 3 files changed, 13 insertions(+), 65 deletions(-) diff --git a/bind.go b/bind.go index a55b91cb38..0b95a3f24d 100644 --- a/bind.go +++ b/bind.go @@ -19,34 +19,7 @@ type StructValidator interface { // Bind struct type Bind struct { - ctx Ctx - should bool -} - -// Should To handle binder errors manually, you can prefer Should method. -// It's default behavior of binder. -func (b *Bind) Should() *Bind { - b.should = true - - return b -} - -// Must If you want to handle binder errors automatically, you can use Must. -// If there's an error it'll return error and 400 as HTTP status. -func (b *Bind) Must() *Bind { - b.should = false - - return b -} - -// Check Should/Must errors and return it by usage. -func (b *Bind) returnErr(err error) error { - if err == nil || b.should { - return err - } - - b.ctx.Status(StatusBadRequest) - return NewError(StatusBadRequest, "Bad request: "+err.Error()) + ctx Ctx } // Struct validation. @@ -67,7 +40,7 @@ func (b *Bind) Custom(name string, dest any) error { binders := b.ctx.App().customBinders for _, customBinder := range binders { if customBinder.Name() == name { - return b.returnErr(customBinder.Parse(b.ctx, dest)) + return customBinder.Parse(b.ctx, dest) } } @@ -76,7 +49,7 @@ func (b *Bind) Custom(name string, dest any) error { // Header binds the request header strings into the struct, map[string]string and map[string][]string. func (b *Bind) Header(out any) error { - if err := b.returnErr(binder.HeaderBinder.Bind(b.ctx.Request(), out)); err != nil { + if err := binder.HeaderBinder.Bind(b.ctx.Request(), out); err != nil { return err } @@ -85,17 +58,17 @@ func (b *Bind) Header(out any) error { // RespHeader binds the response header strings into the struct, map[string]string and map[string][]string. func (b *Bind) RespHeader(out any) error { - if err := b.returnErr(binder.RespHeaderBinder.Bind(b.ctx.Response(), out)); err != nil { + if err := binder.RespHeaderBinder.Bind(b.ctx.Response(), out); err != nil { return err } return b.validateStruct(out) } -// Cookie binds the requesr cookie strings into the struct, map[string]string and map[string][]string. +// Cookie binds the request cookie strings into the struct, map[string]string and map[string][]string. // NOTE: If your cookie is like key=val1,val2; they'll be binded as an slice if your map is map[string][]string. Else, it'll use last element of cookie. func (b *Bind) Cookie(out any) error { - if err := b.returnErr(binder.CookieBinder.Bind(b.ctx.Context(), out)); err != nil { + if err := binder.CookieBinder.Bind(b.ctx.Context(), out); err != nil { return err } @@ -104,7 +77,7 @@ func (b *Bind) Cookie(out any) error { // Query binds the query string into the struct, map[string]string and map[string][]string. func (b *Bind) Query(out any) error { - if err := b.returnErr(binder.QueryBinder.Bind(b.ctx.Context(), out)); err != nil { + if err := binder.QueryBinder.Bind(b.ctx.Context(), out); err != nil { return err } @@ -113,7 +86,7 @@ func (b *Bind) Query(out any) error { // JSON binds the body string into the struct. func (b *Bind) JSON(out any) error { - if err := b.returnErr(binder.JSONBinder.Bind(b.ctx.Body(), b.ctx.App().Config().JSONDecoder, out)); err != nil { + if err := binder.JSONBinder.Bind(b.ctx.Body(), b.ctx.App().Config().JSONDecoder, out); err != nil { return err } @@ -122,7 +95,7 @@ func (b *Bind) JSON(out any) error { // XML binds the body string into the struct. func (b *Bind) XML(out any) error { - if err := b.returnErr(binder.XMLBinder.Bind(b.ctx.Body(), out)); err != nil { + if err := binder.XMLBinder.Bind(b.ctx.Body(), out); err != nil { return err } @@ -131,7 +104,7 @@ func (b *Bind) XML(out any) error { // Form binds the form into the struct, map[string]string and map[string][]string. func (b *Bind) Form(out any) error { - if err := b.returnErr(binder.FormBinder.Bind(b.ctx.Context(), out)); err != nil { + if err := binder.FormBinder.Bind(b.ctx.Context(), out); err != nil { return err } @@ -140,7 +113,7 @@ func (b *Bind) Form(out any) error { // URI binds the route parameters into the struct, map[string]string and map[string][]string. func (b *Bind) URI(out any) error { - if err := b.returnErr(binder.URIBinder.Bind(b.ctx.Route().Params, b.ctx.Params, out)); err != nil { + if err := binder.URIBinder.Bind(b.ctx.Route().Params, b.ctx.Params, out); err != nil { return err } @@ -149,7 +122,7 @@ func (b *Bind) URI(out any) error { // MultipartForm binds the multipart form into the struct, map[string]string and map[string][]string. func (b *Bind) MultipartForm(out any) error { - if err := b.returnErr(binder.FormBinder.BindMultipart(b.ctx.Context(), out)); err != nil { + if err := binder.FormBinder.BindMultipart(b.ctx.Context(), out); err != nil { return err } @@ -171,7 +144,7 @@ func (b *Bind) Body(out any) error { for _, customBinder := range binders { for _, mime := range customBinder.MIMETypes() { if mime == ctype { - return b.returnErr(customBinder.Parse(b.ctx, out)) + return customBinder.Parse(b.ctx, out) } } } diff --git a/bind_test.go b/bind_test.go index 710c7aec8c..6fd5e1838c 100644 --- a/bind_test.go +++ b/bind_test.go @@ -19,15 +19,6 @@ import ( const helloWorld = "hello world" -// go test -run Test_returnErr -v -func Test_returnErr(t *testing.T) { - app := New() - c := app.AcquireCtx(&fasthttp.RequestCtx{}) - - err := c.Bind().Must().returnErr(nil) - require.NoError(t, err) -} - // go test -run Test_Bind_Query -v func Test_Bind_Query(t *testing.T) { t.Parallel() @@ -1625,21 +1616,6 @@ func Test_Bind_CustomBinder(t *testing.T) { require.Equal(t, "john", d.Name) } -// go test -run Test_Bind_Must -func Test_Bind_Must(t *testing.T) { - app := New() - c := app.AcquireCtx(&fasthttp.RequestCtx{}) - - type RequiredQuery struct { - Name string `query:"name,required"` - } - rq := new(RequiredQuery) - c.Request().URI().SetQueryString("") - err := c.Bind().Must().Query(rq) - require.Equal(t, StatusBadRequest, c.Response().StatusCode()) - require.Equal(t, "Bad request: bind: name is empty", err.Error()) -} - // simple struct validator for testing type structValidator struct{} diff --git a/ctx.go b/ctx.go index 9e61d0903e..1c6970aa97 100644 --- a/ctx.go +++ b/ctx.go @@ -1859,7 +1859,6 @@ func (c *DefaultCtx) Bind() *Bind { if c.bind == nil { c.bind = &Bind{ ctx: c, - should: true, } } return c.bind