Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Route: leading * screws named params #1341

Closed
mgurov opened this Issue Sep 22, 2012 · 9 comments

Comments

Projects
None yet
4 participants

mgurov commented Sep 22, 2012

Mixing plain wildcards with named parameters in routes produces inconsistent results.

works more or less ok when the * is after named parameters:

app.get('/file/:name/*', function(req, res){
    console.log('tailing *', req.params);
    res.end(req.params.name);
});

...
request(app).get('/file/name/blah/fooe').expect('name', done); //succeeds
> tailing * [ 'blah/fooe', name: 'name' ]

while same exercise fails with leading asterisk:

app.get('*/file/:name/', function(req, res){
    console.log('leading *', req.params);
    res.end(req.params.name);
});

...
request(app).get('/context/subcontext/file/name').expect('name', done); //fails
> leading * [ 'name', name: '/context/subcontext' ]
Technical notes

The reason for this behaviour is that the keys parameter is not touched for the plain wildcards in utils.pathRegexp function and captures later are assigned incorrectly at Route.prototype.match (route.js).
Fixing this might be tricky, unless managed to inject .replace(/\*/g, '(.*)') into the main replace statement (utile.pathRegexp). At the very least, detecting such mixture would not be too difficult and issuing warning / error could be an option.

Workaround

Hand-crafted regexp works of course, but at a price of less readability and index-based param referencing:

app.get(/.*\/file\/(.*)/, function(req, res){
    res.end(req.params[0]);
});
...
request(app).get('/context/subcontext/file/name').expect('name', done); //works

mgurov added a commit to mgurov/express that referenced this issue Sep 24, 2012

mgurov added a commit to mgurov/express that referenced this issue Sep 24, 2012

Is this a real use case scenario though? I mean matching all urls that have /file/something anywhere inside of them ?

Owner

tj commented Oct 17, 2012

im not too concerned about this personally, we can't possibly account for everything a regexp can do, unfortunately we don't have named capture groups in js! that would be nice

@visionmedia actually xregexp module enables named capture groups. I've used them in https://github.com/web-napopa/node-reversable-router.

@mgurov i still don't know whether this is real world use. e.g. something in the middle or start or end of anything.

Owner

tj commented Oct 17, 2012

@antitoxic yeah but that's a dirty hack.. I don't include stuff like that in cores of projects

@visionmedia i haven't looked into the source but performance seems to remain the same. what do you have in mind for the dirty hacks? im really interested as i don't want to be doing work using bad practise! i just thought there some good people promoting this project.

mgurov commented Oct 18, 2012

@antitoxic it was a real enough life for me. I am mocking a certain web service, which can potentially be attached to different paths, and for me it was much more convenient not to think about the details of * in */file/:name/. Again, there are multiple workarounds and this particular issue is not a show-stopper for me, but feels a bit of "dirty" due to the inconsistency of the behavior, IMHO.

Owner

tj commented Oct 18, 2012

there is no inconsistent behaviour though, it's always (.*)

mgurov commented Jan 28, 2013

Yes, it is always (.*), from the underlying regexp point of view. From the captured groups replacement there is some difference, and to the user of the router who got used to /x/* and /x/:name the current behavior might be unexpected.

Member

jonathanong commented Sep 10, 2013

closing as i don't think express will ever support this.

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