Express 5.0.0 Route with regex not working

[HarishGangula]

We have created simple express application with following code

const express = require('express')
const app = express()
const port = 4000

function logErrors(err, req, res, next) {
    console.error(err.stack)
    next(err)
}

function clientErrorHandler(err, req, res, next) {
    if (req.xhr) {
        res.status(500).send({ error: 'Something failed!' })
    } else {
        next(err)
    }
}
function errorHandler (err, req, res, next) {
   
    res.status(err.statusCode || 500)
    res.json({ message: err.message || "something wrong" })
}

app.get('/', async (req, res) => {
    await Promise.reject({"message":"dsfjh", statusCode: 400})
    res.status(200).send("asfas")
})

app.get('/[discussion|page]/:slug', async (req, res) => {
    throw {statusCode: 400, message: "harish"}
    res.status(200).send("asfas")
})


app.use(logErrors)
app.use(clientErrorHandler)
app.use(errorHandler)

app.listen(port, () => {
    console.log(`Example app listening on port ${port}`)
})

It is throwing following error

~/express5/node_modules/path-to-regexp/dist/index.js:136
        throw new TypeError(`Unexpected ${nextType} at ${index}, expected ${type}: ${DEBUG_URL}`);
        ^

TypeError: Unexpected [ at 1, expected END: https://git.new/pathToRegexpError
    at Iter.consume (~/express5/node_modules/path-to-regexp/dist/index.js:136:15)
    at consume (~/express5/node_modules/path-to-regexp/dist/index.js:193:16)
    at parse (~/express5/node_modules/path-to-regexp/dist/index.js:197:20)
    at ~/express5/node_modules/path-to-regexp/dist/index.js:308:74
    at Array.map (<anonymous>)
    at pathToRegexp (~/express5/node_modules/path-to-regexp/dist/index.js:308:25)
    at Object.match (~/express5/node_modules/path-to-regexp/dist/index.js:278:30)
    at matcher (~/express5/node_modules/router/lib/layer.js:83:23)
    at new Layer (~/express5/node_modules/router/lib/layer.js:90:62)
    at Function.route (~/express5/node_modules/router/index.js:421:17)

We identified that path to regexp node module is broken which is causing this issue.

Node.js v20.10.0 used

[thomashohn]

I have seen a similar issue with express 5.0 - which seems have root cause in path-to-regexp module:

/some-repo/node_modules/path-to-regexp/dist/index.js:85
throw new TypeError(Missing parameter name at ${i}: ${DEBUG_URL});
^

TypeError: Missing parameter name at 1: https://git.new/pathToRegexpError
at name (/some-repo/node_modules/path-to-regexp/dist/index.js:85:19)
at lexer (/some-repo/node_modules/path-to-regexp/dist/index.js:103:27)
at lexer.next ()
at Iter.peek (/some-repo/node_modules/path-to-regexp/dist/index.js:119:38)
at Iter.tryConsume (/some-repo/node_modules/path-to-regexp/dist/index.js:125:28)
at Iter.text (/some-repo/node_modules/path-to-regexp/dist/index.js:141:30)
at consume (/some-repo/node_modules/path-to-regexp/dist/index.js:166:29)
at parse (/some-repo/node_modules/path-to-regexp/dist/index.js:197:20)
at /some-repo/node_modules/path-to-regexp/dist/index.js:308:74
at Array.map ()

[ex1st]

Is it realtes to https://blakeembrey.com/posts/2024-09-web-redos/ and GHSA-9wv6-86v2-598j?
But I don't understand, why the major version of "path-to-regexp" was updated when express@5 was released?

[thomashohn]

I agree - seems "weird"..

[SherbekMavlonov]

#5936 (comment)

[wesleytodd]

We fully dropped support for regular expressions in strings for routes. You can check out the docs for path-to-regexp@8 in that repo. We will be putting together a migration guide for folks, but in this case you need to either use an array of paths or write your own regular expression (very discouraged).

So it would become this:

app.get(['/discussion/:slug', '/page/:slug'], async (req, res) => {
    throw {statusCode: 400, message: "harish"}
    res.status(200).send("asfas")
})

[ex1st]

@wesleytodd, sad to hear, we used regexp for data type routing.

app.get('/product/:page([0-9]+)', async (req, res) => {

[wesleytodd]

We will be recommending folks use a more robust input validation approach for this. If you want, here is the open api library I maintain: https://github.com/wesleytodd/express-openapi/

The reason for this is mainly security around DOS protection. There is no way to support path segment regular expression matching while also avoiding major performance regressions. See more here: https://blakeembrey.com/posts/2024-09-web-redos/

This is a 100% big win for the ecosystem longer term, as regular expression route matching was one of the worst decisions from the early days which has stuck around. So while we understand the pain this change will cause, we stand by this decision as the right thing for the ecosystem and all express users (even if you need to change your approach to this sort of routing).

[blakeembrey]

@HarishGangula I do want to include as an aside here that '/[discussion|page]/:slug' did not do what you expected in express 4 either, since [] is a regex character group, so this was matching /d/x for example.

Due to time constraints I haven't had the capacity to write a version that supports parsing of regex characters into something that can be guaranteed safe to avoid ReDoS, but that's not a forever situation. They've been reserved so it's possible to add it back again later.

@ex1st Totally understand the concern, it's my biggest let down too. For now you'd have to include it in the method and call next() when it doesn't match a number. This is both good and bad, since it'll motivate people to use better validation libaries for more complex regex cases instead of trying to cram it into the path itself, but obviously for something like yours it looks like a regression.

[HarishGangula]

@blakeembrey This is one of the example but we have routes which uses regex like at end all routes i want keep '*' as route and send specific response instead of express default one.

[blakeembrey]

If you want to use a regex it’s fine to write a regex directly, it’s just not supported in the string.

[richardsimko]

The migration guide explicitly mentions using regex as a string, but I take it that's not correct seeing as the example provided doesn't work (/:foo(.*))?

https://expressjs.com/en/guide/migrating-5.html#path-syntax

The following changes have been made to how the path string is matched to an incoming request:

[HarishGangula]

#5948 (comment)
This solution is working fine. Thanks @richardsimko @blakeembrey for the quick help.

[wesleytodd]

I take it that's not correct

Yes, that guide needs updating. Please submit a PR on https://github.com/expressjs/expressjs.com