🐛 Internal redirect does not reset route and handler index

[mtneug]

Fiber version

v2.25.0

Issue description

I found this issue while searching for a way to do internal redirects. #1398 seems to suggest that this is not supported, however, there is the out-of-tree rewrite middleware that is exactly doing that. It essentially uses the c.Path() method to set a new path. The code below shows a simplified example.

As also mentioned in #1398, an internal redirect would need to reset c.indexRoute (and c.indexHandler). This is currently not done which could lead to unexpected routing decisions. In the example below requesting GET /old would return Not Found while I would expect it to be new. This is because the /new route was already checked before the internal redirect and is thus skipped. If instead the /new route would be registered after the redirect handler it would be executed.

I initially thought of resetting c.indexRoute and c.indexHandler in c.Path(), but this could be unexpected as well, e.g. handlers might be executed twice. Making this an explicit choice by the user might be more appropriate.

What do you think about adding a Restart method to the context that resets c.indexRoute and then executes c.app.next()?

Code snippet

package main

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

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

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

	app.Use(func(c *fiber.Ctx) error {
		c.Path("/new")
		return c.Next()
	})

	app.Use(func(c *fiber.Ctx) error {
		// e.g. catch-all for 404
		return fiber.ErrNotFound
	})

	app.Listen(":3000")
}

[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

[DarthBenro008]

@mtneug , can you please provide a use-case for this feature? i don't see any benefits of adding a c.Reset() because GoFiber is highly inspired by Express and top to bottom middleware registrations in the tree is what it follows.

In your given example, even if we add a c.Reset(), you will never reach NotFound page, instead why not register a route late like below?

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

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

	app.Use(func(c *fiber.Ctx) error {
		c.Path("/new")
		return c.Next()
	})

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

	app.Use(func(c *fiber.Ctx) error {
		// e.g. catch-all for 404
		return fiber.ErrNotFound
	})

	app.Listen(":3000")
}

I believe its more of an application-design issue and not of fiber framework

[mtneug]

@DarthBenro008: my point with NotFound in the example above is that it is executed when routes are not registered in the correct order. I don't want it to be executed so basically making your code and mine functionally identical.

The use case can be a simple one as with the rewrite middleware. Without restarting route handling, it is not guaranteed that handlers matching the new route are executed.

My original use case is implementing per-host routes with multiple possible matches. So e.g. I would register a different set of routes for foo.bar, *.example.com and test.example.com. I map these three candidates to the path when registering routes. This is done dynamically through configuration so I don't control what candidates are available. I set up the routes in a way such that a request to http://test.example.com is executed in the following way:

// globally available routes
-> normal fiber routing -> catch-all handler internal redirect to `pathPrefix("test.example.com") + originalPath`

// routes for test.example.com
-> normal fiber routing -> catch-all handler internal redirect to `pathPrefix("*.example.com") + originalPath`

// routes for *.example.com
-> normal fiber routing -> catch-all handler returning 404

// foo.bar is not a candidate as it does not match test.example.com

Each redirect requires a restart of the request handling. Do you have a better idea for implementing this with fiber (BTW: differentiating routes by the host would be a welcome addition in v3)? Yes, I could add a host check in each of the handlers, but I would need to do that for every middleware. With this approach middleware and handlers don't care about host matching.