🤗 a not intuitive group middleware matching problem

[trim21]

Question description
I'm using App.Group() with middleware but I found a middleware add in group A may match request to group B.

This may not be a bug because prefix /p indeed should match path /page, but it would be more intuitive if these middleware are only group-wide.

I'm guesing app.Group(prefix, middleware) is working just like app.Use(prefix, middleware)?

Code snippet Optional

you will see output "group '/p" and "group '/page" in this example:

package main

import (
	"fmt"
	"io"
	"net/http"
	"net/http/httptest"

	"github.com/gofiber/fiber/v2"
)

func main() {
	app := fiber.New()
	g1 := app.Group("/p", func(c *fiber.Ctx) error {
		fmt.Println("group '/p")
		return c.Next()
	})

	g1.Get("/1", func(c *fiber.Ctx) error {
		return c.SendString("/p/1")
	})

	g2 := app.Group("/page", func(c *fiber.Ctx) error {
		fmt.Println("group '/page")
		return c.Next()
	})

	g2.Get("/2", func(c *fiber.Ctx) error {
		return c.SendString("/page/1")
	})

	req := httptest.NewRequest(fiber.MethodGet, "/page/2", http.NoBody)
	res, err := app.Test(req)
	if err != nil {
		panic(err)
	}
	defer res.Body.Close()
	content, err := io.ReadAll(res.Body)
	if err != nil {
		panic(err)
	}

	fmt.Println(string(content))
}

[ReneWerner87]

Yes right
Group with a handler is working like the app.Use method

https://github.com/gofiber/fiber/blob/master/group.go#L179

I think express is working like the same or?
What's you improvement suggestion?
Behavior change or additional documentation?

[ReneWerner87]

Concept of real groups does not exist in express, they are just grouped paths that are checked in the order they are declared.
We do the same in fiber.

[trim21]

In fact I'd sugestion buth

Behavior change

Just for me, I think it would be better to understand if group middlware is scoped:

	group := app.Group(prefix, middleware)
	group.Get(path, handler)
	group.Get(path2, handler2)

middleware's behavior is like this:

	group := app.Group(prefix)
	group.Get(path, middleware, handler)
	group.Get(path2, middleware, handler2)

NOT:

	app.Use(prefix, middleware)
	group := app.Group(prefix)
	group.Get(path, handler)
	group.Get(path2, handler2)

additional documentation

this can be avoid by add tailing slash to group prefix:

	g1 := app.Group("/p/", middleware)
	g2 := app.Group("/page/", middleware)

this won't change final request paths but can aviod group middleware match other group's request, as long as users don't create multiple group with same prefix.

[ReneWerner87]

We will think about it.

[ReneWerner87]

@trim21

have tested this and it does not fix the problem

	g1 := app.Group("/p/", middleware)
	g2 := app.Group("/page/", middleware)

otherwise i would have put this kind of solution in the group method

[trim21]

by “fix” I mean all router with /p/ or /page/ prefix are added by g1 or g2, and middlewares of g1 won't affect g2, not means that it won't mess up the routers add by app.Add(method, "/p/a-router") or app.Add(method, "/page/a-router") or app.Group("/p/prefix2/").Add() or app.Group("/page/prefix2/").Add()

[trim21]

and as for document, I mean that mention group middleware is App.Use( in fact and may match another group, adding slash can aviod some unexpected matches (not a real fix).