Express: Express middleware not honoured if there is a duplicate slash (//)

[micophilip]

Hello,

I'm running express v 4.16.2 and I have a middleware routing that looks as follows:

app.use('/path/to/my/resource/*', middlewares.myMiddleware);

If there's an extra / in certain spots, the middleware is bypassed.

Middleware is bypassed in the following cases:

/path/to/my//resource/resourceId
/path/to//my/resource/resourceId

Middleware is not bypassed in the following cases and works as expected:

//path/to/my/resource/resourceId
/path//to/my/resource/resourceId
/path/to/my/resource//resourceId

[dougwilson]

I think this is a path-to-regexp issue, as it generates the regular expressions to match the paths against.

[dougwilson]

https://github.com/pillarjs/path-to-regexp

[micophilip]

@dougwilson Thanks for the quick response. Do you suggest I open an issue against that package? Or is there someone on express team that can confirm it is an issue with path-to-regexp?

[dougwilson]

I was just trying to provide a quick response that may help point to the root cause. I have not invested anything yet. Someone (you could if you're inclined / able) needs to track down the technical cause of this issue and figure out either (a) is this expected behavior (b) what the specific fix is or (c) something else.

I hope this helps.

[micophilip]

Here's a simple app that I used to reproduce the issue:

var express = require('express');
var app = express();
var router = express.Router();

app.use('/path/to/my/resource/*', function(req, res, next) {
  console.log('in middleware');
  return next();
});

app.use('/path/to', router.use('/my', router.get('/resource/:resourceId', function(req, res) {
  res.send();
})));

app.listen(3009);

If I do localhost:3009/path/to//my//resource/resourceId, I get 200, but I don't get the print statement (i.e, it didn't go through the middleware).

[mintunitish]

Hey I would like to take this issue up. Any pointers where to start?

[micophilip]

Thanks @mintunitish for offering to help. I ran the above example in debug mode and express creates a separate layer for every route used. So what happens is as follows:

1st layer app.use('/path/to/my/resource/*')
path: /path/to/my//resource/resourceId
regex: /^\/path\/to\/my\/resource\/(.*)\/?(?=\/|$)/i
match: false

2nd layer app.use('/path/to')
path: /path/to
regex: /^\/path\/to\/?(?=\/|$)/i
match: true

3rd layer router.use('/my')
path: /my/ please note the trailing slash
regex: /^\/my\/?(?=\/|$)/i
match: true

4th layer router.get('/resource/:resourceId')
path: /resource/resourceId
regex: /^\/resource\/(?:([^\/]+?))\/?$/i
match: true

I expected the 4th layer path to be //resource/resourceId. Because express doesn't attempt to match the path as a whole, but matches them in chunks, it depends on the code structure whether express matches the pattern or not. For example if I had router.get('/path/to/my/resource/:resourceId', function(req, res){}) and I attempt localhost:3009/path/to//my//resource/resourceId I'd get 404 as expected. However listing routers as full routes like router.get('/path/to/my/resource/:resourceId', function(req, res){}) does not allow for better encapsulation.

[wesleytodd]

I would move this conversation over to the path-to-regexp repo. I am sure @blakeembrey will have input and opinions, and no matter the change you land on, pulling that into the express release will be a separate issue.

[blakeembrey]

I believe this works as expected in path-to-regexp >= 1, it was one of the first things I resolved when I refactored. I didn't alter this behaviour in the version that Express.js uses because it would have broken some layer of compatibility that someone _may_ have been using.

[blakeembrey]

However, based on the comments, I can't really tell what you expect here. To change the behaviour to purposefully match duplicate slashes is a different change than making it act consistently, which is the behaviour I mentioned in my previous comments. If you _want_ it to match multiple slashes as if they were one, that would require a change (but could, for now, be resolved with a middleware that rewrites URLs).

[Emuentes]

@micophilip Were you able to resolve this issue?
I created a Repl with the version of Express that you noted so that I could test the cases you said were failing but in both cases I ended up seeing the "in middleware" log printed.
https://repl.it/@ejmuentes/expressissues3752
https://imgur.com/UlLMSGS

If you were able to sort the issue out and you have a moment to clarify what your solution was, it may help others landing on this issue from a search engine.

Either way, if this issue no longer needs attention from the Express maintainers, please close this issue.

[micophilip]

@Emuentes Thanks for checking in. The issue is still reproducible actually with the latest express 4.17.1. However I restructured my middlewares in a way where it's not reproducible as illustrated in my earlier comments. Closing the issue.

[Emuentes]

@micophilip glad to hear you were able to refactor the problem away.

If you find a solution using the approach you were trying to implement, add a codesandbox example for any folks who may have a similar issue :-D

If you have example code (that you can share) illustrating what you did as an alternative, it may be helpful to others who find stumble upon this issue.

Share a running code example if you have a chance :-)