Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐞 Nested routing not working, id interpolated incorrectly #354

Closed
cgiacomi opened this issue May 10, 2020 · 6 comments
Closed

🐞 Nested routing not working, id interpolated incorrectly #354

cgiacomi opened this issue May 10, 2020 · 6 comments
Assignees

Comments

@cgiacomi
Copy link

cgiacomi commented May 10, 2020

Fiber version/commit
1.9.5

Issue description
Nested routes not called, id interpolated incorrectly

Expected behavior
Nested route called correctly

Steps to reproduce

go run server.go
(snippet below)

curl --request GET --url http://localhost:1500/resource
(this route called correctly)

curl --request GET --url http://localhost:1500/resource/12345
(this route will work, and response contains the correct id '12345')

curl --request GET --url http://localhost:1500/resource/12345/bug
(this route will not work, 'bug' is considered part of the id of the resource, still routes to the previous route)

curl --request GET --url http://localhost:1500/resource/12345/bug/999
(also not working, will still call 'reusource/:id' with the invalid id)

Code snippet

//server.go
package main

import "github.com/gofiber/fiber"

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

	app.Get("/resource", func(c *fiber.Ctx) {
		c.Send("resource")
	})

	app.Get("/resource/:id", func(c *fiber.Ctx) {
		c.Send("resource - id: " + c.Params("id"))
	})

	app.Get("/resource/:id/bug", func(c *fiber.Ctx) {
		c.Send("NEVER CALLED BUG id: " + c.Params("id"))
	})

        app.Get("/resource/:id/bug/:bid", func(c *fiber.Ctx) {
		c.Send("NEVER CALLED BUG bid: " + c.Params("bid"))
	})

	app.Listen(1500)
}

Thank you for the great work with this framework. Bug reports are so impersonal, this one doesn't want to be like that, and it's not a show stopper for me as I reverted to 1.9.3 which seems to work fine. Thank you.

@Fenny
Copy link
Member

Fenny commented May 11, 2020

cc @ReneWerner87

   --- FAIL: Test_With_Nested_Routing_Multiple_Params (0.00s)
        router_test.go:251: Path.Match() got = [], want [12345 999]
        router_test.go:254: Path.Match() got1 = false, want true

@cgiacomi, thanks for your bug report, I can confirm that this is unwanted behavior and will be fixed asap with a hotfix.

@cgiacomi
Copy link
Author

cgiacomi commented May 11, 2020 via email

@ReneWerner87
Copy link
Member

found the bug in the meantime, I'm working on a fix

@cgiacomi
Copy link
Author

@ReneWerner87 @Fenny thank you for your time.

ReneWerner87 pushed a commit to ReneWerner87/fastpath that referenced this issue May 11, 2020
ReneWerner87 pushed a commit to ReneWerner87/fastpath that referenced this issue May 11, 2020
ReneWerner87 pushed a commit to ReneWerner87/fiber that referenced this issue May 11, 2020
🐞 Nested routing not working, id interpolated incorrectly gofiber#354
gofiber#354
ReneWerner87 pushed a commit to ReneWerner87/fastpath that referenced this issue May 11, 2020
ReneWerner87 pushed a commit to ReneWerner87/fastpath that referenced this issue May 11, 2020
@ReneWerner87
Copy link
Member

fixed in PR:
#356
fix in fastpath:
renanbastos93/fastpath#6

@cgiacomi thanks for the bug report

This was referenced May 11, 2020
Fenny added a commit that referenced this issue May 11, 2020
**🚀 Fiber `v1.9.6`**

Special thanks to @renanbastos93 & @ReneWerner87 for optimizing the current router.
Help use translate our API documentation by [clicking here](https://crowdin.com/project/gofiber)

🔥 New
- `AcquireCtx` / `ReleaseCtx`
The Ctx pool is now accessible for third-party packages
- Fiber docs merged [Russian](https://docs.gofiber.io/v/ru/) translations **84%**
- Fiber docs merged [Spanish](https://docs.gofiber.io/v/es/) translations  **65%**
- Fiber docs merged [French](https://docs.gofiber.io/v/fr/) translations  **40%**
- Fiber docs merged [German](https://docs.gofiber.io/v/de/) translations  **32%**
- Fiber docs merged [Portuguese](https://docs.gofiber.io/v/pt/) translations  **24%**

🩹 Fixes
- Hotfix for interpolated params in nested routes #354
- Some `Ctx` methods didn't work correctly when called without an `*App` pointer.
- `ctx.Vary` sometimes added duplicates to the response header
- Improved router by ditching regexp and increased performance by **817%** without allocations.
```go
// Tested with 350 github API routes
Benchmark_Router_OLD-4      614   2467460 ns/op   68902 B/op   600 allocs/op
Benchmark_Router_NEW-4     3429    302033 ns/op       0 B/op     0 allocs/op
```

🧹 Updates
- Add context benchmarks
- Remove some unnecessary functions from `utils`
- Add router & param test cases
- Add new coffee supporters to readme
- Add third party middlewares to readme
- Add more comments to source code
- Cleanup some old helper functions

🧬 Middleware
- [gofiber/adaptor](https://github.com/gofiber/adaptor) `v0.0.1` Converter for net/http handlers to/from Fiber handlers
- [gofiber/session](https://github.com/gofiber/session) `v1.0.0` big improvements and support for storage providers
- [gofiber/logger](https://github.com/gofiber/logger) `v0.0.6` supports `${error}` param
- [gofiber/embed](https://github.com/gofiber/embed) `v0.0.9` minor improvements and support for directory browsing 

Co-authored-by: ReneWerner87 <renewerner87@googlemail.com>
@Fenny
Copy link
Member

Fenny commented May 11, 2020

Thanks @ReneWerner87!
@cgiacomi thnx for your report, hotfix is tagged in v1.9.6

@Fenny Fenny closed this as completed May 11, 2020
renanbastos93 added a commit to renanbastos93/fastpath that referenced this issue May 11, 2020
Nested routing not working, id interpolated incorrectly #354
gofiber/fiber#354
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants