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

Routes, named params and case sensitivity #1

Closed
kpowick opened this issue Mar 7, 2014 · 10 comments
Closed

Routes, named params and case sensitivity #1

kpowick opened this issue Mar 7, 2014 · 10 comments
Assignees
Labels

Comments

@kpowick
Copy link

kpowick commented Mar 7, 2014

Am I correct in assuming that the design that allows for quick route matching also restricts the routes themselves to being case sensitive?

e.g. For the given route with a named paramter

func Hello(w http.ResponseWriter, r *http.Request, vars map[string]string) {
    fmt.Fprintf(w, "Hello, %s!\n", vars["name"])
}
func main() {
    router := httprouter.New()
    router.GET("/hello/:name", Hello)
    log.Fatal(http.ListenAndServe(":12345", router))
}

Is there any way to always return Hello Joe for the following requests?

/hello/Joe
/HELLO/Joe
/hElLo/Joe

i.e. The desire is to have the route be case insensitive with the values of named parameters retaining their casing.

@julienschmidt
Copy link
Owner

Except for the domain name portion URLs are case-sensitive as defined in the RFC.

But this is interesting thought. It would be possible to add a parameter to globally (for all routes) make the routing case insensitive by just applying strings.ToLower on the requested path.

Any other ideas?

@julienschmidt
Copy link
Owner

Sorry, I missed your last sentence. Just applying strings.ToLower wouldn't work in that case.

@julienschmidt
Copy link
Owner

It could work if strings.ToLower is used on the path substrings, when they are compared to the nodes path, so something like strings.ToLower(path[:len(n.path)]) == n.path in https://github.com/julienschmidt/httprouter/blob/master/tree.go#L254 and also somehow handle https://github.com/julienschmidt/httprouter/blob/master/tree.go#L343
This should be it.

But this would still rely on a per-router setting, is that acceptable?

@julienschmidt julienschmidt self-assigned this Mar 7, 2014
@kpowick
Copy link
Author

kpowick commented Mar 8, 2014

But this would still rely on a per-router setting...

Just to clarify. Such a setting would be per-router, thus affecting all routes defined for a particular router instance? I would have no problem with that.

I understand your point about URL RFC compliance, but that doesn't help with end users that type what they want. :)

Thanks for the consideration.

@julienschmidt
Copy link
Owner

Just to clarify. Such a setting would be per-router, thus affecting all routes defined for a particular router instance?

Exactly.

Also using both /path and /Path is a really bad practice for usability reasons.
This would definitely be a good addition. Thanks for the request. I'll try to implement it very soon.

@kpowick
Copy link
Author

kpowick commented Mar 8, 2014

Ok, thanks!

@jmervine
Copy link

jmervine commented Jun 5, 2014

In addition to this, a global option to force everything to lower, including named params as you mention above would be nice as well. Understanding of course that it will violate RFC compliance if turned on.

Something like:

httprouter.NormalizePathsToLower = true

And then here adding:

if NormalizePathsToLower {
    path = strings.ToLower(path)
}

PS: I'd be happy to submit a PR.

@julienschmidt
Copy link
Owner

This took longer and was a lot harder to implement than I hoped...

Instead of case-insensitive routes the router now supports the automatic redirection of routes having wrong cases. I think this is better since it doesn't introduce duplicate URLs (\foo, \Foo and \FOO are completely different URLs).
Another reason is, that normalizing unicode-strings to lowercase is quite expensive. Therefore the matching would be a lot more expensive compared to the case-sensitive lookups.

The router does now the following (if not configured otherwise):

  1. Make a case-sensitive lookup. If it was successful, the matching is done.
  2. Check for a TrailingSlashRecommendation, redirecting if the same URL exists with/without a trailing slash
  3. Make a case-insensitive lookup otherwise. If a handler can be found, redirect the respective path
  4. Handle the 404 otherwise

If you only have lowercase routes, this normalizes all request paths to lowercase as requested by @jmervine. But it uses a 301 permanent redirection instead of silently accepting the wrong URL 😃 (better for SEO etc.).

@julienschmidt
Copy link
Owner

tl;dr: The behavior described by @kpowick in the first post is now default, without any additional configuration.

@jmervine
Copy link

jmervine commented Jun 6, 2014

Nice! Excellent solution.

similark pushed a commit to similarweb/httprouter that referenced this issue May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants