'/' route breaks strict routing

[sgentle]

Hi there,

Currently, if you define a route like this:

route = require('express').Router({strict: true});
route.get('/', function(req, res) {
  res.send('hi');
});

And use() that in an express application like this:

app = require('express')();
app.use('/strict/', route);

You will receive a 200 when requesting /strict/ and /strict. I would expect only /strict/ to return a 200.

I've found an acceptable workaround by adding a check like this:

route.get('/', function(req, res, next) {
    if (req.originalUrl.slice(-1) != '/') return next();
    res.send('root with slash');
});
route.get('/', function(req, res) {
    res.send('root without slash');
});

But I think it would be less surprising if route.get('/') only worked for the path ending in / when strict routing is enabled, and perhaps route.get('') could be used for the no-slash case.

[sgnl]

try defining a route for '/strict' as well, I think that's the functionality of strict routing, you have to provide a route to handle for both.

app = require('express')();
app.use('/strict/', route);
app.use('/strict', route);

[gabeio]

When the route gets passed to trim fix it adds the / to the /strict request... as it evaluates the path as '' when it arrives at trim fix and trimfix assures a leading slash.

[gabeio]

https://github.com/strongloop/express/blob/master/lib/router/index.js#L247 the req.url is '' but becomes '/' after.

[gabeio]

It seems that settings in the router are not being passed to express... as such the strict setting in the router is being ignored, it apparently is expecting express's .enable("strict routing") for anything .use related.

[arjunrp]

@sgentle Its working perfectly for me(https://gist.github.com/arjunrp/dba902fe3c1a0dba6f8d). Can u give a sample code?

[dougwilson]

@arjunrp sample code is in the first post. all the code is one example, just split up by the explanation.

[arjunrp]

@dougwilson I checked it out with the latest version,its working properly

[dougwilson]

i just retried and it's still broken. here is the simplest example to demonstrate:

var express = require('express')
var app = express()
var router = express.Router({ strict: true })

router.get('/', function (req, res) {
  res.send('hi!')
})

app.use('/strict/', router)
app.listen(4000)

GET /strict should probably be 404, but it isn't. That's what @sgentle is reporting. The issue still exists in 4.9

[danieljuhl]

Another related case...

var express = require('express');
var app = express();
var router = express.Router({ strict: true });

router.use('', function (req, res) {
  res.send('hi');
});

app.use('/strict', router);
app.listen(4000);

GET /strict should give 200 and GET /strict/ should give 404, but both give a 200. Or is there another way to route a path to the routers root?

[dougwilson]

@danieljuhl the issue is really a fundamental functionality of API guarantees within the router. The only work-around right now is to make both routers strict (your app with strict routing and the router strict) or use the work-around posted in the initial issue comment.

[danieljuhl]

@dougwilson I'm not sure, if I fully understand the work-around you're explaining. So far, I can't manage to get the result I'm looking for, no matter how many times I declare the app and the router strict. Can you provide a sample of how to accomplish this?

[dougwilson]

@danieljuhl the work-around was posted above in https://github.com/strongloop/express/issues/2281#issue-39535754 by the original user :) For the code you posted, here is the work-around added to it:

var express = require('express');
var app = express();
var parseUrl = require('parseurl');
var router = express.Router({ strict: true });

router.use('', function (req, res, next) {
  if (parseUrl.original(req).pathname !== req.baseUrl) return next(); // skip this for strictness
  res.send('hi');
});

app.use('/strict', router);
app.listen(4000);

[danieljuhl]

@dougwilson Sorry, I read your comment, as if there were two work-arounds (the one you re-posted) AND another one using strict-routing on both application and router.

[benmurrell]

+1, having the same issue. I have enabled strict routing at the app level and at the router level. Seems like this should still be classified as a bug?

[dougwilson]

It's an extremely difficult fix, but can be done, just needs some effort for probably 5.0. I'll fix the labels :)

[benmurrell]

var express = require('express');
var app = express();
app.set('strict routing', true);
var router = express.Router({ strict: true });

router.use('', function (req, res, next) {
  res.send('noslash');
});

router.use( '/', function(req, res) {
    res.send('slash');
});

app.use('/strict', router);
app.use('/strictslash/', router);

app.listen(4000);

For the code above, here are some test cases that can be used by whoever works on this: GET /strict -> noslash GET /strict/ -> noslash (expect slash) GET /strict// -> noslash (expect 404) GET /strictslash -> noslash (expect 404) GET /strictslash/ -> noslash GET /strictslash// -> noslash (expect 404 or slash - not sure) GET /strictslash/// -> noslash (expect 404)

[blakeembrey]

@dougwilson Just FYI, all these issues are fixed on latest path-to-regexp (I fixed it last July) and should be possible to backport. Also, in the latest path-to-regexp you no longer need to do the req.url || '/' hack.

Edit: Sorry, you're right. Forgot about .use('/test', route) and route.get('/') still working with /test. However, there's enough catches there that you hopefully can just do if (!options.strict) { req.url = req.url || '/'; }

[blakeembrey]

@dougwilson Just wanted to clarify the previous message. It's actually all provided out of the box with the latest path-to-regexp version and those changes could be back-ported (maybe some odd incompatibility though). I realised that a / regexp that's not strict will actually match an empty string anyway. Same with the reverse definition. So it should pass all expected tests in strict and non-strict.

[dougwilson]

The issue here is with Express and the way the router works, rather than anything with path-to-regexp :) Essentially what happens in the reported case is that req.url will never, ever be set to an empty string by Express, but it's not a possible valid value, ever. Instead if Express's router is about to set req.url to an empty string, it sets it to / instead. The sub router will not pick up the fact that there was no trailing slash is the issue. That can be fixed by Express by mucking about within req.originalUrl.

[blakeembrey]

I've looked through the code and found that before, but why wouldn't you consider an empty string to be valid? It's factually correct (/test vs /test/) and would be matched still with the single slash regexp in the latest version. I just wanted to confirm that it's all currently possible without mucking with the path logic and you can fully rely on the regexp to be correct now.

[dougwilson]

but why wouldn't you consider an empty string to be valid?

Because it's the contact of the req.url API from Node.js. The first line of an HTTP request is in the form <method> <path> HTTP/<version><CR><LF>. req.url refers to whatever the exact string that appears in the <path> portion and GET HTTP/1.1 (that is, GET<space><space>HTTP/1.1, because GitHub seems to ignore the double-space when rendered) is a syntax error (where the <path> would resolve to an empty string). As such, we will not violate the expectation that req.url cannot be an empty string.

[blakeembrey]

Interesting, thanks. I wasn't sure if this would be treated differently with nested routing.

[dougwilson]

Nope :) We want the nested stuff to think they are not even nested, which really helps with reusable code. serve-static does similar req.originalUrl "mucking" that I can pictures the router doing to be able to handle this case. Express may very well decide to pass something into path-to-regexp regular expressions that are not always exactly req.url, if it works :)

[xytis]

Hi, I'm having a similar issue in 4.x branch: https://github.com/expressjs/express/blob/4.x/lib/router/index.js#L466

Poked around the code base for reasons behind this:

path-to-regex considers 'strict' an option which means: "Match ^path$ instead of ^path(?|/?)$" While express expects 'strict' only influence trailing slash.

Even if I enforce 'strict' inheritance in this particular line -- I end up with middleware no longer working on child paths ('/path/sub/' no longer triggers middleware mounted on '/path/', since path-to-regex with 'strict' no longer matches).

[dougwilson]

Hi @xytis, I'm not sure what you are reporting, I'm sorry. That line forces strict mode for a specific reason, and it is not meant to be changed. If you can please open a new GitHub issue (https://github.com/expressjs/express/issues/new) and describe your issue, and even better if you provide code we can use to reproduce the issue with the current version of Express, we would be happy to look into the issue (but please make a new issue).

[xytis]

Code example:

app.enable('strict routing');
router = ...
app.use('/path/', router);

From documentation I would expect that my application will not serve "/path" endpoint, yet it gets forwarded to given router.

Line of code that I reference basically says "do not perform strict routing on internal layers". Which, by my understanding, is exactly what I expect in above example. I went deeper into path-to-regex of currently used version found the inconsistency in "strict" definition, which I mentioned earlier.

I am not sure if this requires a separate issue, since, by the looks of it, it will be fixed as soon as express updates it's dependancy of path-to-regex.

[dougwilson]

.From documentation I would expect that my application will not serve "/path" endpoint, yet it gets forwarded to given router.

Unfortunately the documentation in this case is simply wrong. The strict routing feature in no way affects paths under app.use, and it is not intended to. This issue is definitely different from what you are describing.

I am not sure if this requires a separate issue, since, by the looks of it, it will be fixed as soon as express updates it's dependancy of path-to-regex.

It will not, because app.use is not affected by strict routing, and will always act in a non-strict manor.

There is a lot of off topic chatter in this thread, and some is issues like yours that are actually misunderstandings and some are of the original bug. The bug is only the code in the initial post. It is a bug because router.get is being used, which absolutely should be enforcing strict routing. Your code is not utilizing any methods that actually are affected by strict routing.

[tilleps]

If you don't care about supporting empty paths, using the below middleware seems to enforce strict routing:

//  Results:
//
//  GET /strict -> (404)
//  GET /strict/ -> slash
//  GET /strict// -> (404)
//  GET /strictslash -> (404)
//  GET /strictslash/ -> slash
//  GET /strictslash// -> (404)
//  GET /strictslash/// -> (404)

const express = require("express");

const app = express();
app.set("strict routing", true);

const router = express.Router({ strict: true });

//  Middleware to enforce strict URLs
//  Note: must be applied to the router (and not app)
router.use(function strict(req, res, next) {
  if (req.app.get("strict routing")) {
    req.url = req.originalUrl.substring(req.baseUrl.length);
  }
  next();
});

//  This solution does not support empty paths like these:
//  router.use("", function (req, res, next) {
router.all("", function (req, res, next) {
  res.send("noslash");
});

//  Note: using .use() will not work, use methods instead (all, get, post, etc):
//  router.use("", function (req, res, next) {
router.all("/", function(req, res) {
    res.send("slash");
});

app.use("/strict", router);
app.use("/strictslash/", router);

app.listen(4000);

[dbauszus-glx]

@dougwilson Is there any more information in regard to 'mucking about' with the original url?

We really would like to use ourcompany.com as address and not ourcompany.com/

To be fair we use express mostly in a debug environment and the serverless routing support routes without trailing slashes. It would be great if the same links work in the express debug environment.

[dougwilson]

Hi @dbauszus-glx sorry you are having issues. Your description, to me, sounds like a more specific bug instance. Would you be willing to open a new issue about this and include a reproduction case? I believe for your specific scenario ("We really would like to use ourcompany.com as address and not ourcompany.com/") it is the Node.hs HTTP server that is at issue here which is why it works serverless (it's not used) vs not (we use it to parse the HTTP requests).

[dbauszus-glx]

@dougwilson I will have to come back to this once a I get a chance to deploy an express instance. At the moment we use Express only for debugging which means I can only describe the problem with localhost which is probably not very helpful. I understand that req.url may not be empty and would expect app.get("", handler) to return localhost:3000/. I am only surprised why req.url is empty when I set app.get("/root", handler) and returns localhost:3000/root instead of localhost:3000/root

I will do a bit more reading on this and get back on this once I am better capable of explaining the issue. It's very low impact for now and I plan to use a micro router for my debug case in the future.

[skaziweb]

I use middleware for add / and redirect all routes in new address. I fink it's possible to use in strict mode.

app.use((req, res, next) => {
  const url = req.url;
  const fileRegExp = new RegExp(/\./);
  const isParams = Object.keys(req.params).length;
  const isQuery = Object.keys(req.query).length;
  if(url.length && fileRegExp.test(url) || isParams || isQuery){
    next();
  } else if(url.length && url.slice(-1) != '/') {
    res.redirect(301, `${url}/`);
  } else {
    next();
  }
});