Skip to content

Commit b9ccbfa

Browse files
committed
🐛 bug: guard typed-nil errors
1 parent dec796e commit b9ccbfa

8 files changed

Lines changed: 125 additions & 6 deletions

File tree

app.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"github.com/valyala/fasthttp"
3131

3232
"github.com/gofiber/fiber/v3/binder"
33+
"github.com/gofiber/fiber/v3/internal/nilerror"
3334
"github.com/gofiber/fiber/v3/log"
3435
)
3536

@@ -618,6 +619,10 @@ var httpReadResponse = http.ReadResponse
618619

619620
// DefaultErrorHandler that process return errors from handlers
620621
func DefaultErrorHandler(c Ctx, err error) error {
622+
if nilerror.IsNil(err) {
623+
err = nil
624+
}
625+
621626
code := StatusInternalServerError
622627
var e *Error
623628
matched := errors.As(err, &e)

app_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,14 @@ import (
3737
"github.com/valyala/fasthttp/fasthttputil"
3838
)
3939

40+
type typedNilHandlerError struct {
41+
message string
42+
}
43+
44+
func (e *typedNilHandlerError) Error() string {
45+
return e.message
46+
}
47+
4048
type fileView struct {
4149
path string
4250
content string
@@ -645,6 +653,21 @@ func Test_DefaultErrorHandler_TypedNilFiberError(t *testing.T) {
645653
require.Equal(t, utils.StatusMessage(StatusInternalServerError), string(c.fasthttp.Response.Body()))
646654
}
647655

656+
func Test_DefaultErrorHandler_TypedNilCustomError(t *testing.T) {
657+
t.Parallel()
658+
659+
app := New()
660+
c := app.AcquireCtx(&fasthttp.RequestCtx{}).(*DefaultCtx) //nolint:errcheck,forcetypeassert // not needed
661+
t.Cleanup(func() { app.ReleaseCtx(c) })
662+
663+
var err *typedNilHandlerError
664+
require.NotPanics(t, func() {
665+
require.NoError(t, DefaultErrorHandler(c, err))
666+
})
667+
require.Equal(t, StatusInternalServerError, c.fasthttp.Response.StatusCode())
668+
require.Equal(t, utils.StatusMessage(StatusInternalServerError), string(c.fasthttp.Response.Body()))
669+
}
670+
648671
func Test_DefaultErrorHandler_WrappedFiberErrorUsesWrappedMessage(t *testing.T) {
649672
t.Parallel()
650673

bind.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"sync"
1010

1111
"github.com/gofiber/fiber/v3/binder"
12+
"github.com/gofiber/fiber/v3/internal/nilerror"
1213
"github.com/gofiber/schema"
1314
"github.com/gofiber/utils/v2"
1415
utilsbytes "github.com/gofiber/utils/v2/bytes"
@@ -158,7 +159,7 @@ func (b *Bind) SkipValidation(skip bool) *Bind {
158159

159160
// Check WithAutoHandling/WithoutAutoHandling errors and return it by usage.
160161
func (b *Bind) returnErr(err error) error {
161-
if err == nil {
162+
if nilerror.IsNil(err) {
162163
return nil
163164
}
164165

bind_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ import (
2525

2626
const helloWorld = "hello world"
2727

28+
type typedNilBindError struct {
29+
message string
30+
}
31+
32+
func (e *typedNilBindError) Error() string {
33+
return e.message
34+
}
35+
2836
// go test -run Test_returnErr -v
2937
func Test_returnErr(t *testing.T) {
3038
app := New()
@@ -45,6 +53,19 @@ func Test_returnErr_TypedNilFiberError(t *testing.T) {
4553
require.NoError(t, c.Bind().WithAutoHandling().returnErr(err))
4654
}
4755

56+
func Test_returnErr_TypedNilCustomError(t *testing.T) {
57+
t.Parallel()
58+
59+
app := New()
60+
c := app.AcquireCtx(&fasthttp.RequestCtx{})
61+
t.Cleanup(func() { app.ReleaseCtx(c) })
62+
63+
var err *typedNilBindError
64+
require.NotPanics(t, func() {
65+
require.NoError(t, c.Bind().WithAutoHandling().returnErr(err))
66+
})
67+
}
68+
4869
func Test_returnBindErr_TypedNilFiberError(t *testing.T) {
4970
t.Parallel()
5071

ctx_interface_gen.go

Lines changed: 26 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/nilerror/nilerror.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package nilerror
2+
3+
import (
4+
"reflect"
5+
)
6+
7+
// IsNil reports whether err is nil or contains a typed-nil value.
8+
func IsNil(err error) bool {
9+
if err == nil {
10+
return true
11+
}
12+
13+
v := reflect.ValueOf(err)
14+
switch v.Kind() {
15+
case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, reflect.Pointer, reflect.Slice:
16+
return v.IsNil()
17+
default:
18+
return false
19+
}
20+
}

middleware/limiter/limiter.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"errors"
55

66
"github.com/gofiber/fiber/v3"
7+
"github.com/gofiber/fiber/v3/internal/nilerror"
78
)
89

910
const (
@@ -30,6 +31,10 @@ func New(config ...Config) fiber.Handler {
3031

3132
// getEffectiveStatusCode returns the actual status code, considering both the error and response status
3233
func getEffectiveStatusCode(c fiber.Ctx, err error) int {
34+
if nilerror.IsNil(err) {
35+
return c.Response().StatusCode()
36+
}
37+
3338
// If there's an error and it's a *fiber.Error, use its status code
3439
var fiberErr *fiber.Error
3540
if errors.As(err, &fiberErr) && fiberErr != nil {

middleware/limiter/limiter_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ type failingLimiterStorage struct {
2626

2727
const testLimiterClientKey = "client-key"
2828

29+
type typedNilLimiterError struct {
30+
message string
31+
}
32+
33+
func (e *typedNilLimiterError) Error() string {
34+
return e.message
35+
}
36+
2937
func newFailingLimiterStorage() *failingLimiterStorage {
3038
return &failingLimiterStorage{
3139
data: make(map[string][]byte),
@@ -211,6 +219,21 @@ func TestGetEffectiveStatusCodeTypedNilFiberError(t *testing.T) {
211219
})
212220
}
213221

222+
func TestGetEffectiveStatusCodeTypedNilCustomError(t *testing.T) {
223+
t.Parallel()
224+
225+
app := fiber.New()
226+
c := app.AcquireCtx(&fasthttp.RequestCtx{})
227+
t.Cleanup(func() { app.ReleaseCtx(c) })
228+
229+
c.Response().SetStatusCode(fiber.StatusAccepted)
230+
231+
var err *typedNilLimiterError
232+
require.NotPanics(t, func() {
233+
require.Equal(t, fiber.StatusAccepted, getEffectiveStatusCode(c, err))
234+
})
235+
}
236+
214237
func TestLimiterFixedStorageGetError(t *testing.T) {
215238
t.Parallel()
216239

0 commit comments

Comments
 (0)