🐛 [Bug]: Specifying middleware in the handler doesn't work

[vrnvu]

Bug Description

Specifying middleware in the handler doesn't work:
https://docs.gofiber.io/next/middleware/keyauth#specifying-middleware-in-the-handler

➜  ~ curl localhost:3000/allowed
Successfully authenticated!%

How to Reproduce

Copy and paste:
https://docs.gofiber.io/next/middleware/keyauth#specifying-middleware-in-the-handler

module github.com/vrnvu/go-test-fiber

go 1.23.4

require (
	github.com/andybalholm/brotli v1.1.1 // indirect
	github.com/fxamacker/cbor/v2 v2.7.0 // indirect
	github.com/gofiber/fiber/v3 v3.0.0-beta.4 // indirect
	github.com/gofiber/schema v1.3.0 // indirect
	github.com/gofiber/utils/v2 v2.0.0-beta.7 // indirect
	github.com/google/uuid v1.6.0 // indirect
	github.com/klauspost/compress v1.17.11 // indirect
	github.com/mattn/go-colorable v0.1.14 // indirect
	github.com/mattn/go-isatty v0.0.20 // indirect
	github.com/philhofer/fwd v1.1.3-0.20240916144458-20a13a1f6b7c // indirect
	github.com/tinylib/msgp v1.2.5 // indirect
	github.com/valyala/bytebufferpool v1.0.0 // indirect
	github.com/valyala/fasthttp v1.58.0 // indirect
	github.com/valyala/tcplisten v1.0.0 // indirect
	github.com/x448/float16 v0.8.4 // indirect
	golang.org/x/crypto v0.33.0 // indirect
	golang.org/x/net v0.35.0 // indirect
	golang.org/x/sys v0.30.0 // indirect
	golang.org/x/text v0.22.0 // indirect
)

Expected Behavior

It seems that its working if we put auth in group, it doesn't work at handler level.

Fiber Version

github.com/gofiber/fiber/v3

Code Snippet (optional)

package main

import (
	"crypto/sha256"
	"crypto/subtle"

	"github.com/gofiber/fiber/v3"
	"github.com/gofiber/fiber/v3/middleware/keyauth"
)

const (
	apiKey = "my-super-secret-key"
)

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

	authMiddleware := keyauth.New(keyauth.Config{
		Validator: func(c fiber.Ctx, key string) (bool, error) {
			hashedAPIKey := sha256.Sum256([]byte(apiKey))
			hashedKey := sha256.Sum256([]byte(key))

			if subtle.ConstantTimeCompare(hashedAPIKey[:], hashedKey[:]) == 1 {
				return true, nil
			}
			return false, keyauth.ErrMissingOrMalformedAPIKey
		},
	})

	app.Get("/", func(c fiber.Ctx) error {
		return c.SendString("Welcome")
	})

	app.Get("/allowed", authMiddleware, func(c fiber.Ctx) error {
		return c.SendString("Successfully authenticated!")
	})

	app.Listen(":3000")
}

Checklist:

• I agree to follow Fiber's Code of Conduct.
• I have checked for existing issues that describe my problem prior to opening this one.
• I understand that improperly formatted bug reports may be closed without explanation.

[welcome]

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

[ReneWerner87]

thx for the report
there is indeed a problem

https://expressjs.com/de/api.html#router.methods

but the current logic packs the handler after the middlewares

fiber/router.go

Line 324 in 845a7f8

handlers = append(handlers, handler)

@pjebs was there a reason for this ?
it is coming from #2176

[ReneWerner87]

my fix would be

[JIeJaitt]

@ReneWerner87 Maybe it can be fixed this way?

handlers := middleware
handlers = append(handlers, handler)

[ReneWerner87]

no, that's how it was before and nothing will change
if we say that the last parameter with the other handlers is the stack and we add the handler to the stack, it ends up at the end

[JIeJaitt]

@ReneWerner87 May I ask what is the difference between handlers = append(handlers, handler) and handlers = append([]Handler{handler,handlers...}) What's the difference?

[ReneWerner87]

The order of the registered handlers under the route and with this the order of the process.

