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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 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
18 changes: 9 additions & 9 deletions bind.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,22 @@ func (b *Bind) Should() *Bind {
return b
}

// Must If you want to handle binder errors automatically, you can use Must.
// If you want to handle binder errors automatically, you can use WithAutoHandling.
ReneWerner87 marked this conversation as resolved.
Show resolved Hide resolved
// If there's an error it'll return error and 400 as HTTP status.
func (b *Bind) Must() *Bind {
func (b *Bind) WithAutoHandling() *Bind {
b.should = false

return b
}

// Check Should/Must errors and return it by usage.
// Check Should/WithAutoHandling 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.should {
ItsMeSamey marked this conversation as resolved.
Show resolved Hide resolved
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 +62,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: Should/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 +92,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 Should/WithAutoHandling

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
7 changes: 4 additions & 3 deletions docs/api/bind.md
Original file line number Diff line number Diff line change
Expand Up @@ -458,13 +458,14 @@ 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
Expand Down