fiber icon indicating copy to clipboard operation
fiber copied to clipboard

🤗 Case sensitivity for parameters in GetRouteURL

Open martinwang2002 opened this issue 2 years ago • 7 comments

Question description

When using capitalized letters in fiber.Map{} with GetRouteURL, the case sensitivity of the keys is ignored.

Code snippet

package main

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

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

	// app := fiber.New(fiber.Config{
	// 	CaseSensitive: true,
	// })

	app.Get("/", func(c *fiber.Ctx) error {

		idUrl, err := c.GetRouteURL("id", fiber.Map{
			"testid": "ABC",
			"testId": "DEF",
		})

		if err != nil {
			panic(err)
		}

		return c.SendString(idUrl)
	})

	app.Get("/:testid/:testId/other_paths", func(c *fiber.Ctx) error {
		return c.SendString("Hello, World!\ntestid: " + c.Params("testid") + "\ntestId: " + c.Params("testId"))
	}).Name("id")

	app.Listen(":3001")
}

For example, GET /, you will receive /ABC/ABC/other_paths. The case sensitivity for GetRouteURL is ignored

On the other hand, c.Params can tell the difference between testid and testId.

I think that Params and GetRouteURL should behave the same behavior on case sensitivity.


Meanwhile, enabling CaseSensitive in the config will receive /ABC/DEF/other_paths

martinwang2002 avatar May 12 '22 11:05 martinwang2002

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

welcome[bot] avatar May 12 '22 11:05 welcome[bot]

code is developed by sujit-baniya -> https://github.com/gofiber/fiber/pull/1831 @sujit-baniya can you check this

ReneWerner87 avatar May 12 '22 11:05 ReneWerner87

@martinwang2002 Thank you for raising the issue. I checked with CaseSensitive enabled and disabled.

Case 1: CaseSensitive = false by default it's false In this scenario the case is ignored. so testid and testId are same in this case

Case 2: CaseSensitive = true In this scenario the case is checked. so testid and testId are different.

I was not able to replicate the issue as mentioned.

the fiber version used: 2.33.0

Here's the video of testing. https://storyxpress.co/video/l332lwf6vd11rxxhl

Please let me know if I missed anything in the test

sujit-baniya avatar May 12 '22 14:05 sujit-baniya

I agree with you.

Could you also please check this part?

app.Get("/:testid/:testId/other_paths", func(c *fiber.Ctx) error {
		return c.SendString("Hello, World!\ntestid: " + c.Params("testid") + "\ntestId: " + c.Params("testId"))
}).Name("id")

with url: http://127.0.0.1:3001/first/second/other_paths

In this part of code, you will find that testid and testId are different.


That's why I have this confustion. We can get parameters with case sensitivity, but we cannot set proper keys via GetRouteURL with CaseSensitive = false. Is this expected?

martinwang2002 avatar May 12 '22 14:05 martinwang2002

@martinwang2002 I got it now. The issue seems to be coming from route parser.

@ReneWerner87 Can we do anything for it?

&{/ false   0 false false false false 1}
&{ true testid / 2 false false false false 0}
&{/ false   0 false false false false 1}
&{ true testid /other_paths 1 false false false false 0}
&{/other_paths false   0 false false true false 12}

sujit-baniya avatar May 12 '22 14:05 sujit-baniya

if CaseSensitive is set to false, the route is reduced to lower case and the parameters are also attached to the segments of the route in lowercase(in the registration process), since this does not matter according to the setting.

https://github.com/gofiber/fiber/blob/aa229287cfeeb2002534cb6145b50f7798f76b3e/router.go#L236-L251 https://github.com/gofiber/fiber/blob/aa229287cfeeb2002534cb6145b50f7798f76b3e/router.go#L178-L188

the request url is also reduced to lowercase for recognition when the setting is enabled https://github.com/gofiber/fiber/blob/aa229287cfeeb2002534cb6145b50f7798f76b3e/ctx.go#L1548-L1555

possible solution: is in the case that caseSensitive is set to false(default) we should everywhere where access to the key is tried to get the value, compare it additionally with the lower case variant, so that the upper and lower case really doesn't matter util: https://github.com/gofiber/fiber/blob/aa229287cfeeb2002534cb6145b50f7798f76b3e/utils/strings.go#L8 places: https://github.com/gofiber/fiber/blob/aa229287cfeeb2002534cb6145b50f7798f76b3e/ctx.go#L835 https://github.com/gofiber/fiber/blob/aa229287cfeeb2002534cb6145b50f7798f76b3e/ctx.go#L1149

ReneWerner87 avatar May 12 '22 15:05 ReneWerner87

But please still take a look at the performance after these changes

i expect only a slight increase if the value could not be found by the first check

https://github.com/gofiber/fiber/blob/aa229287cfeeb2002534cb6145b50f7798f76b3e/utils/strings.go#L65 or maybe a check with equalFolds, as this is even faster than converting to lowercase

ToLower -> 50ns EqualFold -> 32ns

ReneWerner87 avatar May 12 '22 15:05 ReneWerner87