From e5ef561b3c0326a12c3957eeb0907d4f45f7c651 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= Date: Sat, 22 Feb 2025 18:07:31 +0100 Subject: [PATCH 1/5] fix handler order in routing #3312 --- app.go | 12 ++++++------ group.go | 48 ++++++++++++++++++++++++------------------------ register.go | 8 ++++---- router.go | 32 ++++++++++++++++---------------- router_test.go | 33 +++++++++++++++++++++++++++++++++ 5 files changed, 83 insertions(+), 50 deletions(-) diff --git a/app.go b/app.go index c867cc4e957..76fbcaddc06 100644 --- a/app.go +++ b/app.go @@ -750,7 +750,7 @@ func (app *App) Use(args ...any) Router { return app } - app.register([]string{methodUse}, prefix, nil, nil, handlers...) + app.register([]string{methodUse}, prefix, nil, handlers...) } return app @@ -810,15 +810,15 @@ func (app *App) Patch(path string, handler Handler, middleware ...Handler) Route } // Add allows you to specify multiple HTTP methods to register a route. -func (app *App) Add(methods []string, path string, handler Handler, middleware ...Handler) Router { - app.register(methods, path, nil, handler, middleware...) +func (app *App) Add(methods []string, path string, handler Handler, handlers ...Handler) Router { + app.register(methods, path, nil, append([]Handler{handler}, handlers...)...) return app } // All will register the handler on all HTTP methods -func (app *App) All(path string, handler Handler, middleware ...Handler) Router { - return app.Add(app.config.RequestMethods, path, handler, middleware...) +func (app *App) All(path string, handler Handler, handlers ...Handler) Router { + return app.Add(app.config.RequestMethods, path, handler, handlers...) } // Group is used for Routes with common prefix to define a new sub-router with optional middleware. @@ -828,7 +828,7 @@ func (app *App) All(path string, handler Handler, middleware ...Handler) Router func (app *App) Group(prefix string, handlers ...Handler) Router { grp := &Group{Prefix: prefix, app: app} if len(handlers) > 0 { - app.register([]string{methodUse}, prefix, grp, nil, handlers...) + app.register([]string{methodUse}, prefix, grp, handlers...) } if err := app.hooks.executeOnGroupHooks(*grp); err != nil { panic(err) diff --git a/group.go b/group.go index 4142b0ba239..fe5cc8d014a 100644 --- a/group.go +++ b/group.go @@ -97,7 +97,7 @@ func (grp *Group) Use(args ...any) Router { return grp } - grp.app.register([]string{methodUse}, getGroupPath(grp.Prefix, prefix), grp, nil, handlers...) + grp.app.register([]string{methodUse}, getGroupPath(grp.Prefix, prefix), grp, handlers...) } if !grp.anyRouteDefined { @@ -109,60 +109,60 @@ func (grp *Group) Use(args ...any) Router { // Get registers a route for GET methods that requests a representation // of the specified resource. Requests using GET should only retrieve data. -func (grp *Group) Get(path string, handler Handler, middleware ...Handler) Router { - return grp.Add([]string{MethodGet}, path, handler, middleware...) +func (grp *Group) Get(path string, handler Handler, handlers ...Handler) Router { + return grp.Add([]string{MethodGet}, path, handler, handlers...) } // Head registers a route for HEAD methods that asks for a response identical // to that of a GET request, but without the response body. -func (grp *Group) Head(path string, handler Handler, middleware ...Handler) Router { - return grp.Add([]string{MethodHead}, path, handler, middleware...) +func (grp *Group) Head(path string, handler Handler, handlers ...Handler) Router { + return grp.Add([]string{MethodHead}, path, handler, handlers...) } // Post registers a route for POST methods that is used to submit an entity to the // specified resource, often causing a change in state or side effects on the server. -func (grp *Group) Post(path string, handler Handler, middleware ...Handler) Router { - return grp.Add([]string{MethodPost}, path, handler, middleware...) +func (grp *Group) Post(path string, handler Handler, handlers ...Handler) Router { + return grp.Add([]string{MethodPost}, path, handler, handlers...) } // Put registers a route for PUT methods that replaces all current representations // of the target resource with the request payload. -func (grp *Group) Put(path string, handler Handler, middleware ...Handler) Router { - return grp.Add([]string{MethodPut}, path, handler, middleware...) +func (grp *Group) Put(path string, handler Handler, handlers ...Handler) Router { + return grp.Add([]string{MethodPut}, path, handler, handlers...) } // Delete registers a route for DELETE methods that deletes the specified resource. -func (grp *Group) Delete(path string, handler Handler, middleware ...Handler) Router { - return grp.Add([]string{MethodDelete}, path, handler, middleware...) +func (grp *Group) Delete(path string, handler Handler, handlers ...Handler) Router { + return grp.Add([]string{MethodDelete}, path, handler, handlers...) } // Connect registers a route for CONNECT methods that establishes a tunnel to the // server identified by the target resource. -func (grp *Group) Connect(path string, handler Handler, middleware ...Handler) Router { - return grp.Add([]string{MethodConnect}, path, handler, middleware...) +func (grp *Group) Connect(path string, handler Handler, handlers ...Handler) Router { + return grp.Add([]string{MethodConnect}, path, handler, handlers...) } // Options registers a route for OPTIONS methods that is used to describe the // communication options for the target resource. -func (grp *Group) Options(path string, handler Handler, middleware ...Handler) Router { - return grp.Add([]string{MethodOptions}, path, handler, middleware...) +func (grp *Group) Options(path string, handler Handler, handlers ...Handler) Router { + return grp.Add([]string{MethodOptions}, path, handler, handlers...) } // Trace registers a route for TRACE methods that performs a message loop-back // test along the path to the target resource. -func (grp *Group) Trace(path string, handler Handler, middleware ...Handler) Router { - return grp.Add([]string{MethodTrace}, path, handler, middleware...) +func (grp *Group) Trace(path string, handler Handler, handlers ...Handler) Router { + return grp.Add([]string{MethodTrace}, path, handler, handlers...) } // Patch registers a route for PATCH methods that is used to apply partial // modifications to a resource. -func (grp *Group) Patch(path string, handler Handler, middleware ...Handler) Router { - return grp.Add([]string{MethodPatch}, path, handler, middleware...) +func (grp *Group) Patch(path string, handler Handler, handlers ...Handler) Router { + return grp.Add([]string{MethodPatch}, path, handler, handlers...) } // Add allows you to specify multiple HTTP methods to register a route. -func (grp *Group) Add(methods []string, path string, handler Handler, middleware ...Handler) Router { - grp.app.register(methods, getGroupPath(grp.Prefix, path), grp, handler, middleware...) +func (grp *Group) Add(methods []string, path string, handler Handler, handlers ...Handler) Router { + grp.app.register(methods, getGroupPath(grp.Prefix, path), grp, append([]Handler{handler}, handlers...)...) if !grp.anyRouteDefined { grp.anyRouteDefined = true } @@ -171,8 +171,8 @@ func (grp *Group) Add(methods []string, path string, handler Handler, middleware } // All will register the handler on all HTTP methods -func (grp *Group) All(path string, handler Handler, middleware ...Handler) Router { - _ = grp.Add(grp.app.config.RequestMethods, path, handler, middleware...) +func (grp *Group) All(path string, handler Handler, handlers ...Handler) Router { + _ = grp.Add(grp.app.config.RequestMethods, path, handler, handlers...) return grp } @@ -183,7 +183,7 @@ func (grp *Group) All(path string, handler Handler, middleware ...Handler) Route func (grp *Group) Group(prefix string, handlers ...Handler) Router { prefix = getGroupPath(grp.Prefix, prefix) if len(handlers) > 0 { - grp.app.register([]string{methodUse}, prefix, grp, nil, handlers...) + grp.app.register([]string{methodUse}, prefix, grp, handlers...) } // Create new group diff --git a/register.go b/register.go index ab67447c5aa..06f0ba7d609 100644 --- a/register.go +++ b/register.go @@ -45,8 +45,8 @@ type Registering struct { // }) // // This method will match all HTTP verbs: GET, POST, PUT, HEAD etc... -func (r *Registering) All(handler Handler, middleware ...Handler) Register { - r.app.register([]string{methodUse}, r.path, nil, handler, middleware...) +func (r *Registering) All(handler Handler, handlers ...Handler) Register { + r.app.register([]string{methodUse}, r.path, nil, append([]Handler{handler}, handlers...)...) return r } @@ -105,8 +105,8 @@ func (r *Registering) Patch(handler Handler, middleware ...Handler) Register { } // Add allows you to specify multiple HTTP methods to register a route. -func (r *Registering) Add(methods []string, handler Handler, middleware ...Handler) Register { - r.app.register(methods, r.path, nil, handler, middleware...) +func (r *Registering) Add(methods []string, handler Handler, handlers ...Handler) Register { + r.app.register(methods, r.path, nil, append([]Handler{handler}, handlers...)...) return r } diff --git a/router.go b/router.go index 9612da170b9..e567dd834c5 100644 --- a/router.go +++ b/router.go @@ -20,18 +20,18 @@ import ( type Router interface { Use(args ...any) Router - Get(path string, handler Handler, middleware ...Handler) Router - Head(path string, handler Handler, middleware ...Handler) Router - Post(path string, handler Handler, middleware ...Handler) Router - Put(path string, handler Handler, middleware ...Handler) Router - Delete(path string, handler Handler, middleware ...Handler) Router - Connect(path string, handler Handler, middleware ...Handler) Router - Options(path string, handler Handler, middleware ...Handler) Router - Trace(path string, handler Handler, middleware ...Handler) Router - Patch(path string, handler Handler, middleware ...Handler) Router - - Add(methods []string, path string, handler Handler, middleware ...Handler) Router - All(path string, handler Handler, middleware ...Handler) Router + Get(path string, handler Handler, handlers ...Handler) Router + Head(path string, handler Handler, handlers ...Handler) Router + Post(path string, handler Handler, handlers ...Handler) Router + Put(path string, handler Handler, handlers ...Handler) Router + Delete(path string, handler Handler, handlers ...Handler) Router + Connect(path string, handler Handler, handlers ...Handler) Router + Options(path string, handler Handler, handlers ...Handler) Router + Trace(path string, handler Handler, handlers ...Handler) Router + Patch(path string, handler Handler, handlers ...Handler) Router + + Add(methods []string, path string, handler Handler, handlers ...Handler) Router + All(path string, handler Handler, handlers ...Handler) Router Group(prefix string, handlers ...Handler) Router @@ -318,10 +318,10 @@ func (*App) copyRoute(route *Route) *Route { } } -func (app *App) register(methods []string, pathRaw string, group *Group, handler Handler, middleware ...Handler) { - handlers := middleware - if handler != nil { - handlers = append(handlers, handler) +func (app *App) register(methods []string, pathRaw string, group *Group, handlers ...Handler) { + // A route requires atleast one ctx handler + if group == nil && len(handlers) == 0 { + panic(fmt.Sprintf("missing handler in route: %s\n", pathRaw)) } // Precompute path normalization ONCE diff --git a/router_test.go b/router_test.go index fe5b3429e08..eed678847b5 100644 --- a/router_test.go +++ b/router_test.go @@ -31,6 +31,39 @@ func init() { } } +func Test_Route_Handler_Order(t *testing.T) { + t.Parallel() + + app := New() + + var order []int + + handler1 := func(c Ctx) error { + order = append(order, 1) + return c.Next() + } + handler2 := func(c Ctx) error { + order = append(order, 2) + return c.Next() + } + handler3 := func(c Ctx) error { + order = append(order, 3) + return c.Next() + } + + app.Get("/test", handler1, handler2, handler3, func(c Ctx) error { + order = append(order, 4) + return c.SendStatus(200) + }) + + resp, err := app.Test(httptest.NewRequest(MethodGet, "/test", nil)) + require.NoError(t, err, "app.Test(req)") + require.Equal(t, 200, resp.StatusCode, "Status code") + + expectedOrder := []int{1, 2, 3, 4} + require.Equal(t, expectedOrder, order, "Handler order") +} + func Test_Route_Match_SameLength(t *testing.T) { t.Parallel() From 249557ca91dcdc97afc2100bc636f818ca83ce7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= Date: Sat, 22 Feb 2025 18:12:23 +0100 Subject: [PATCH 2/5] fix handler order in routing #3312 --- app.go | 36 +++++++++---------- docs/api/app.md | 24 ++++++------- docs/partials/routing/handler.md | 28 +++++++-------- register.go | 60 ++++++++++++++++---------------- 4 files changed, 74 insertions(+), 74 deletions(-) diff --git a/app.go b/app.go index 76fbcaddc06..084630bfd01 100644 --- a/app.go +++ b/app.go @@ -758,55 +758,55 @@ func (app *App) Use(args ...any) Router { // Get registers a route for GET methods that requests a representation // of the specified resource. Requests using GET should only retrieve data. -func (app *App) Get(path string, handler Handler, middleware ...Handler) Router { - return app.Add([]string{MethodGet}, path, handler, middleware...) +func (app *App) Get(path string, handler Handler, handlers ...Handler) Router { + return app.Add([]string{MethodGet}, path, handler, handlers...) } // Head registers a route for HEAD methods that asks for a response identical // to that of a GET request, but without the response body. -func (app *App) Head(path string, handler Handler, middleware ...Handler) Router { - return app.Add([]string{MethodHead}, path, handler, middleware...) +func (app *App) Head(path string, handler Handler, handlers ...Handler) Router { + return app.Add([]string{MethodHead}, path, handler, handlers...) } // Post registers a route for POST methods that is used to submit an entity to the // specified resource, often causing a change in state or side effects on the server. -func (app *App) Post(path string, handler Handler, middleware ...Handler) Router { - return app.Add([]string{MethodPost}, path, handler, middleware...) +func (app *App) Post(path string, handler Handler, handlers ...Handler) Router { + return app.Add([]string{MethodPost}, path, handler, handlers...) } // Put registers a route for PUT methods that replaces all current representations // of the target resource with the request payload. -func (app *App) Put(path string, handler Handler, middleware ...Handler) Router { - return app.Add([]string{MethodPut}, path, handler, middleware...) +func (app *App) Put(path string, handler Handler, handlers ...Handler) Router { + return app.Add([]string{MethodPut}, path, handler, handlers...) } // Delete registers a route for DELETE methods that deletes the specified resource. -func (app *App) Delete(path string, handler Handler, middleware ...Handler) Router { - return app.Add([]string{MethodDelete}, path, handler, middleware...) +func (app *App) Delete(path string, handler Handler, handlers ...Handler) Router { + return app.Add([]string{MethodDelete}, path, handler, handlers...) } // Connect registers a route for CONNECT methods that establishes a tunnel to the // server identified by the target resource. -func (app *App) Connect(path string, handler Handler, middleware ...Handler) Router { - return app.Add([]string{MethodConnect}, path, handler, middleware...) +func (app *App) Connect(path string, handler Handler, handlers ...Handler) Router { + return app.Add([]string{MethodConnect}, path, handler, handlers...) } // Options registers a route for OPTIONS methods that is used to describe the // communication options for the target resource. -func (app *App) Options(path string, handler Handler, middleware ...Handler) Router { - return app.Add([]string{MethodOptions}, path, handler, middleware...) +func (app *App) Options(path string, handler Handler, handlers ...Handler) Router { + return app.Add([]string{MethodOptions}, path, handler, handlers...) } // Trace registers a route for TRACE methods that performs a message loop-back // test along the path to the target resource. -func (app *App) Trace(path string, handler Handler, middleware ...Handler) Router { - return app.Add([]string{MethodTrace}, path, handler, middleware...) +func (app *App) Trace(path string, handler Handler, handlers ...Handler) Router { + return app.Add([]string{MethodTrace}, path, handler, handlers...) } // Patch registers a route for PATCH methods that is used to apply partial // modifications to a resource. -func (app *App) Patch(path string, handler Handler, middleware ...Handler) Router { - return app.Add([]string{MethodPatch}, path, handler, middleware...) +func (app *App) Patch(path string, handler Handler, handlers ...Handler) Router { + return app.Add([]string{MethodPatch}, path, handler, handlers...) } // Add allows you to specify multiple HTTP methods to register a route. diff --git a/docs/api/app.md b/docs/api/app.md index 6582159c0f8..23171a24a38 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -135,18 +135,18 @@ func (app *App) Route(path string) Register ```go type Register interface { - All(handler Handler, middleware ...Handler) Register - Get(handler Handler, middleware ...Handler) Register - Head(handler Handler, middleware ...Handler) Register - Post(handler Handler, middleware ...Handler) Register - Put(handler Handler, middleware ...Handler) Register - Delete(handler Handler, middleware ...Handler) Register - Connect(handler Handler, middleware ...Handler) Register - Options(handler Handler, middleware ...Handler) Register - Trace(handler Handler, middleware ...Handler) Register - Patch(handler Handler, middleware ...Handler) Register - - Add(methods []string, handler Handler, middleware ...Handler) Register + All(handler Handler, handlers ...Handler) Register + Get(handler Handler, handlers ...Handler) Register + Head(handler Handler, handlers ...Handler) Register + Post(handler Handler, handlers ...Handler) Register + Put(handler Handler, handlers ...Handler) Register + Delete(handler Handler, handlers ...Handler) Register + Connect(handler Handler, handlers ...Handler) Register + Options(handler Handler, handlers ...Handler) Register + Trace(handler Handler, handlers ...Handler) Register + Patch(handler Handler, handlers ...Handler) Register + + Add(methods []string, handler Handler, handlers ...Handler) Register Route(path string) Register } diff --git a/docs/partials/routing/handler.md b/docs/partials/routing/handler.md index 2b94a07cdae..8a0a1e09cd8 100644 --- a/docs/partials/routing/handler.md +++ b/docs/partials/routing/handler.md @@ -9,22 +9,22 @@ Registers a route bound to a specific [HTTP method](https://developer.mozilla.or ```go title="Signatures" // HTTP methods -func (app *App) Get(path string, handler Handler, middlewares ...Handler) Router -func (app *App) Head(path string, handler Handler, middlewares ...Handler) Router -func (app *App) Post(path string, handler Handler, middlewares ...Handler) Router -func (app *App) Put(path string, handler Handler, middlewares ...Handler) Router -func (app *App) Delete(path string, handler Handler, middlewares ...Handler) Router -func (app *App) Connect(path string, handler Handler, middlewares ...Handler) Router -func (app *App) Options(path string, handler Handler, middlewares ...Handler) Router -func (app *App) Trace(path string, handler Handler, middlewares ...Handler) Router -func (app *App) Patch(path string, handler Handler, middlewares ...Handler) Router +func (app *App) Get(path string, handler Handler, handlers ...Handler) Router +func (app *App) Head(path string, handler Handler, handlers ...Handler) Router +func (app *App) Post(path string, handler Handler, handlers ...Handler) Router +func (app *App) Put(path string, handler Handler, handlers ...Handler) Router +func (app *App) Delete(path string, handler Handler, handlers ...Handler) Router +func (app *App) Connect(path string, handler Handler, handlers ...Handler) Router +func (app *App) Options(path string, handler Handler, handlers ...Handler) Router +func (app *App) Trace(path string, handler Handler, handlers ...Handler) Router +func (app *App) Patch(path string, handler Handler, handlers ...Handler) Router // Add allows you to specify a method as value -func (app *App) Add(method, path string, handler Handler, middlewares ...Handler) Router +func (app *App) Add(method, path string, handler Handler, handlers ...Handler) Router // All will register the route on all HTTP methods // Almost the same as app.Use but not bound to prefixes -func (app *App) All(path string, handler Handler, middlewares ...Handler) Router +func (app *App) All(path string, handler Handler, handlers ...Handler) Router ``` ```go title="Examples" @@ -47,9 +47,9 @@ Can be used for middleware packages and prefix catchers. These routes will only func (app *App) Use(args ...any) Router // Different usage variations -func (app *App) Use(handler Handler, middlewares ...Handler) Router -func (app *App) Use(path string, handler Handler, middlewares ...Handler) Router -func (app *App) Use(paths []string, handler Handler, middlewares ...Handler) Router +func (app *App) Use(handler Handler, handlers ...Handler) Router +func (app *App) Use(path string, handler Handler, handlers ...Handler) Router +func (app *App) Use(paths []string, handler Handler, handlers ...Handler) Router func (app *App) Use(path string, app *App) Router ``` diff --git a/register.go b/register.go index 06f0ba7d609..c7e8a12a8b6 100644 --- a/register.go +++ b/register.go @@ -6,18 +6,18 @@ package fiber // Register defines all router handle interface generate by Route(). type Register interface { - All(handler Handler, middleware ...Handler) Register - Get(handler Handler, middleware ...Handler) Register - Head(handler Handler, middleware ...Handler) Register - Post(handler Handler, middleware ...Handler) Register - Put(handler Handler, middleware ...Handler) Register - Delete(handler Handler, middleware ...Handler) Register - Connect(handler Handler, middleware ...Handler) Register - Options(handler Handler, middleware ...Handler) Register - Trace(handler Handler, middleware ...Handler) Register - Patch(handler Handler, middleware ...Handler) Register - - Add(methods []string, handler Handler, middleware ...Handler) Register + All(handler Handler, handlers ...Handler) Register + Get(handler Handler, handlers ...Handler) Register + Head(handler Handler, handlers ...Handler) Register + Post(handler Handler, handlers ...Handler) Register + Put(handler Handler, handlers ...Handler) Register + Delete(handler Handler, handlers ...Handler) Register + Connect(handler Handler, handlers ...Handler) Register + Options(handler Handler, handlers ...Handler) Register + Trace(handler Handler, handlers ...Handler) Register + Patch(handler Handler, handlers ...Handler) Register + + Add(methods []string, handler Handler, handlers ...Handler) Register Route(path string) Register } @@ -52,56 +52,56 @@ func (r *Registering) All(handler Handler, handlers ...Handler) Register { // Get registers a route for GET methods that requests a representation // of the specified resource. Requests using GET should only retrieve data. -func (r *Registering) Get(handler Handler, middleware ...Handler) Register { - r.app.Add([]string{MethodGet}, r.path, handler, middleware...) +func (r *Registering) Get(handler Handler, handlers ...Handler) Register { + r.app.Add([]string{MethodGet}, r.path, handler, handlers...) return r } // Head registers a route for HEAD methods that asks for a response identical // to that of a GET request, but without the response body. -func (r *Registering) Head(handler Handler, middleware ...Handler) Register { - return r.Add([]string{MethodHead}, handler, middleware...) +func (r *Registering) Head(handler Handler, handlers ...Handler) Register { + return r.Add([]string{MethodHead}, handler, handlers...) } // Post registers a route for POST methods that is used to submit an entity to the // specified resource, often causing a change in state or side effects on the server. -func (r *Registering) Post(handler Handler, middleware ...Handler) Register { - return r.Add([]string{MethodPost}, handler, middleware...) +func (r *Registering) Post(handler Handler, handlers ...Handler) Register { + return r.Add([]string{MethodPost}, handler, handlers...) } // Put registers a route for PUT methods that replaces all current representations // of the target resource with the request payload. -func (r *Registering) Put(handler Handler, middleware ...Handler) Register { - return r.Add([]string{MethodPut}, handler, middleware...) +func (r *Registering) Put(handler Handler, handlers ...Handler) Register { + return r.Add([]string{MethodPut}, handler, handlers...) } // Delete registers a route for DELETE methods that deletes the specified resource. -func (r *Registering) Delete(handler Handler, middleware ...Handler) Register { - return r.Add([]string{MethodDelete}, handler, middleware...) +func (r *Registering) Delete(handler Handler, handlers ...Handler) Register { + return r.Add([]string{MethodDelete}, handler, handlers...) } // Connect registers a route for CONNECT methods that establishes a tunnel to the // server identified by the target resource. -func (r *Registering) Connect(handler Handler, middleware ...Handler) Register { - return r.Add([]string{MethodConnect}, handler, middleware...) +func (r *Registering) Connect(handler Handler, handlers ...Handler) Register { + return r.Add([]string{MethodConnect}, handler, handlers...) } // Options registers a route for OPTIONS methods that is used to describe the // communication options for the target resource. -func (r *Registering) Options(handler Handler, middleware ...Handler) Register { - return r.Add([]string{MethodOptions}, handler, middleware...) +func (r *Registering) Options(handler Handler, handlers ...Handler) Register { + return r.Add([]string{MethodOptions}, handler, handlers...) } // Trace registers a route for TRACE methods that performs a message loop-back // test along the r.Path to the target resource. -func (r *Registering) Trace(handler Handler, middleware ...Handler) Register { - return r.Add([]string{MethodTrace}, handler, middleware...) +func (r *Registering) Trace(handler Handler, handlers ...Handler) Register { + return r.Add([]string{MethodTrace}, handler, handlers...) } // Patch registers a route for PATCH methods that is used to apply partial // modifications to a resource. -func (r *Registering) Patch(handler Handler, middleware ...Handler) Register { - return r.Add([]string{MethodPatch}, handler, middleware...) +func (r *Registering) Patch(handler Handler, handlers ...Handler) Register { + return r.Add([]string{MethodPatch}, handler, handlers...) } // Add allows you to specify multiple HTTP methods to register a route. From e7960995e082a6f4246c05cba33315a63750f7ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= Date: Sat, 22 Feb 2025 18:15:58 +0100 Subject: [PATCH 3/5] fix handler order in routing #3312 --- router.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/router.go b/router.go index e567dd834c5..2c149893526 100644 --- a/router.go +++ b/router.go @@ -319,9 +319,10 @@ func (*App) copyRoute(route *Route) *Route { } func (app *App) register(methods []string, pathRaw string, group *Group, handlers ...Handler) { - // A route requires atleast one ctx handler - if group == nil && len(handlers) == 0 { - panic(fmt.Sprintf("missing handler in route: %s\n", pathRaw)) + // A regular route requires at least one ctx handler + isMount := group != nil && group.app != app + if len(handlers) == 0 && !isMount { + panic(fmt.Sprintf("missing handler/middleware in route: %s\n", pathRaw)) } // Precompute path normalization ONCE @@ -349,11 +350,6 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler panic(fmt.Sprintf("add: invalid http method %s\n", method)) } - isMount := group != nil && group.app != app - if len(handlers) == 0 && !isMount { - panic(fmt.Sprintf("missing handler/middleware in route: %s\n", pathRaw)) - } - isUse := method == methodUse isStar := pathClean == "/*" isRoot := pathClean == "/" From 49371049989664b349916512eabf3778144c252f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= Date: Sat, 22 Feb 2025 18:35:43 +0100 Subject: [PATCH 4/5] fix handler order in routing #3312 --- app_test.go | 6 ++++++ hooks_test.go | 4 ++++ mount.go | 4 ++-- router.go | 13 ++++++++++--- router_test.go | 28 ++++++++++++++++++++++------ 5 files changed, 44 insertions(+), 11 deletions(-) diff --git a/app_test.go b/app_test.go index d64f9a68131..1c581e3abcb 100644 --- a/app_test.go +++ b/app_test.go @@ -482,6 +482,8 @@ func Test_App_Use_Params(t *testing.T) { defer func() { if err := recover(); err != nil { require.Equal(t, "use: invalid handler func()\n", fmt.Sprintf("%v", err)) + } else { + t.Fatalf("expected panic, but no panic occurred") } }() @@ -1058,6 +1060,8 @@ func Test_App_Group_Invalid(t *testing.T) { defer func() { if err := recover(); err != nil { require.Equal(t, "use: invalid handler int\n", fmt.Sprintf("%v", err)) + } else { + t.Fatalf("expected panic, but no panic occurred") } }() New().Group("/").Use(1) @@ -1288,6 +1292,8 @@ func Test_App_Init_Error_View(t *testing.T) { defer func() { if err := recover(); err != nil { require.Equal(t, "implement me", fmt.Sprintf("%v", err)) + } else { + t.Fatalf("expected panic, but no panic occurred") } }() diff --git a/hooks_test.go b/hooks_test.go index f96f5707064..f719655199b 100644 --- a/hooks_test.go +++ b/hooks_test.go @@ -86,6 +86,8 @@ func Test_Hook_OnName_Error(t *testing.T) { defer func() { if err := recover(); err != nil { require.Equal(t, "unknown error", fmt.Sprintf("%v", err)) + } else { + t.Fatalf("expected panic, but no panic occurred") } }() @@ -170,6 +172,8 @@ func Test_Hook_OnGroupName_Error(t *testing.T) { defer func() { if err := recover(); err != nil { require.Equal(t, "unknown error", fmt.Sprintf("%v", err)) + } else { + t.Fatalf("expected panic, but no panic occurred") } }() diff --git a/mount.go b/mount.go index d97b226dfc6..f05ec82d1d0 100644 --- a/mount.go +++ b/mount.go @@ -55,7 +55,7 @@ func (app *App) mount(prefix string, subApp *App) Router { // register mounted group mountGroup := &Group{Prefix: prefix, app: subApp} - app.register([]string{methodUse}, prefix, mountGroup, nil) + app.register([]string{methodUse}, prefix, mountGroup) // Execute onMount hooks if err := subApp.hooks.executeOnMountHooks(app); err != nil { @@ -85,7 +85,7 @@ func (grp *Group) mount(prefix string, subApp *App) Router { // register mounted group mountGroup := &Group{Prefix: groupPath, app: subApp} - grp.app.register([]string{methodUse}, groupPath, mountGroup, nil) + grp.app.register([]string{methodUse}, groupPath, mountGroup) // Execute onMount hooks if err := subApp.hooks.executeOnMountHooks(grp.app); err != nil { diff --git a/router.go b/router.go index 2c149893526..05b3265a990 100644 --- a/router.go +++ b/router.go @@ -320,9 +320,14 @@ func (*App) copyRoute(route *Route) *Route { func (app *App) register(methods []string, pathRaw string, group *Group, handlers ...Handler) { // A regular route requires at least one ctx handler - isMount := group != nil && group.app != app - if len(handlers) == 0 && !isMount { - panic(fmt.Sprintf("missing handler/middleware in route: %s\n", pathRaw)) + if len(handlers) == 0 && group == nil { + panic(fmt.Sprintf("missing handler/middleware in route: %s", pathRaw)) + } + // No nil handlers allowed + for _, h := range handlers { + if nil == h { + panic(fmt.Sprintf("nil handler in route: %s", pathRaw)) + } } // Precompute path normalization ONCE @@ -344,6 +349,8 @@ func (app *App) register(methods []string, pathRaw string, group *Group, handler parsedRaw := parseRoute(pathRaw, app.customConstraints...) parsedPretty := parseRoute(pathPretty, app.customConstraints...) + isMount := group != nil && group.app != app + for _, method := range methods { method = utils.ToUpper(method) if method != methodUse && app.methodInt(method) == -1 { diff --git a/router_test.go b/router_test.go index eed678847b5..86863754a80 100644 --- a/router_test.go +++ b/router_test.go @@ -327,12 +327,28 @@ func Test_Router_Register_Missing_Handler(t *testing.T) { t.Parallel() app := New() - defer func() { - if err := recover(); err != nil { - require.Equal(t, "missing handler/middleware in route: /doe\n", fmt.Sprintf("%v", err)) - } - }() - app.register([]string{"USE"}, "/doe", nil, nil) + + t.Run("No Handler", func(t *testing.T) { + defer func() { + if err := recover(); err != nil { + require.Equal(t, "missing handler/middleware in route: /doe", fmt.Sprintf("%v", err)) + } else { + t.Fatalf("expected panic, but no panic occurred") + } + }() + app.register([]string{"USE"}, "/doe", nil) + }) + + t.Run("Nil Handler", func(t *testing.T) { + defer func() { + if err := recover(); err != nil { + require.Equal(t, "nil handler in route: /doe", fmt.Sprintf("%v", err)) + } else { + t.Fatalf("expected panic, but no panic occurred") + } + }() + app.register([]string{"USE"}, "/doe", nil, nil) + }) } func Test_Ensure_Router_Interface_Implementation(t *testing.T) { From 3a588a2cfc4f9ec43d5a93205498588e65c1d919 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= Date: Sat, 22 Feb 2025 18:53:15 +0100 Subject: [PATCH 5/5] fix handler order in routing #3312 --- app_test.go | 40 ++++++++++++---------------------------- hooks_test.go | 24 ++++++------------------ router.go | 4 ++-- router_test.go | 27 ++++++++++----------------- 4 files changed, 30 insertions(+), 65 deletions(-) diff --git a/app_test.go b/app_test.go index 1c581e3abcb..162ba8703b9 100644 --- a/app_test.go +++ b/app_test.go @@ -479,16 +479,10 @@ func Test_App_Use_Params(t *testing.T) { require.NoError(t, err, "app.Test(req)") require.Equal(t, 200, resp.StatusCode, "Status code") - defer func() { - if err := recover(); err != nil { - require.Equal(t, "use: invalid handler func()\n", fmt.Sprintf("%v", err)) - } else { - t.Fatalf("expected panic, but no panic occurred") - } - }() - - app.Use("/:param/*", func() { - // this should panic + require.PanicsWithValue(t, "use: invalid handler func()\n", func() { + app.Use("/:param/*", func() { + // this should panic + }) }) } @@ -1057,14 +1051,10 @@ func Test_App_Mixed_Routes_WithSameLen(t *testing.T) { func Test_App_Group_Invalid(t *testing.T) { t.Parallel() - defer func() { - if err := recover(); err != nil { - require.Equal(t, "use: invalid handler int\n", fmt.Sprintf("%v", err)) - } else { - t.Fatalf("expected panic, but no panic occurred") - } - }() - New().Group("/").Use(1) + + require.PanicsWithValue(t, "use: invalid handler int\n", func() { + New().Group("/").Use(1) + }) } func Test_App_Group(t *testing.T) { @@ -1289,16 +1279,10 @@ func Test_App_Init_Error_View(t *testing.T) { t.Parallel() app := New(Config{Views: invalidView{}}) - defer func() { - if err := recover(); err != nil { - require.Equal(t, "implement me", fmt.Sprintf("%v", err)) - } else { - t.Fatalf("expected panic, but no panic occurred") - } - }() - - err := app.config.Views.Render(nil, "", nil) - require.NoError(t, err) + require.PanicsWithValue(t, "implement me", func() { + //nolint:errcheck // not needed + _ = app.config.Views.Render(nil, "", nil) + }) } // go test -run Test_App_Stack diff --git a/hooks_test.go b/hooks_test.go index f719655199b..21426b7bbbe 100644 --- a/hooks_test.go +++ b/hooks_test.go @@ -2,7 +2,6 @@ package fiber import ( "errors" - "fmt" "testing" "time" @@ -83,19 +82,14 @@ func Test_Hook_OnName(t *testing.T) { func Test_Hook_OnName_Error(t *testing.T) { t.Parallel() app := New() - defer func() { - if err := recover(); err != nil { - require.Equal(t, "unknown error", fmt.Sprintf("%v", err)) - } else { - t.Fatalf("expected panic, but no panic occurred") - } - }() app.Hooks().OnName(func(_ Route) error { return errors.New("unknown error") }) - app.Get("/", testSimpleHandler).Name("index") + require.PanicsWithError(t, "unknown error", func() { + app.Get("/", testSimpleHandler).Name("index") + }) } func Test_Hook_OnGroup(t *testing.T) { @@ -169,20 +163,14 @@ func Test_Hook_OnGroupName(t *testing.T) { func Test_Hook_OnGroupName_Error(t *testing.T) { t.Parallel() app := New() - defer func() { - if err := recover(); err != nil { - require.Equal(t, "unknown error", fmt.Sprintf("%v", err)) - } else { - t.Fatalf("expected panic, but no panic occurred") - } - }() app.Hooks().OnGroupName(func(_ Group) error { return errors.New("unknown error") }) - grp := app.Group("/x").Name("x.") - grp.Get("/test", testSimpleHandler) + require.PanicsWithError(t, "unknown error", func() { + _ = app.Group("/x").Name("x.") + }) } func Test_Hook_OnShutdown(t *testing.T) { diff --git a/router.go b/router.go index 05b3265a990..a14d2edd749 100644 --- a/router.go +++ b/router.go @@ -321,12 +321,12 @@ func (*App) copyRoute(route *Route) *Route { func (app *App) register(methods []string, pathRaw string, group *Group, handlers ...Handler) { // A regular route requires at least one ctx handler if len(handlers) == 0 && group == nil { - panic(fmt.Sprintf("missing handler/middleware in route: %s", pathRaw)) + panic(fmt.Sprintf("missing handler/middleware in route: %s\n", pathRaw)) } // No nil handlers allowed for _, h := range handlers { if nil == h { - panic(fmt.Sprintf("nil handler in route: %s", pathRaw)) + panic(fmt.Sprintf("nil handler in route: %s\n", pathRaw)) } } diff --git a/router_test.go b/router_test.go index 86863754a80..0ac0c212ca2 100644 --- a/router_test.go +++ b/router_test.go @@ -7,7 +7,6 @@ package fiber import ( "encoding/json" "errors" - "fmt" "io" "net/http" "net/http/httptest" @@ -329,25 +328,19 @@ func Test_Router_Register_Missing_Handler(t *testing.T) { app := New() t.Run("No Handler", func(t *testing.T) { - defer func() { - if err := recover(); err != nil { - require.Equal(t, "missing handler/middleware in route: /doe", fmt.Sprintf("%v", err)) - } else { - t.Fatalf("expected panic, but no panic occurred") - } - }() - app.register([]string{"USE"}, "/doe", nil) + t.Parallel() + + require.PanicsWithValue(t, "missing handler/middleware in route: /doe\n", func() { + app.register([]string{"USE"}, "/doe", nil) + }) }) t.Run("Nil Handler", func(t *testing.T) { - defer func() { - if err := recover(); err != nil { - require.Equal(t, "nil handler in route: /doe", fmt.Sprintf("%v", err)) - } else { - t.Fatalf("expected panic, but no panic occurred") - } - }() - app.register([]string{"USE"}, "/doe", nil, nil) + t.Parallel() + + require.PanicsWithValue(t, "nil handler in route: /doe\n", func() { + app.register([]string{"USE"}, "/doe", nil, nil) + }) }) }