express icon indicating copy to clipboard operation
express copied to clipboard

`App.use(path, ...)` Does Not Mount at `'//'`

Open gebbber opened this issue 3 years ago • 4 comments

Router instance mounted explicitly at '//' exposes routes when path begins only with a single /:

const TestRoutes = express.Router();

TestRoutes.get('/drop/table/:table', (req, res) => { 
  // drop table
  res.send(`Dropped table '${req.params.table}'`);
})

TestRoutes.get('/', sendListOfTestRoutes);

if (TEST) app.use('//', TestRoutes);

app.listen();

Expected behavior is that navigating to http://hostname/ would bypass this Router and go to the main app, but instead Express serves the list of test routes mounted within the router at //.

gebbber avatar Mar 28 '21 16:03 gebbber

I didn't dive too deep into this, but when copying and slightly adapting your example code I noticed that it all seems to work as intended depending on if you mount your main app before or after app.use('//', ...). Your observed behavior seems to happen when the main app's routes are added after. If added before the subrouter is mounted, / goes to the main app, // to a test route.

Mounting at /\/\// (regular expression vs '//') seems to work regardless of what line you put it but then you run into the problem that you need to access TestRoutes.get('/drop/table/:table', ...) via http://localhost///drop/table/test, with three slashes.

app.set('strict routing', true) didn't seem to help.

Spekpannenkoek avatar Mar 31 '21 12:03 Spekpannenkoek

Hi @gebbber, does answer above addresses your issue?

rodion-arr avatar Mar 31 '21 22:03 rodion-arr

Mounting at /// seems to provide the same result. I've currently implemented a workaround like:

Router.get(`/`, sendListOfTestAdminRoutes);
Router.get(`/adminRoute1`, adminRoute1);

app.use(`///`, Router); // mount at '//'

This mounts the routes at:

  • http://host// for the 'list'
  • http://host///adminRoute1 for the specific route

I would have expected it to mount the second route at http://host//adminRoute1.

This is acceptable as a workaround in my project for now, because in sendListOf... the links are generated programmatically and the third / can be coded in, but the behavior is not intuitive and could confuse me again if I need to come back to it later.

Admittedly, this seems like a pretty obscure use case. Once I saw that // is a legal route, though, it seemed like a clean choice for this type of application.

gebbber avatar Mar 31 '21 23:03 gebbber

Hi @gebbber thanks for the report. I was able to reproduce the issue. I think this is similar to #3798 ; ideally your app should work like you expect when enabling strict routing, but in Express 4.x the strictness does not affect app.use statements. It may not be possible to fix this within 4.x due to breakages, but I think we could probably fix it within the 5.x series before it releases. I am currently back to work on getting the deps released to spin out the 5.x release I think within a few weeks or something. So if someone wants to take a stab at it, the sooner the better to land 👍

dougwilson avatar Apr 01 '21 00:04 dougwilson