GOMODULES
/
fiber
mirror of https://github.com/gofiber/fiber.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

🐛 [Bug-Fix]: Mounted subapps don't work correctly if parent app attached … (#2331) 

* 🐛 [Bug]: Mounted subapps don't work correctly if parent app attached additional middlewares after mounting (v2.40.1 bug) #2233
pull/2416/head
RW 2023-04-13 14:19:04 +02:00 committed by GitHub
parent 2237e9c511
commit 65e0ce285a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 196 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
app.go
View File

@ -1085,12 +1085,7 @@ func (app *App) startupProcess() *App {
app.mutex.Lock()
defer app.mutex.Unlock()
// add routes of sub-apps
app.mountFields.subAppsRoutesAdded.Do(func() {
app.appendSubAppLists(app.mountFields.appList)
app.addSubAppsRoutes(app.mountFields.appList)
app.generateAppListKeys()
})
app.mountStartupProcess()
// build route tree stack
app.buildTree()

8
helpers.go
View File

@ -93,7 +93,7 @@ func (app *App) methodExist(ctx *Ctx) bool {
continue
}
// Reset stack index
ctx.indexRoute = -1
indexRoute := -1
tree, ok := ctx.app.treeStack[i][ctx.treePath]
if !ok {
tree = ctx.app.treeStack[i][""]
@ -101,11 +101,11 @@ func (app *App) methodExist(ctx *Ctx) bool {
// Get stack length
lenr := len(tree) - 1
// Loop over the route stack starting from previous index
for ctx.indexRoute < lenr {
for indexRoute < lenr {
// Increment route index
ctx.indexRoute++
indexRoute++
// Get *Route
route := tree[ctx.indexRoute]
route := tree[indexRoute]
// Skip use routes
if route.use {
continue

134
mount.go
View File

@ -19,6 +19,8 @@ type mountFields struct {
appListKeys []string
// check added routes of sub-apps
subAppsRoutesAdded sync.Once
// check mounted sub-apps
subAppsProcessed sync.Once
// Prefix of app if it was mounted
mountPath string
}
@ -36,22 +38,26 @@ func newMountFields(app *App) *mountFields {
// compose them as a single service using Mount. The fiber's error handler and
// any of the fiber's sub apps are added to the application's error handlers
// to be invoked on errors that happen within the prefix route.
func (app *App) Mount(prefix string, fiber *App) Router {
func (app *App) Mount(prefix string, subApp *App) Router {
prefix = strings.TrimRight(prefix, "/")
if prefix == "" {
prefix = "/"
}
// Support for configs of mounted-apps and sub-mounted-apps
for mountedPrefixes, subApp := range fiber.mountFields.appList {
for mountedPrefixes, subApp := range subApp.mountFields.appList {
path := getGroupPath(prefix, mountedPrefixes)
subApp.mountFields.mountPath = path
app.mountFields.appList[path] = subApp
}
// register mounted group
mountGroup := &Group{Prefix: prefix, app: subApp}
app.register(methodUse, prefix, mountGroup)
// Execute onMount hooks
if err := fiber.hooks.executeOnMountHooks(app); err != nil {
if err := subApp.hooks.executeOnMountHooks(app); err != nil {
panic(err)
}
@ -61,7 +67,7 @@ func (app *App) Mount(prefix string, fiber *App) Router {
// Mount attaches another app instance as a sub-router along a routing path.
// It's very useful to split up a large API as many independent routers and
// compose them as a single service using Mount.
func (grp *Group) Mount(prefix string, fiber *App) Router {
func (grp *Group) Mount(prefix string, subApp *App) Router {
groupPath := getGroupPath(grp.Prefix, prefix)
groupPath = strings.TrimRight(groupPath, "/")
if groupPath == "" {
@ -69,15 +75,19 @@ func (grp *Group) Mount(prefix string, fiber *App) Router {
}
// Support for configs of mounted-apps and sub-mounted-apps
for mountedPrefixes, subApp := range fiber.mountFields.appList {
for mountedPrefixes, subApp := range subApp.mountFields.appList {
path := getGroupPath(groupPath, mountedPrefixes)
subApp.mountFields.mountPath = path
grp.app.mountFields.appList[path] = subApp
}
// register mounted group
mountGroup := &Group{Prefix: groupPath, app: subApp}
grp.app.register(methodUse, groupPath, mountGroup)
// Execute onMount hooks
if err := fiber.hooks.executeOnMountHooks(grp.app); err != nil {
if err := subApp.hooks.executeOnMountHooks(grp.app); err != nil {
panic(err)
}
@ -89,6 +99,26 @@ func (app *App) MountPath() string {
return app.mountFields.mountPath
}
// hasMountedApps Checks if there are any mounted apps in the current application.
func (app *App) hasMountedApps() bool {
return len(app.mountFields.appList) > 1
}
// mountStartupProcess Handles the startup process of mounted apps by appending sub-app routes, generating app list keys, and processing sub-app routes.
func (app *App) mountStartupProcess() {
if app.hasMountedApps() {
// add routes of sub-apps
app.mountFields.subAppsProcessed.Do(func() {
app.appendSubAppLists(app.mountFields.appList)
app.generateAppListKeys()
})
// adds the routes of the sub-apps to the current application.
app.mountFields.subAppsRoutesAdded.Do(func() {
app.processSubAppsRoutes()
})
}
}
// generateAppListKeys generates app list keys for Render, should work after appendSubAppLists
func (app *App) generateAppListKeys() {
for key := range app.mountFields.appList {
@ -102,14 +132,20 @@ func (app *App) generateAppListKeys() {
// appendSubAppLists supports nested for sub apps
func (app *App) appendSubAppLists(appList map[string]*App, parent ...string) {
// Optimize: Cache parent prefix
parentPrefix := ""
if len(parent) > 0 {
parentPrefix = parent[0]
}
for prefix, subApp := range appList {
// skip real app
if prefix == "" {
continue
}
if len(parent) > 0 {
prefix = getGroupPath(parent[0], prefix)
if parentPrefix != "" {
prefix = getGroupPath(parentPrefix, prefix)
}
if _, ok := app.mountFields.appList[prefix]; !ok {
@ -123,27 +159,75 @@ func (app *App) appendSubAppLists(appList map[string]*App, parent ...string) {
}
}
// addSubAppsRoutes adds routes of sub apps nestedly when to start the server
func (app *App) addSubAppsRoutes(appList map[string]*App, parent ...string) {
for prefix, subApp := range appList {
// processSubAppsRoutes adds routes of sub-apps recursively when the server is started
func (app *App) processSubAppsRoutes() {
for prefix, subApp := range app.mountFields.appList {
// skip real app
if prefix == "" {
continue
}
if len(parent) > 0 {
prefix = getGroupPath(parent[0], prefix)
// process the inner routes
if subApp.hasMountedApps() {
subApp.mountFields.subAppsRoutesAdded.Do(func() {
subApp.processSubAppsRoutes()
})
}
// add routes
stack := subApp.stack
for m := range stack {
for r := range stack[m] {
route := app.copyRoute(stack[m][r])
app.addRoute(route.Method, app.addPrefixToRoute(prefix, route), true)
}
}
atomic.AddUint32(&app.handlersCount, subApp.handlersCount)
}
var handlersCount uint32
// Iterate over the stack of the parent app
for m := range app.stack {
// Keep track of the position shift caused by adding routes for mounted apps
var positionShift uint32
// Iterate over each route in the stack
stackLen := len(app.stack[m])
for i := 0; i < stackLen; i++ {
route := app.stack[m][i]
// Check if the route has a mounted app
if !route.mount {
// If not, update the route's position and continue
route.pos += positionShift
if !route.use || (route.use && m == 0) {
handlersCount += uint32(len(route.Handlers))
}
continue
}
// Update the position shift to account for the mounted app's routes
positionShift = route.pos
// Create a slice to hold the sub-app's routes
subRoutes := make([]*Route, len(route.group.app.stack[m]))
// Iterate over the sub-app's routes
for j, subAppRoute := range route.group.app.stack[m] {
// Clone the sub-app's route
subAppRouteClone := app.copyRoute(subAppRoute)
// Add the parent route's path as a prefix to the sub-app's route
app.addPrefixToRoute(route.path, subAppRouteClone)
// Add the cloned sub-app's route to the slice of sub-app routes
subRoutes[j] = subAppRouteClone
}
// Insert the sub-app's routes into the parent app's stack
newStack := make([]*Route, len(app.stack[m])+len(subRoutes)-1)
copy(newStack[:i], app.stack[m][:i])
copy(newStack[i:i+len(subRoutes)], subRoutes)
copy(newStack[i+len(subRoutes):], app.stack[m][i+1:])
app.stack[m] = newStack
// Decrease the parent app's route count to account for the mounted app's original route
atomic.AddUint32(&app.routesCount, ^uint32(0))
i--
// Increase the parent app's route count to account for the sub-app's routes
atomic.AddUint32(&app.routesCount, uint32(len(subRoutes)))
// Mark the parent app's routes as refreshed
app.routesRefreshed = true
// update stackLen after appending subRoutes to app.stack[m]
stackLen = len(app.stack[m])
}
}
atomic.StoreUint32(&app.handlersCount, handlersCount)
}

50
mount_test.go
View File

@ -88,6 +88,56 @@ func Test_App_Mount_Nested(t *testing.T) {
utils.AssertEqual(t, 200, resp.StatusCode, "Status code")
utils.AssertEqual(t, uint32(6), app.handlersCount)
utils.AssertEqual(t, uint32(6), app.routesCount)
}
// go test -run Test_App_Mount_Express_Behavior
func Test_App_Mount_Express_Behavior(t *testing.T) {
t.Parallel()
createTestHandler := func(body string) func(c *Ctx) error {
return func(c *Ctx) error {
return c.SendString(body)
}
}
testEndpoint := func(app *App, route, expectedBody string) {
resp, err := app.Test(httptest.NewRequest(MethodGet, route, nil))
utils.AssertEqual(t, nil, err, "app.Test(req)")
body, err := io.ReadAll(resp.Body)
utils.AssertEqual(t, nil, err)
utils.AssertEqual(t, expectedBody, string(body), "Response body")
}
app := New()
subApp := New()
// app setup
{
subApp.Get("/hello", createTestHandler("subapp hello!"))
subApp.Get("/world", createTestHandler("subapp world!")) // <- wins
app.Get("/hello", createTestHandler("app hello!")) // <- wins
app.Mount("/", subApp) // <- subApp registration
app.Get("/world", createTestHandler("app world!"))
app.Get("/bar", createTestHandler("app bar!"))
subApp.Get("/bar", createTestHandler("subapp bar!")) // <- wins
subApp.Get("/foo", createTestHandler("subapp foo!")) // <- wins
app.Get("/foo", createTestHandler("app foo!"))
// 404 Handler
app.Use(func(c *Ctx) error {
return c.SendStatus(StatusNotFound)
})
}
// expectation check
testEndpoint(app, "/world", "subapp world!")
testEndpoint(app, "/hello", "app hello!")
testEndpoint(app, "/bar", "subapp bar!")
testEndpoint(app, "/foo", "subapp foo!")
testEndpoint(app, "/unknown", ErrNotFound.Message)
utils.AssertEqual(t, uint32(17), app.handlersCount)
utils.AssertEqual(t, uint32(16+9), app.routesCount)
}
// go test -run Test_App_MountPath

46
router.go
View File

@ -46,9 +46,11 @@ type Router interface {
// Route is a struct that holds all metadata for each registered handler.
type Route struct {
// always keep in sync with the copy method "app.copyRoute"
// Data for routing
pos uint32 // Position in stack -> important for the sort of the matched routes
use bool // USE matches path prefixes
mount bool // Indicated a mounted app on a specific route
star bool // Path equals '*'
root bool // Path equals '/'
path string // Prettified path
@ -105,18 +107,25 @@ func (app *App) next(c *Ctx) (bool, error) {
if !ok {
tree = app.treeStack[c.methodINT][""]
}
lenr := len(tree) - 1
lenTree := len(tree) - 1
// Loop over the route stack starting from previous index
for c.indexRoute < lenr {
for c.indexRoute < lenTree {
// Increment route index
c.indexRoute++
// Get *Route
route := tree[c.indexRoute]
var match bool
var err error
// skip for mounted apps
if route.mount {
continue
}
// Check if it matches the request path
match := route.match(c.detectionPath, c.path, &c.values)
match = route.match(c.detectionPath, c.path, &c.values)
if !match {
// No match, next route
continue
@ -131,7 +140,9 @@ func (app *App) next(c *Ctx) (bool, error) {
// Execute first handler of route
c.indexHandler = 0
err := route.Handlers[0](c)
if len(route.Handlers) > 0 {
err = route.Handlers[0](c)
}
return match, err // Stop scanning the stack
}
@ -194,15 +205,19 @@ func (app *App) addPrefixToRoute(prefix string, route *Route) *Route {
func (*App) copyRoute(route *Route) *Route {
return &Route{
// Router booleans
use: route.use,
star: route.star,
root: route.root,
use: route.use,
mount: route.mount,
star: route.star,
root: route.root,
// Path data
path: route.path,
routeParser: route.routeParser,
Params: route.Params,
// misc
pos: route.pos,
// Public data
Path: route.Path,
Method: route.Method,
@ -217,8 +232,10 @@ func (app *App) register(method, pathRaw string, group *Group, handlers ...Handl
if method != methodUse && app.methodInt(method) == -1 {
panic(fmt.Sprintf("add: invalid http method %s\n", method))
}
// is mounted app
isMount := group != nil && group.app != app
// A route requires atleast one ctx handler
if len(handlers) == 0 {
if len(handlers) == 0 && !isMount {
panic(fmt.Sprintf("missing handler in route: %s\n", pathRaw))
}
// Cannot have an empty path
@ -252,9 +269,10 @@ func (app *App) register(method, pathRaw string, group *Group, handlers ...Handl
// Create route metadata without pointer
route := Route{
// Router booleans
use: isUse,
star: isStar,
root: isRoot,
use: isUse,
mount: isMount,
star: isStar,
root: isRoot,
// Path data
path: RemoveEscapeChar(pathPretty),
@ -278,11 +296,11 @@ func (app *App) register(method, pathRaw string, group *Group, handlers ...Handl
for _, m := range app.config.RequestMethods {
// Create a route copy to avoid duplicates during compression
r := route
app.addRoute(m, &r)
app.addRoute(m, &r, isMount)
}
} else {
// Add route to stack
app.addRoute(method, &route)
app.addRoute(method, &route, isMount)
}
return app
}
@ -438,7 +456,7 @@ func (app *App) addRoute(method string, route *Route, isMounted ...bool) {
// prevent identically route registration
l := len(app.stack[m])
if l > 0 && app.stack[m][l-1].Path == route.Path && route.use == app.stack[m][l-1].use {
if l > 0 && app.stack[m][l-1].Path == route.Path && route.use == app.stack[m][l-1].use && !route.mount && !app.stack[m][l-1].mount {
preRoute := app.stack[m][l-1]
preRoute.Handlers = append(preRoute.Handlers, route.Handlers...)
} else {