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

🐞 "/:param" -> "/:param" doesn't return param #405

Closed
pavel1337 opened this issue May 23, 2020 · 6 comments
Closed

🐞 "/:param" -> "/:param" doesn't return param #405

pavel1337 opened this issue May 23, 2020 · 6 comments

Comments

@pavel1337
Copy link

pavel1337 commented May 23, 2020

Fiber version/commit
v1.10.0 (Master branch)

Issue description
Server panics on ":param" GET value if ":param" is set in route. Error is below.

        _______ __
  ____ / ____(_) /_  ___  _____
_____ / /_  / / __ \/ _ \/ ___/
  __ / __/ / / /_/ /  __/ /
    /_/   /_/_.___/\___/_/ v1.10.0
Started listening on 0.0.0.0:3000
panic: runtime error: index out of range [0] with length 0

goroutine 5 [running]:
github.com/gofiber/fiber.(*Ctx).Params(0xc00020a000, 0x902cea, 0x5, 0x7, 0xc000067b01)
	/home/pavel/go/src/github.com/gofiber/fiber/ctx.go:578 +0x102
main.main.func1(0xc00020a000)
	/home/pavel/go/src/github.com/pavel1337/currency/main.go:10 +0x44
github.com/gofiber/fiber.(*App).next(0xc0001e4000, 0xc00020a000, 0xc00002a148)
	/home/pavel/go/src/github.com/gofiber/fiber/router.go:36 +0x16a
github.com/gofiber/fiber.(*App).handler(0xc0001e4000, 0xc000206000)
	/home/pavel/go/src/github.com/gofiber/fiber/router.go:59 +0x8c
github.com/valyala/fasthttp.(*Server).serveConn(0xc0000ca900, 0x9b6420, 0xc000010018, 0x0, 0x0)
	/home/pavel/go/src/github.com/valyala/fasthttp/server.go:2150 +0x11bb
github.com/valyala/fasthttp.(*workerPool).workerFunc(0xc0000b3180, 0xc00000e040)
	/home/pavel/go/src/github.com/valyala/fasthttp/workerpool.go:223 +0xc0
github.com/valyala/fasthttp.(*workerPool).getCh.func1(0xc0000b3180, 0xc00000e040, 0x857d40, 0xc00000e040)
	/home/pavel/go/src/github.com/valyala/fasthttp/workerpool.go:195 +0x35
created by github.com/valyala/fasthttp.(*workerPool).getCh
	/home/pavel/go/src/github.com/valyala/fasthttp/workerpool.go:194 +0x101
exit status 2

Steps to reproduce

go run main.go
curl localhost:3000/:param

Code snippet


import (
	"github.com/gofiber/fiber"
)

func main() {
	app := fiber.New()
	app.Get("/:param", func(c *fiber.Ctx) {
		c.Send("GET request with param: ", c.Params("param"))
	})
	app.Listen(3000)
}
@welcome
Copy link

welcome bot commented May 23, 2020

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

@thomasvvugt
Copy link
Contributor

thomasvvugt commented May 23, 2020

Hi @pavel1337 ,

Thanks for creating this issue.
I have two questions before moving this issue to a high priority.
Have you tried renaming the parameter to something else like /:foo ?

Also, do you use Go Modules or go get to import Fiber as a package?
If you use Go Modules, could you send me the exact Fiber line version from your go.sum file?
This should look something like this:

github.com/gofiber/fiber v1.9.0/go.mod h1:yQhhFUJprqnZVaEbd5h4ZqU+wb9vzP5imw7UbjGlDuQ=

Thank you very much!

Additional comment:
I have not been able to reproduce this bug using Go Modules, not even after go get -u to update packages. Maybe this bug is caused by /home/pavel/go/src/github.com/pavel1337/currency/main.go:10 ?

@Fenny
Copy link
Member

Fenny commented May 23, 2020

@pavel1337 thank you for your detailed bug report. I can confirm that passing the exact same parameter key including : semicolon in the request path will cause a panic when fetching the param. This is a rare use case but it shouldn't happen. I merged a hot fix for now but expect a real fix soon, for now this only seems to happen on the master branch.

@ReneWerner87, current master branch:
app.Get("/:param") -> GET /:param
app.Get("/:foo") -> GET /:foo

app.Get("/:param") -> GET /:foo ✔️
app.Get("/:foo") -> GET /:param ✔️

ReneWerner87 pushed a commit to ReneWerner87/fiber that referenced this issue May 24, 2020
@ReneWerner87
Copy link
Member

Fenny added a commit that referenced this issue May 24, 2020
@pavel1337
Copy link
Author

Hi guys, thank you for incredibly fast reaction!
@thomasvvugt, sorry for incomplete description, I hope @Fenny's reply answers your questions.

@Fenny I would not even call it "rare use" it s more like possible "misuse" or even "attack vector".

I can confirm that after go get -u github.com/gofiber/fiber app is not panicing anymore. That's the behavior I see now:

$ curl localhost:3000/asdasd
GET request with param: asdasd

$ curl localhost:3000/:foo
GET request with param: 

$ curl localhost:3000/::foo
GET request with param: ::foo

with the code as follows:

package main

import "github.com/gofiber/fiber"

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

	app.Get("/:foo", func(c *fiber.Ctx) {
		c.Send("GET request with param: ", c.Params("foo"), "\n")
	})
	app.Listen(3000)
}

For me issue is resolved now, even though the current behavior seems somehow incomplete. I would expect :foo to show up anyway or return "422 Unprocessable Entity" or something like this.

@Fenny
Copy link
Member

Fenny commented May 24, 2020

@ReneWerner87 and I are working on a fix, this happens because of our router logic. It will always check for exact matches first /:param == /:param before it tries to parse parameters.

We will change this behaviour to threat /:param -> /:param as actual "params" instead of a static route.

ReneWerner87 pushed a commit to ReneWerner87/fiber that referenced this issue May 24, 2020
Fenny added a commit that referenced this issue May 24, 2020
@Fenny Fenny changed the title 🐞 Panic on ":param" 🐞 "/:param" -> "/:param" doesn't return param May 24, 2020
ReneWerner87 pushed a commit to ReneWerner87/fiber that referenced this issue May 24, 2020
ReneWerner87 pushed a commit to ReneWerner87/fiber that referenced this issue May 24, 2020
Fenny added a commit that referenced this issue May 24, 2020
@Fenny Fenny mentioned this issue May 24, 2020
@Fenny Fenny closed this as completed May 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants