Probably the most cringy pattern I see when creating apps is wildcard routing. With the next version of Express changing support for wildcards, many people have already begun complaining.
const error = require("http-errors");
app.use("/thing/:id/*", function(req, res, next) {
Thing.get(req.params.id)
.then(function(user) {
if (!user) throw error(404);
req.user = user;
})
.then(next, next);
});
Contents
- What if the route doesn't exist?
- What if it's more than just a database call?
- Only define routes that are actually served
- The Koa way
- Conclusion
What if the route doesn't exist?
What if a user GET /thing/:id/dslighortghhtgo
? What would actually happen is:
- Match
/thing/:id/*
. - Get
req.user
. - Look for the next match.
- Return
404
(because there are no matches).
Your app has executed an extra database call when it didn't have to:
- Match all routes.
- Return
404
because none match.
Except this doesn't actually really work with wildcard routing.
What if it's more than just a database call?
If you add a multipart parser that downloads disks to files like this:
app.use(bodyParser.multipart())
If an attacker simply posts a bunch of files to any route, your server would quickly be flooded with files and run out of disk. This is one of the main reasons the multipart parser was removed from Connect and Express. Because implementing multipart is controversial, it's better not to provide the parser at all.
Only define routes that are actually served
Your routes should really just look like:
app
.route("/things")
.get()
.post();
app
.route("/thing/:id([0-9a-f]{24})/comments")
.get()
.post();
The router itself knows whether a path 404s and resolves the routes accordingly. Unfortunately, this is annoying to do as your app will quickly look like this:
app
.route("/things")
.post(authenticate(), bodyParser.multipart(), function(req, res, next) {
// only download files after authentication.
});
app
.route("/things/:id")
.patch(authenticateUser, getThing, bodyParser.json(), function(req, res, next) {
// update user
})
.get(getThing, function(req, res, next) {
res.json(req.thing);
});
The Koa way
It becomes very complex and ugly to route properly Express because of the extensive use of callbacks. It's simply not an option to have every route have 10 nested callbacks. But with Koa or any future async / await
framework, things will be much easier when your code looks like this:
const parse = require("co-body");
// NOTE: koa doesn't actually provide a router
app.post("/things", function*(next) {
let user = yield authenticate(this);
this.assert(user, 401, "You must be signed in!");
this.assert(user.permission.post, 403, "You cannot post!");
let body = yield parse(this);
this.assert(body, 400, "Invalid body!");
this.assert(body.name, 422, "Invalid name!");
let thing = yield Thing.create(body);
this.body = thing;
});
Conclusion
Try to avoid using wildcard routing. If you're using Express, prepare for some possible breaking changes in 5.x
. If you're using a trie router, don't even bother with it.