New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
馃悰 Internal redirect does not reset route and handler index #1731
Comments
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 |
@mtneug , can you please provide a use-case for this feature? i don't see any benefits of adding a In your given example, even if we add a 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 |
@DarthBenro008: my point with 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
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. |
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
(andc.indexHandler
). This is currently not done which could lead to unexpected routing decisions. In the example below requestingGET /old
would returnNot Found
while I would expect it to benew
. 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
andc.indexHandler
inc.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 resetsc.indexRoute
and then executesc.app.next()
?Code snippet
The text was updated successfully, but these errors were encountered: