fix handler order in routing

#3312
gofiber · Feb 22, 2025 · 4937104 · 4937104
1 parent e796099
commit 4937104
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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) {