Optional parameter issue with NON-trailing double slash

[lordbah]

Bug Report

Double slash confuses parameter parser.

Restify Version

5.1.0

Node.js Version

8.2.1

Expected behaviour

Don't match this pattern.

Actual behaviour

Matched, and assigned later part of URL to earlier parameters.

Repro case

const restify = require('restify');
const server = restify.createServer();

server.put('/products/:catalog/:item', function(req, res, next) {
console.log(`URL=${req.url}`);
console.log(`catalog='${req.params.catalog}' item='${req.params.item}'`);
});

server.listen(8989, () => {
console.log('listening');
});

Then used Postman to send some PUT requests with these results:
URL=/products/one/two
catalog='one' item='two'
URL=/products//two
catalog='' item='two'
URL=/products//two/three
catalog='two' item='three'

The first two results are what I expect. In the third case I expect no match.

/products/one/two/three was of course not matched by this pattern. The third case should be the same except that it should match an empty string to catalog. The second case shows that it is capable of matching an empty string.

Cause

I don't know.

Are you willing and able to fix this?

No. I mean, at my current level of Javascript experience I don't think you want me messing around in there.

[DonutEspresso]

Hi @lordbah, thanks for the report! This certainly does appear to be unexpected behavior. I'll have to take some time to look into what's happening.

In the meantime, a possible work around for you might be to opt-in to URL sanitization by using the dedupeSlashes plugin:

server.pre(restify.plugins.pre.dedupeSlashes());

This will turn /products//two into /products/two, and /products//two/three into /products/two/three, which will allow you to entirely bypass this somewhat ambiguous behavior.

[hekike]

@DonutEspresso here is a fix: #1478
But non-trailing slashes were tested before for skipping, we should agree about the expected behavior first.

I believe with this change it would be more consistent.
What do you think?

[retrohacker]

Closing this as resolved by #1512

If this is not the case, please comment and we can re-open 😄