[pjebs]

I actually haven't been involved in v3 at all.
I did propose a change to a laravel style ordering a few years back but it created lots of pushback due to departure from express.

I remember someone did a survey at one point and my suggestion was popular, but last I remember there was going to be a configuration to pick your preferred ordering.

[pjebs]

It looked like my suggestion got merged, in which case your ordering is wrong. The docs will need to get updated since it was merged.

The compulsory end handler comes first (because every endpoint by definition needs a handler), then the list of (optional) middlewear. In practice, you would have a whole group of routes that require authMiddleware so adding ad-hoc middleware is not the usual way in a real-world application.

[pjebs]

See #2160.

v2 definition:

type Router interface {
	Use(args ...interface{}) Router

	Get(path string, handlers ...Handler) Router
	Head(path string, handlers ...Handler) Router
	Post(path string, handlers ...Handler) Router
	Put(path string, handlers ...Handler) Router
	Delete(path string, handlers ...Handler) Router
	Connect(path string, handlers ...Handler) Router
	Options(path string, handlers ...Handler) Router
	Trace(path string, handlers ...Handler) Router
	Patch(path string, handlers ...Handler) Router

	Add(method, path string, handlers ...Handler) Router
	Static(prefix, root string, config ...Static) Router
	All(path string, handlers ...Handler) Router

	Group(prefix string, handlers ...Handler) Router

	Route(prefix string, fn func(router Router), name ...string) Router

	Mount(prefix string, fiber *App) Router

	Name(name string) Router
}

v3 definition:

It would be good if the signature could be changed for these:

	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(method, path string, handler Handler, middleware ...Handler) Router
	All(path string, handler Handler, middleware ...Handler) Router

Secondly,

// Note: methods plural
Add(methods []string, path string, handler Handler, middleware ...Handler) Router

[pjebs]

@vrnvu

The end-handler goes first and the list of optional middleware goes at the end so your code should be:

	app.Get("/", func(c fiber.Ctx) error {
		return c.SendString("Welcome")
	})

	app.Get("/allowed", func(c fiber.Ctx) error {
		return c.SendString("Successfully authenticated!")
	}, authMiddleware, middleware2, middleware3)

If you are adamant on using strict Express style ordering you can also just pass nil for your handler and pretend everything is middleware:

	app.Get("/", func(c fiber.Ctx) error {
		return c.SendString("Welcome")
	})

	app.Get("/allowed", nil, authMiddleware, func(c fiber.Ctx) error {
		return c.SendString("Successfully authenticated!")
	})

That way you can still see "the flow" the request travels through.

However,

In practice, you would have a whole group of routes that require authMiddleware so adding ad-hoc middleware is not the usual way in a real-world application.

In the short-term, you can also define a function to remind yourself (and not get mentally confused) of Express ordering. That way if you forget the new signature, it will create a compile-time error to remind you:

	func Express(handlers ...Handler) []Handler {
		return handlers
	}

	// Legacy pattern
	func X(handlers ...Handler) []Handler {
		return handlers
	}


	app.Get("/allowed", nil, X(authMiddleware, func(c fiber.Ctx) error {
		return c.SendString("Successfully authenticated!")
	})...)

[ReneWerner87]

okay thanks for the explanation
as we tend to follow express concepts, i would change the internal sequence
I didn't notice that at the time
additional ideas from other frameworks/languages are fine as long as they don't conflict with the flow/concept of epxressjs

[pjebs]

my fix would be

@ReneWerner87 You will need to actually change the signature back to v2 rather than your adjustment (which will work) but it contradicts the signature.

According to the proposal: #2160 (if memory serves me correctly since it was 2-3 years ago)

My understanding after the proposal was that a survey was done by someone on the Fiber team that revealed it was very popular (after some initial backlash) and then you merged it.

Then I remember you made a comment that you wanted it to be configurable to accept both signatures.

Also note, the v3 signature is consistent with Express provided you pass nil for the second argument.

Also note, as per @vrnvu, he/she naturally named his middleware: authMiddleware. The v3 function's signature implies where it belongs.