delvedor/find-my-way

Bug: Broken routing for aliased routes ending with `<...>/::something` & `<...>/:some_var`

darkuranium opened this issue · 4 comments

Versions tested:

  • 4.5.1 (Fastify dependency: "find-my-way": "^4.5.0")
  • 5.2.0 (latest)

Both contain the bug, but the symptoms are different. The bug is more severe in 5.2.0.

TL;DR: There appears to be a bug for pairs of routes, one ending with /::something and the other with /:some_var. The exact behavior depends on the find-my-way version (see "Observed behavior" below), but is incorrect in either case. See "Failure MCVE" below.

Prefixes other than :: seem to work correctly (tested: ~, @, $). See "Working MCVE" below.


Failure MCVE: (index.mjs)

import http from 'http';
// npm install --save-prod find-my-way@4.5.1 or find-my-way@5.2.0
import FindMyWay from 'find-my-way';

const router = FindMyWay();

router.on('GET', '/:world_name/::articles', (req, res, params) => {
    res.end('matched /:world_name/::articles\n' + JSON.stringify(params, null, 2));
});
router.on('GET', '/:world_name/:article_name', (req, res, params) => {
    res.end('matched /:world_name/:article_name\n' + JSON.stringify(params, null, 2));
});

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');
});

Observed behavior v4.5.1: GET /myworld/:articles request matches the second (/:world_name/:article_name) route (see below).
Observed behavior v5.2.0: Both GET /myworld/:articles and GET /myworld/somearticle result in a 404 Not Found error. In other words, neither route works, as if they did not exist.
Expected behavior: The above request should match the first (/:world_name/::articles) route. Additionally (v5.2.0): The second route should still work as normal for /myworld/somearticle.
Partial workaround: No known workaround on my part for the exact same route, but changing ::articles to a different prefix symbol (e.g. ~articles) works in both 4.5.1 and 5.2.0.

GET /myworld/:articles:

matched /:world_name/:article_name
{
  "world_name": "myworld",
  "article_name": ":articles"
}

Success MCVE: (modified failure example, by changing ::articles to ~articles in the route)

router.on('GET', '/:world_name/~articles', (req, res, params) => {
    res.end('matched /:world_name/~articles\n' + JSON.stringify(params, null, 2));
});
router.on('GET', '/:world_name/:article_name', (req, res, params) => {
    res.end('matched /:world_name/:article_name\n' + JSON.stringify(params, null, 2));
});

… (correctly) results in the following being matched for a GET /myworld/~articles:

matched /:world_name/~articles
{
  "world_name": "myworld"
}

You can't have ambiguous behavior. If : is used to detect the start of param, you can't use it as a prefix.

I'm not sure if we support it, but having /something/\::foo would be nice. What's the syntax of express for this case?

You can't have ambiguous behavior. If : is used to detect the start of param, you can't use it as a prefix.

I don't see the ambiguity here. According to the readme of this project, :: is the way to escape a :. Thus, /::articles should mean a literal /:articles in the URL.

Quoted from the readme:

If you just want a path containing a colon without declaring a parameter, use a double colon. For example, /name::customVerb will be interpreted as /name:customVerb

The only ambiguity is between /:some_param and /something_not_a_param, but this case is (to my knowledge, and definitely behavior-wise) handled by find-my-way anyway (by first checking the "static" routes).

Come think of it, using /:article_name and then checking if(params.article_name === ':articles') is a viable workaround; but it is a bit ugly, as it essentially means having another layer of routing on top of the existing router.

The invite stands: would you like to send a PR?

The invite stands: would you like to send a PR?

I can maybe look into it in a few days, but can't be sure I'll be able to, as my schedule is currently very busy.
So: I'll see what I can do, but no promises are made.