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

🐛 fix: Nil pointer dereference with Must Bind binding #3171

Merged
merged 17 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 17 additions & 16 deletions bind.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,34 +19,35 @@ type StructValidator interface {

// Bind struct
type Bind struct {
ctx Ctx
should bool
ctx Ctx
dontHandleErrs bool
}

// Should To handle binder errors manually, you can prefer Should method.
// If you want to handle binder errors manually, you can use `WithoutAutoHandling`.
// It's default behavior of binder.
func (b *Bind) Should() *Bind {
b.should = true
func (b *Bind) WithoutAutoHandling() *Bind {
b.dontHandleErrs = 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
// If you want to handle binder errors automatically, you can use `WithAutoHandling`.
// If there's an error, it will return the error and set HTTP status to `400 Bad Request`.
// You must still return on error explicitly
func (b *Bind) WithAutoHandling() *Bind {
b.dontHandleErrs = false

return b
}

// Check Should/Must errors and return it by usage.
// Check WithAutoHandling/WithoutAutoHandling errors and return it by usage.
func (b *Bind) returnErr(err error) error {
if !b.should {
b.ctx.Status(StatusBadRequest)
return NewError(StatusBadRequest, "Bad request: "+err.Error())
if err == nil || b.dontHandleErrs {
return err
}

return err
b.ctx.Status(StatusBadRequest)
return NewError(StatusBadRequest, "Bad request: "+err.Error())
ItsMeSamey marked this conversation as resolved.
Show resolved Hide resolved
}

// Struct validation.
Expand All @@ -62,7 +63,7 @@ func (b *Bind) validateStruct(out any) error {
// Custom To use custom binders, you have to use this method.
// You can register them from RegisterCustomBinder method of Fiber instance.
// They're checked by name, if it's not found, it will return an error.
// NOTE: Should/Must is still valid for Custom binders.
// NOTE: WithAutoHandling/WithAutoHandling is still valid for Custom binders.
func (b *Bind) Custom(name string, dest any) error {
binders := b.ctx.App().customBinders
for _, customBinder := range binders {
Expand Down Expand Up @@ -92,7 +93,7 @@ func (b *Bind) RespHeader(out any) error {
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.RequestCtx(), out)); err != nil {
Expand Down
15 changes: 12 additions & 3 deletions bind_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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().WithAutoHandling().returnErr(nil)
require.NoError(t, err)
}

// go test -run Test_Bind_Query -v
func Test_Bind_Query(t *testing.T) {
t.Parallel()
Expand Down Expand Up @@ -1616,8 +1625,8 @@ 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) {
// go test -run Test_Bind_WithAutoHandling
func Test_Bind_WithAutoHandling(t *testing.T) {
app := New()
c := app.AcquireCtx(&fasthttp.RequestCtx{})

Expand All @@ -1626,7 +1635,7 @@ func Test_Bind_Must(t *testing.T) {
}
rq := new(RequiredQuery)
c.Request().URI().SetQueryString("")
err := c.Bind().Must().Query(rq)
err := c.Bind().WithAutoHandling().Query(rq)
require.Equal(t, StatusBadRequest, c.Response().StatusCode())
require.Equal(t, "Bad request: bind: name is empty", err.Error())
}
Expand Down
6 changes: 3 additions & 3 deletions binder/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ app.Get("/", func(c fiber.Ctx) error {
// curl "http://localhost:3000/?name=john&pass=doe&products=shoe,hat"
```

### Behaviors of Should/Must
### Behaviors of WithAutoHandling/WithAutoHandlingWithStatus

Normally, Fiber returns binder error directly. However; if you want to handle it automatically, you can prefer `Must()`.
Normally, Fiber returns binder error directly. However; if you want to handle it automatically, you can prefer `WithAutoHandling()`.

If there's an error it'll return error and 400 as HTTP status. Here's an example for it:

Expand All @@ -99,7 +99,7 @@ type Person struct {
app.Get("/", func(c fiber.Ctx) error {
p := new(Person)

if err := c.Bind().Must().JSON(p); err != nil {
if err := c.Bind().WithAutoHandling().JSON(p); err != nil {
return err
// Status code: 400
// Response: Bad request: name is empty
Expand Down
8 changes: 4 additions & 4 deletions ctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ func (c *DefaultCtx) Get(key string, defaultValue ...string) string {
}

// GetReqHeader returns the HTTP request header specified by filed.
// This function is generic and can handle differnet headers type values.
// This function is generic and can handle different headers type values.
func GetReqHeader[V GenericType](c Ctx, key string, defaultValue ...V) V {
var v V
return genericParseType[V](c.App().getString(c.Request().Header.Peek(key)), v, defaultValue...)
Expand Down Expand Up @@ -1083,7 +1083,7 @@ func (c *DefaultCtx) Params(key string, defaultValue ...string) string {
}

// Params is used to get the route parameters.
// This function is generic and can handle differnet route parameters type values.
// This function is generic and can handle different route parameters type values.
//
// Example:
//
Expand Down Expand Up @@ -1860,8 +1860,8 @@ func (c *DefaultCtx) IsFromLocal() bool {
func (c *DefaultCtx) Bind() *Bind {
if c.bind == nil {
c.bind = &Bind{
ctx: c,
should: true,
ctx : c,
dontHandleErrs: true,
}
}
return c.bind
Expand Down
13 changes: 7 additions & 6 deletions docs/api/bind.md
Original file line number Diff line number Diff line change
Expand Up @@ -458,22 +458,23 @@ The `MIMETypes` method is used to check if the custom binder should be used for

For more control over error handling, you can use the following methods.

### Must
### WithAutoHandling

If you want to handle binder errors automatically, you can use `Must`.
If you want to handle binder errors automatically, you can use `WithAutoHandling`.
If there's an error, it will return the error and set HTTP status to `400 Bad Request`.
This function does NOT panic therefor you must still return on error explicitly

```go title="Signature"
func (b *Bind) Must() *Bind
func (b *Bind) WithAutoHandling() *Bind
```

### Should
### WithoutAutoHandling

To handle binder errors manually, you can use the `Should` method.
To handle binder errors manually, you can use the `WithoutAutoHandling` method.
It's the default behavior of the binder.

```go title="Signature"
func (b *Bind) Should() *Bind
func (b *Bind) WithoutAutoHandling() *Bind
```

## SetParserDecoder
Expand Down