Fiber: 🐞 Group middleware does not work when route has parameters

[jos-]

Thanks for the great framework. However, I encountered the following bug:

Fiber version/commit
v1.9.6

Issue description
Group middleware is not executed when the group route has parameters.

Steps to reproduce
See code below and compare http://localhost:8080/a with http://localhost:8080/b
myMiddlewareA is called, but myMiddlewareB not.

Code snippet

package main

import (
    "log"
    "github.com/gofiber/fiber"
)

func myMiddlewareA(c *fiber.Ctx) {
    log.Println("myMiddlewareA called")
    c.Next()
}

func myMiddlewareB(c *fiber.Ctx) {
    log.Println("myMiddlewareB called") // Never happens
    c.Next()
}

func main() {
    app := fiber.New()

    groupA := app.Group("/a", myMiddlewareA)
    groupA.Get("/", func(c *fiber.Ctx) {
        c.Send("Group A") // Works with middleware
    })

    groupB := app.Group("/:somerouteparameter", myMiddlewareB)
    groupB.Get("/", func(c *fiber.Ctx) {
        c.Send("Group B") // The response is returned, but the middleware is not called
    })

    log.Fatal(app.Listen(8080))
}

[welcome[bot]]

Thanks for opening your first issue here! 🎉 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

[rof20004]

Why you need your group B start with a parameter?

In terms of design I think this is not correct, because if your group can be a parameter, than groupB can be many groups dinamycally

[jos-]

This was just an example. If the group doesn't start with a parameter, but is e.g. /v1/users/:id, the same problem occurs.

Starting a group with a parameter can be useful if it's not a root-level group.

[rof20004]

@jos-

I disagree, I think a group need a name to identify it, than it needs to be a static name. In my opinion.

[jos-]

Consider part of the GitHub api (https://developer.github.com/v3/repos/branches/) as an example. It has the following routes:

/repos/:owner/:repo/branches
/repos/:owner/:repo/branches/:branch
/repos/:owner/:repo/branches/:branch/protection
/repos/:owner/:repo/branches/:branch/protection/required_status_checks
/repos/:owner/:repo/branches/:branch/protection/required_pull_request_reviews
/repos/:owner/:repo/branches/:branch/protection/required_signatures
/repos/:owner/:repo/branches/:branch/protection/enforce_admins
etc.

Here it can be very useful to have a route group for /repos/:owner or /repos/:owner/:repo with middleware that checks if the current user is authorized to access these routes.

[rof20004]

Like I said, this is a design. What are you doing is a code design, not a rest application design. The Github example is a rest uri design, not a code application design.

https://restfulapi.net/rest-api-design-tutorial-with-example/#model-uris

Your code is this:

groupB := app.Group("/:somerouteparameter", myMiddlewareB)

If you analize, /:somerouteparameter can be /a, ok? But /a is your first group, your rest uri design is confused.

[rof20004]

I code like this, using your example:

groupB := app.Group("/v1/users", myMiddlewareB)
groupB.Get("/:id", func(c *fiber.Ctx) {
      c.Send("Group B")
})

I consider that group need to be identified with a static name and subroutes can be dynamic.

[jos-]

My point was:

1. Parameters in route groups already work, no need to fix this. Removing this possibility would be a bad idea IMO, since many other frameworks support this.
2. I gave a minimal code example to show the issue. Of course this was not according to rest practices. If you want a more restful example from the GitHub api:

package main

import (
    "log"
    "github.com/gofiber/fiber"
)

func someMiddleware(c *fiber.Ctx) {
    // Check if the current user is authorized to access :owner/:repo

    c.Next()
}

func main() {
    app := fiber.New()

    api := app.Group("/v1")
    repo := api.Group("/repos/:owner/:repo", someMiddleware) // Doesn't work
    branch := repo.Group("/branches/:branch")

    repo.Get("/branches", func(c *fiber.Ctx) {
        // ...
    })

    branch.Get("/", func(c *fiber.Ctx) {
        // ...
    })
    branch.Get("/protection", func(c *fiber.Ctx) {
        // ...
    })
    branch.Get("/required_status_checks", func(c *fiber.Ctx) {
        // ...
    })

    log.Fatal(app.Listen(8080))
}

You can see here that:

1. Parameters in route groups can save a lot of duplicate code.
2. Middleware applied to groups with route parameters doesn't work. It does however work in groups without route parameters, which is confusing.

The current workaround for the above situation is to apply the middleware to every route separately, like:

    repo.Get("/branches", someMiddleware, func(c *fiber.Ctx) {
        // ...
    })

But this results again in a lot of duplicate code if you have to pass the middleware to every route separately.

[Fenny]

Currently app.Group behaves like app.Use if handlers are giving within the Group method. These middleware handlers do not support :params or :optionals? just like in app.Use.

In the new update v1.10 both app.Group & app.Use will support :params & :optional?, expect a release by the end of the week.

PS: For people who are not coming from express.js, app.Use routes only match the prefixes of each request path.

app.Use("/api", handler)
// will match the following request paths:
// GET    /api
// GET    /api/user
// POST   /api/register

express.js doesn't support params in use methods, but we think this would save a lot of code as mentioned here https://github.com/gofiber/fiber/issues/394#issuecomment-631301545

Quick v1.10 test for https://github.com/gofiber/fiber/issues/394#issuecomment-631301545

        _______ __
  ____ / ____(_) /_  ___  _____
_____ / /_  / / __ \/ _ \/ ___/
  __ / __/ / / /_/ /  __/ /
    /_/   /_/_.___/\___/_/ v1.10.0
Started listening on 0.0.0.0:8080
METHOD  [GET]
PATTERN [/v1/repos/:owner/:repo/branches]
REQUEST [/v1/repos/fenny/fiber/branches]
PARAMS  [[fenny fiber]]

METHOD  [GET]
PATTERN [/v1/repos/:owner/:repo/branches/:branch]
REQUEST [/v1/repos/fenny/fiber/branches/master]
PARAMS  [[fenny fiber master]]

METHOD  [GET]
PATTERN [/v1/repos/:owner/:repo/branches/:branch/protection]
REQUEST [/v1/repos/fenny/fiber/branches/master/protection]
PARAMS  [[fenny fiber master]]

[jos-]

Great, many thanks!