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

Colliding routes causes params to be undefined in route handler #149

Closed
ajaco opened this issue May 13, 2020 · 6 comments
Closed

Colliding routes causes params to be undefined in route handler #149

ajaco opened this issue May 13, 2020 · 6 comments

Comments

@ajaco
Copy link

ajaco commented May 13, 2020

Hi!

I was working on a project and I found some odd behaviour while using Fastify. I managed to reproduce it in find-my-way, so here goes.

Given the following routes, each with their own handler:

  • GET /foo/:id (handler 1)
  • GET /foo/:color/:id (handler 2)
  • GET /foo/red (handler3)

If you then do a GET /foo/red/123, the correct handler (handler 2) will be invoked.
The id-param will correctly be '123', but the color param is undefined.

If you do a GET /foo/blue/123, both params are correct (blue, 123).

Handler 3 is still correctly invoked for GET /foo/red. So the routing is correct, but the params are wrong.

If you unregister GET /foo/red the /foo/:color/:id route will then work again with red.

I created a fork of the repo with a unit test to illustrate the problem: ajaco@8b48949

That test will fail with the following output:
image

So I'm not sure if this is a bug or intended, but I found it very odd. Hopefully this issue will either highlight a bug, or make me understand this behaviour.

@Eomm
Copy link
Contributor

Eomm commented May 13, 2020

It seems related to fastify/fastify#2261

@ajaco ajaco changed the title Empty route params in handler in certain cases Colliding routes causes params to be undefined in route handler May 13, 2020
@mcollina
Copy link
Collaborator

The solution is to:

GET /foo/:id (handler 1)
GET /foo/:color/:id (handler 2) <-- same handler
GET /foo/red (handler3)
GET /foo/red/:id (handler 2) <-- same handler

@ajaco
Copy link
Author

ajaco commented May 15, 2020

The solution is to:

GET /foo/:id (handler 1)
GET /foo/:color/:id (handler 2) <-- same handler
GET /foo/red (handler3)
GET /foo/red/:id (handler 2) <-- same handler

This does not solve the problem.

If you request GET /foo/red/123, req.params will look like this: {id: '123'}.

So while you, in this specific case, can infer that color missing from the req.params means that its value is actually red, that is not sustainable. What happens if you register the following routes:

GET /foo/:id (handler 1)
GET /foo/:color/:id (handler 2) <-- same handler
GET /foo/red (handler3)
GET /foo/red/:id (handler 2) <-- same handler
GET /foo/blue (handler 4) 
GET /foo/blue/:id (handler 2) <-- same handler

If someone requests /foo/red/123 now, how do you know its red? It could be blue as well now.

@mcollina
Copy link
Collaborator

If you are using fastify, you can find the options you have passed in when creating the routes as req.context.config.

@mcollina
Copy link
Collaborator

You can also set a default for the color param with configuring a schema for the params: https://www.fastify.io/docs/latest/Validation-and-Serialization/#validation.

@ivan-tymoshenko
Copy link
Collaborator

This issue was fixed. I added a test here #262. We can close this.

mcollina pushed a commit that referenced this issue Apr 25, 2022
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

4 participants