Skip to content

Commit 7ce1722

Browse files
gabygaby
authored
🐛 bug: Execute middleware routes when handling errors (#3846)
1 parent 0e5e040 commit 7ce1722

6 files changed

Lines changed: 261 additions & 19 deletions

File tree

‎app.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1327,6 +1327,22 @@ func (app *App) serverErrorHandler(fctx *fasthttp.RequestCtx, err error) {
13271327
err = NewError(StatusBadRequest, err.Error())
13281328
}
13291329

1330+
if c.getMethodInt() != -1 {
1331+
c.setSkipNonUseRoutes(true)
1332+
defer c.setSkipNonUseRoutes(false)
1333+
1334+
var nextErr error
1335+
if d, isDefault := c.(*DefaultCtx); isDefault {
1336+
_, nextErr = app.next(d)
1337+
} else {
1338+
_, nextErr = app.nextCustom(c)
1339+
}
1340+
1341+
if nextErr != nil && !errors.Is(nextErr, ErrNotFound) && !errors.Is(nextErr, ErrMethodNotAllowed) {
1342+
log.Errorf("serverErrorHandler: middleware traversal failed: %v", nextErr)
1343+
}
1344+
}
1345+
13301346
if catch := app.ErrorHandler(c, err); catch != nil {
13311347
log.Errorf("serverErrorHandler: failed to call ErrorHandler: %v", catch)
13321348
_ = c.SendStatus(StatusInternalServerError) //nolint:errcheck // It is fine to ignore the error here

‎app_integration_test.go‎

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
package fiber_test
2+
3+
import (
4+
"bytes"
5+
"errors"
6+
"net"
7+
"testing"
8+
"time"
9+
10+
"github.com/stretchr/testify/require"
11+
12+
"github.com/gofiber/fiber/v3"
13+
"github.com/gofiber/fiber/v3/middleware/cors"
14+
"github.com/gofiber/fiber/v3/middleware/helmet"
15+
"github.com/gofiber/fiber/v3/middleware/requestid"
16+
"github.com/valyala/fasthttp"
17+
"github.com/valyala/fasthttp/fasthttputil"
18+
)
19+
20+
type integrationCustomCtx struct {
21+
*fiber.DefaultCtx
22+
}
23+
24+
func newIntegrationCustomCtx(app *fiber.App) fiber.CustomCtx {
25+
return &integrationCustomCtx{DefaultCtx: fiber.NewDefaultCtx(app)}
26+
}
27+
28+
func performOversizedRequest(t *testing.T, app *fiber.App, configure func(req *fasthttp.Request)) *fasthttp.Response {
29+
t.Helper()
30+
31+
ln := fasthttputil.NewInmemoryListener()
32+
errCh := make(chan error, 1)
33+
34+
go func() {
35+
errCh <- app.Listener(ln, fiber.ListenConfig{DisableStartupMessage: true})
36+
}()
37+
38+
t.Cleanup(func() {
39+
require.NoError(t, app.Shutdown())
40+
if err := <-errCh; err != nil && !errors.Is(err, net.ErrClosed) {
41+
require.NoError(t, err)
42+
}
43+
})
44+
45+
require.Eventually(t, func() bool {
46+
conn, err := ln.Dial()
47+
if err != nil {
48+
return false
49+
}
50+
if err := conn.Close(); err != nil {
51+
return false
52+
}
53+
return true
54+
}, time.Second, 10*time.Millisecond)
55+
56+
req := fasthttp.AcquireRequest()
57+
resp := fasthttp.AcquireResponse()
58+
59+
req.SetRequestURI("http://example.com/")
60+
req.Header.SetMethod(fiber.MethodPost)
61+
req.Header.Set(fiber.HeaderOrigin, "https://example.com")
62+
req.SetBody(bytes.Repeat([]byte{'a'}, 32))
63+
if configure != nil {
64+
configure(req)
65+
}
66+
67+
client := fasthttp.Client{
68+
Dial: func(string) (net.Conn, error) {
69+
return ln.Dial()
70+
},
71+
}
72+
73+
require.NoError(t, client.Do(req, resp))
74+
75+
respCopy := fasthttp.AcquireResponse()
76+
resp.CopyTo(respCopy)
77+
78+
fasthttp.ReleaseRequest(req)
79+
fasthttp.ReleaseResponse(resp)
80+
81+
t.Cleanup(func() {
82+
fasthttp.ReleaseResponse(respCopy)
83+
})
84+
85+
return respCopy
86+
}
87+
88+
func Test_Integration_App_ServerErrorHandler_PreservesCORSHeadersOnBodyLimit(t *testing.T) {
89+
app := fiber.New(fiber.Config{BodyLimit: 16})
90+
app.Use(cors.New(cors.Config{
91+
AllowOrigins: []string{"https://example.com"},
92+
AllowCredentials: true,
93+
ExposeHeaders: []string{"X-Request-ID"},
94+
}))
95+
app.Post("/", func(c fiber.Ctx) error {
96+
return c.SendStatus(fiber.StatusOK)
97+
})
98+
99+
resp := performOversizedRequest(t, app, nil)
100+
101+
require.Equal(t, fiber.StatusRequestEntityTooLarge, resp.StatusCode())
102+
require.Equal(t, "https://example.com", string(resp.Header.Peek(fiber.HeaderAccessControlAllowOrigin)))
103+
require.Equal(t, "true", string(resp.Header.Peek(fiber.HeaderAccessControlAllowCredentials)))
104+
require.Equal(t, "X-Request-ID", string(resp.Header.Peek(fiber.HeaderAccessControlExposeHeaders)))
105+
require.Equal(t, "Origin", string(resp.Header.Peek(fiber.HeaderVary)))
106+
}
107+
108+
func Test_Integration_App_ServerErrorHandler_PreservesHelmetHeadersOnBodyLimit(t *testing.T) {
109+
app := fiber.New(fiber.Config{BodyLimit: 16})
110+
app.Use(helmet.New())
111+
app.Post("/", func(c fiber.Ctx) error {
112+
return c.SendStatus(fiber.StatusOK)
113+
})
114+
115+
resp := performOversizedRequest(t, app, nil)
116+
117+
require.Equal(t, fiber.StatusRequestEntityTooLarge, resp.StatusCode())
118+
require.Equal(t, "nosniff", string(resp.Header.Peek(fiber.HeaderXContentTypeOptions)))
119+
require.Equal(t, "same-origin", string(resp.Header.Peek("Cross-Origin-Opener-Policy")))
120+
require.Equal(t, "same-origin", string(resp.Header.Peek("Cross-Origin-Resource-Policy")))
121+
require.Equal(t, "require-corp", string(resp.Header.Peek("Cross-Origin-Embedder-Policy")))
122+
}
123+
124+
func Test_Integration_App_ServerErrorHandler_PreservesRequestID(t *testing.T) {
125+
const expectedRequestID = "integration-request-id"
126+
127+
app := fiber.New(fiber.Config{BodyLimit: 16})
128+
app.Use(requestid.New())
129+
app.Post("/", func(c fiber.Ctx) error {
130+
return c.SendStatus(fiber.StatusOK)
131+
})
132+
133+
resp := performOversizedRequest(t, app, func(req *fasthttp.Request) {
134+
req.Header.Set("X-Request-ID", expectedRequestID)
135+
})
136+
137+
require.Equal(t, fiber.StatusRequestEntityTooLarge, resp.StatusCode())
138+
require.Equal(t, expectedRequestID, string(resp.Header.Peek("X-Request-ID")))
139+
}
140+
141+
func Test_Integration_App_ServerErrorHandler_GroupMiddlewareChain(t *testing.T) {
142+
app := fiber.New(fiber.Config{BodyLimit: 16})
143+
app.Use(helmet.New())
144+
145+
api := app.Group("/api")
146+
api.Use(requestid.New())
147+
api.Use(func(c fiber.Ctx) error {
148+
c.Set("X-Group-Middleware", "active")
149+
return c.Next()
150+
})
151+
api.Post("/resource", func(c fiber.Ctx) error {
152+
return c.SendStatus(fiber.StatusOK)
153+
})
154+
155+
resp := performOversizedRequest(t, app, func(req *fasthttp.Request) {
156+
req.SetRequestURI("http://example.com/api/resource")
157+
})
158+
159+
require.Equal(t, fiber.StatusRequestEntityTooLarge, resp.StatusCode())
160+
require.Equal(t, "nosniff", string(resp.Header.Peek(fiber.HeaderXContentTypeOptions)))
161+
require.NotEmpty(t, resp.Header.Peek("X-Request-ID"))
162+
require.Equal(t, "active", string(resp.Header.Peek("X-Group-Middleware")))
163+
}
164+
165+
func Test_Integration_App_ServerErrorHandler_RetainsHeadersFromSubsequentMiddleware(t *testing.T) {
166+
app := fiber.New(fiber.Config{BodyLimit: 8})
167+
app.Use(func(c fiber.Ctx) error {
168+
c.Set("X-Custom-Middleware", "ran")
169+
return c.Next()
170+
})
171+
app.Use(cors.New())
172+
app.Post("/", func(c fiber.Ctx) error {
173+
return c.SendStatus(fiber.StatusOK)
174+
})
175+
176+
resp := performOversizedRequest(t, app, nil)
177+
178+
require.Equal(t, fiber.StatusRequestEntityTooLarge, resp.StatusCode())
179+
require.Equal(t, "ran", string(resp.Header.Peek("X-Custom-Middleware")))
180+
require.Equal(t, "*", string(resp.Header.Peek(fiber.HeaderAccessControlAllowOrigin)))
181+
}
182+
183+
func Test_Integration_App_ServerErrorHandler_WithCustomCtx(t *testing.T) {
184+
app := fiber.NewWithCustomCtx(newIntegrationCustomCtx, fiber.Config{BodyLimit: 16})
185+
app.Use(func(c fiber.Ctx) error {
186+
customCtx, ok := c.(*integrationCustomCtx)
187+
require.True(t, ok)
188+
customCtx.Set("X-Custom-Ctx", "true")
189+
return c.Next()
190+
})
191+
app.Use(cors.New(cors.Config{AllowOrigins: []string{"https://example.org"}}))
192+
app.Post("/", func(c fiber.Ctx) error {
193+
return c.SendStatus(fiber.StatusOK)
194+
})
195+
196+
resp := performOversizedRequest(t, app, func(req *fasthttp.Request) {
197+
req.Header.Set(fiber.HeaderOrigin, "https://example.org")
198+
})
199+
200+
require.Equal(t, fiber.StatusRequestEntityTooLarge, resp.StatusCode())
201+
require.Equal(t, "true", string(resp.Header.Peek("X-Custom-Ctx")))
202+
require.Equal(t, "https://example.org", string(resp.Header.Peek(fiber.HeaderAccessControlAllowOrigin)))
203+
}

‎ctx.go‎

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -49,25 +49,26 @@ const userContextKey contextKey = 0 // __local_user_context__
4949
//
5050
//go:generate ifacemaker --file ctx.go --file req.go --file res.go --struct DefaultCtx --iface Ctx --pkg fiber --promoted --output ctx_interface_gen.go --not-exported true --iface-comment "Ctx represents the Context which hold the HTTP request and response.\nIt has methods for the request query string, parameters, body, HTTP headers and so on."
5151
type DefaultCtx struct {
52-
DefaultReq // Default request api
53-
DefaultRes // Default response api
54-
app *App // Reference to *App
55-
route *Route // Reference to *Route
56-
fasthttp *fasthttp.RequestCtx // Reference to *fasthttp.RequestCtx
57-
bind *Bind // Default bind reference
58-
redirect *Redirect // Default redirect reference
59-
values [maxParams]string // Route parameter values
60-
viewBindMap sync.Map // Default view map to bind template engine
61-
baseURI string // HTTP base uri
62-
pathOriginal string // Original HTTP path
63-
flashMessages redirectionMsgs // Flash messages
64-
path []byte // HTTP path with the modifications by the configuration
65-
detectionPath []byte // Route detection path
66-
treePathHash int // Hash of the path for the search in the tree
67-
indexRoute int // Index of the current route
68-
indexHandler int // Index of the current handler
69-
methodInt int // HTTP method INT equivalent
70-
matched bool // Non use route matched
52+
DefaultReq // Default request api
53+
DefaultRes // Default response api
54+
app *App // Reference to *App
55+
route *Route // Reference to *Route
56+
fasthttp *fasthttp.RequestCtx // Reference to *fasthttp.RequestCtx
57+
bind *Bind // Default bind reference
58+
redirect *Redirect // Default redirect reference
59+
values [maxParams]string // Route parameter values
60+
viewBindMap sync.Map // Default view map to bind template engine
61+
baseURI string // HTTP base uri
62+
pathOriginal string // Original HTTP path
63+
flashMessages redirectionMsgs // Flash messages
64+
path []byte // HTTP path with the modifications by the configuration
65+
detectionPath []byte // Route detection path
66+
treePathHash int // Hash of the path for the search in the tree
67+
indexRoute int // Index of the current route
68+
indexHandler int // Index of the current handler
69+
methodInt int // HTTP method INT equivalent
70+
matched bool // Non use route matched
71+
skipNonUseRoutes bool // Skip non-use routes while iterating middleware
7172
}
7273

7374
// TLSHandler hosts the callback hooks Fiber invokes while negotiating TLS
@@ -554,6 +555,7 @@ func (c *DefaultCtx) Reset(fctx *fasthttp.RequestCtx) {
554555
c.indexHandler = 0
555556
// Reset matched flag
556557
c.matched = false
558+
c.skipNonUseRoutes = false
557559
// Set paths
558560
c.pathOriginal = c.app.toString(fctx.URI().PathOriginal())
559561
// Set method
@@ -584,6 +586,7 @@ func (c *DefaultCtx) release() {
584586
ReleaseRedirect(c.redirect)
585587
c.redirect = nil
586588
}
589+
c.skipNonUseRoutes = false
587590
c.DefaultReq.release()
588591
c.DefaultRes.release()
589592
}
@@ -656,6 +659,10 @@ func (c *DefaultCtx) getMatched() bool {
656659
return c.matched
657660
}
658661

662+
func (c *DefaultCtx) getSkipNonUseRoutes() bool {
663+
return c.skipNonUseRoutes
664+
}
665+
659666
func (c *DefaultCtx) setIndexHandler(handler int) {
660667
c.indexHandler = handler
661668
}
@@ -668,6 +675,10 @@ func (c *DefaultCtx) setMatched(matched bool) {
668675
c.matched = matched
669676
}
670677

678+
func (c *DefaultCtx) setSkipNonUseRoutes(skip bool) {
679+
c.skipNonUseRoutes = skip
680+
}
681+
671682
func (c *DefaultCtx) setRoute(route *Route) {
672683
c.route = route
673684
}

‎ctx_interface.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,11 @@ type CustomCtx interface {
2626
getPathOriginal() string
2727
getValues() *[maxParams]string
2828
getMatched() bool
29+
getSkipNonUseRoutes() bool
2930
setIndexHandler(handler int)
3031
setIndexRoute(route int)
3132
setMatched(matched bool)
33+
setSkipNonUseRoutes(skip bool)
3234
setRoute(route *Route)
3335
}
3436

‎ctx_interface_gen.go‎

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

‎router.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,10 @@ func (app *App) next(c *DefaultCtx) (bool, error) {
139139
continue
140140
}
141141

142+
if c.skipNonUseRoutes && !route.use {
143+
continue
144+
}
145+
142146
// Pass route reference and param values
143147
c.route = route
144148
// Non use handler matched
@@ -235,6 +239,10 @@ func (app *App) nextCustom(c CustomCtx) (bool, error) {
235239
if !route.match(c.getDetectionPath(), c.Path(), c.getValues()) {
236240
continue
237241
}
242+
if c.getSkipNonUseRoutes() && !route.use {
243+
continue
244+
}
245+
238246
// Pass route reference and param values
239247
c.setRoute(route)
240248
// Non use handler matched

0 commit comments

Comments
 (0)