expressjs / express

Fast, unopinionated, minimalist web framework for node.
https://expressjs.com
MIT License
64.99k stars 15.51k forks source link

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

Open gebbber opened 3 years ago

gebbber commented 3 years ago

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 //.

Spekpannenkoek commented 3 years ago

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.

rodion-arr commented 3 years ago

Hi @gebbber, does answer above addresses your issue?

gebbber commented 3 years ago

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:

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.

dougwilson commented 3 years ago

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 👍