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

Case insensitive routes? #1007

Closed
thearegee opened this Issue Jun 29, 2018 · 8 comments

Comments

Projects
None yet
4 participants
@thearegee

thearegee commented Jun 29, 2018

For a project I'm working on using Fastify, there are a collection of routing keywords.

  • content
  • shop
  • product
    ...

Due to URLs that are out in the wild these could be also be requested in sentence case.

With this simple example:

const fastify = require('fastify')()

// Declare a route
fastify.get('/hello', function (request, reply) {
  reply.send({ hello: 'world' })
})

fastify.listen(3000, (err, address) => {
  if (err) throw err
  fastify.log.info(`server listening on ${address}`)
})

I noticed the routes are case sensitive.

$ curl -I http://localhost:3000/hello
# HTTP/1.1 200 OK
$ curl -I http://localhost:3000/Hello
# HTTP/1.1 404 Not Found

I was unaware of this before, I looked at find-my-way but couldn't see any options, I assume this is for performance reasons? I understand I could make the routes regular expressions but I know this is expensive.

I just wanted to double check if I was missing something simple, this is a must have for me and I would rather not build a regex if possible.

Thanks,
Robin

@mcollina

This comment has been minimized.

Member

mcollina commented Jun 29, 2018

This has not been implemented because we did not think about this feature. How are you dealing with it right now?

Would you need this for all routes, or just a few?

I think the best approach would be to add this feature to https://github.com/delvedor/find-my-way in the form of an option that makes all paths lowercase, both during insert and lookup. Ideally this should not have a performance impact apart from an additional toLowerCase() when it's added.

@mcollina

This comment has been minimized.

Member

mcollina commented Jun 29, 2018

@thearegee

This comment has been minimized.

thearegee commented Jun 29, 2018

So I actually just discovered this by chance in testing, so currently not handling it but thought about creating regex route as a workaround.

It would be a case of maybe 10 to 20 routes, however each of those routes has a parameter in them before the routing word.

@jsumners

This comment has been minimized.

Member

jsumners commented Jun 29, 2018

Really this should be handled at the proxy. Only non-conforming servers are case insensitive.

@thearegee

This comment has been minimized.

thearegee commented Jun 30, 2018

Rightly or wrongly, this is an option in express: https://expressjs.com/en/api.html#express.router

@jsumners

This comment has been minimized.

Member

jsumners commented Jun 30, 2018

It is wrong:

When a URI uses components of the generic syntax, the component
syntax equivalence rules always apply; namely, that the scheme and
host are case-insensitive and therefore should be normalized to
lowercase. For example, the URI HTTP://www.EXAMPLE.com/ is
equivalent to http://www.example.com/. The other generic syntax
components are assumed to be case-sensitive unless specifically
defined otherwise by the scheme

https://tools.ietf.org/html/rfc3986#section-6.2.2.1

@mcollina

This comment has been minimized.

Member

mcollina commented Jun 30, 2018

@jsumners I think we should support this, but keep it disabled by default. Case insensitive routes are supported (and enabled by default) in Express: a large of developers expects this features to be available.

Here is a PR: delvedor/find-my-way#84

@jsumners

This comment has been minimized.

Member

jsumners commented Jun 30, 2018

Okay.

@mcollina mcollina referenced this issue Jul 2, 2018

Merged

Support case insensitivity. #1017

4 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment