Routing Weirdness Within a Chunk?
SJrX opened this issue · 10 comments
Hello,
Apologies if this just is a complete duplicate of #161 , I have read it, but admittedly not digested it, but maybe one aspect that wasn't clear to me is that this blurb from the README.md
The routing algorithm matches one chunk at a time (where the chunk is a string between two slashes), this means that it cannot know if a route is static or dynamic until it finishes to match the URL.
I seem to be having issues, where matching is based not on a chunk, but on substrings in a chunk.
const http = require('http')
const router = require('find-my-way')()
const routes = [
'/api/users/award_winners',
'/api/users/admins',
'/api/users/:id',
'/api/:resourceType/foo',
]
routes.forEach(element => {
router.on('GET', element, (req, res, params) => {
res.end(`{"message":"${element}"}`)
})
}
);
const server = http.createServer((req, res) => {
router.lookup(req, res)
})
server.listen(3000, err => {
if (err) throw err
console.log('Server listening on: http://localhost:3000')
})
console.log(router.prettyPrint())
Test Script
#!/bin/bash
URLS=(
/api/users/award_winners
/api/users/admins
/api/users/a766c023-34ec-40d2-923c-e8259a28d2c5
/api/users/b766c023-34ec-40d2-923c-e8259a28d2c5
)
for URL in ${URLS[@]}; do
echo -n "$URL: "
curl -s -o /dev/null -w "%{http_code}" http://localhost:3000$URL
echo -e -n "\n"
done
Router Output
└── /api/
├── users/
│ ├── a
│ │ ├── ward_winners (GET)
│ │ └── dmins (GET)
│ └── :id (GET)
└── :resourceType/foo (GET)
Test Script Output
/api/users/award_winners: 200
/api/users/admins: 200
/api/users/a766c023-34ec-40d2-923c-e8259a28d2c5: 404
/api/users/b766c023-34ec-40d2-923c-e8259a28d2c5: 200
In particular the ids we use are UUIDs, and it seems that when the UUID begins with the letter a, it essentially can't find the route anymore.
If I remove any of the following routes then the /api/users/a766c023-34ec-40d2-923c-e8259a28d2c5 route is 200.
/api/users/award_winners/api/users/admins/api/:resourceType/foo
My read of the readme says that it we should match and consumer, api and then match and consume users greedly, then it should simply have the choice between {award_winners, admins, :id}, and award_winners and admins should take priority, then both UUIDs should resolve to :id.
I'm involved in updating a restify service that has hundreds of routes to a newer version that is based on find-my-way. I guess my general concern is that I don't understand the rules that find-my-way is using to resolve routes, and it seems very brittle, where adding a route anywhere in our platform may subtly break another route and it may break another route based only on some subset of values. I don't even know at this point what to explain to devs in terms of how to think about what the algorithm is doing.
Hello! I can reproduce the issue with the latest release of the router, it looks like it's an edge case of #104.
I'll try to figure out the issue as soon as I have time, meanwhile, feel free to try giving it a stab!
Here you can find some testing that is already existing around that issue.
Apparently, the issue happens when a static route is mixed with a dynamic route, eg:
/api/users/admin/api/users/:id
If you change the routes to:
/api/users/admin/api/user/:id(removed thes)
The problem disappears.
I cannot reproduce the error with this test case, what am I missing?
(tried ignoreTrailingSlash true or false)
'use strict'
const t = require('tap')
const FindMyWay = require('../')
t.test('issue-190', (t) => {
t.plan(6)
const findMyWay = FindMyWay()
let staticCounter = 0
let paramCounter = 0
const staticPath = function staticPath () { staticCounter++ }
const paramPath = function paramPath () { paramCounter++ }
const extraPath = function extraPath () { }
findMyWay.on('GET', '/api/users/award_winners', staticPath)
findMyWay.on('GET', '/api/users/admins', staticPath)
findMyWay.on('GET', '/api/users/:id', paramPath)
findMyWay.on('GET', '/api/:resourceType/foo', extraPath)
t.equal(findMyWay.find('GET', '/api/users/admins').handler, staticPath)
t.equal(findMyWay.find('GET', '/api/users/award_winners').handler, staticPath)
t.equal(findMyWay.find('GET', '/api/users/a766c023-34ec-40d2-923c-e8259a28d2c5').handler, paramPath)
t.equal(findMyWay.find('GET', '/api/users/b766c023-34ec-40d2-923c-e8259a28d2c5').handler, paramPath)
findMyWay.lookup({
method: 'GET',
url: '/api/users/admins',
headers: { }
})
findMyWay.lookup({
method: 'GET',
url: '/api/users/award_winners',
headers: { }
})
findMyWay.lookup({
method: 'GET',
url: '/api/users/a766c023-34ec-40d2-923c-e8259a28d2c5',
headers: { }
})
t.equal(staticCounter, 2)
t.equal(paramCounter, 1)
})Thanks, that one trigger the issue 👍🏽
I have updated the test
I'm not working on it, I had the chance to reproduce it but not to fix it
We are using old Restify version blocked to upgrade it. But I've realized both Restify and Fastify using find-my-way for the routing. Is there any way to prioritize it?
We cannot reproduce it in version 2
find-my-way@^2.0.1:
version "2.2.5"
resolved "https://registry.yarnpkg.com/find-my-way/-/find-my-way-2.2.5.tgz#86ce825266fa28cd962e538a45ec2aaa84c3d514"
We are using old
Restifyversion blocked to upgrade it. But I've realized bothRestifyandFastifyusingfind-my-wayfor the routing. Is there any way to prioritize it?
Would you like to work on this or fund the development of a fix?
I added a test for this issue #260. I guess this issue can be closed.