express icon indicating copy to clipboard operation
express copied to clipboard

5.1.0 Throws error on route with a parameter in it

Open CoryAlbrecht opened this issue 8 months ago • 8 comments

cory@redbarchetta[03:41:13]~/.../Projects/YoctoVideo/backend$ npm run dev

> [email protected] dev
> npx dotenvx run -f .env.development -f .env -- node ./src/index.js

[[email protected]] injecting env (9) from .env.development, .env
/home/cory/Documents/Projects/YoctoVideo/backend/node_modules/path-to-regexp/dist/index.js:73
            throw new TypeError(`Missing parameter name at ${i}: ${DEBUG_URL}`);
                  ^

TypeError: Missing parameter name at 1: https://git.new/pathToRegexpError
    at name (/home/cory/Documents/Projects/YoctoVideo/backend/node_modules/path-to-regexp/dist/index.js:73:19)
    at lexer (/home/cory/Documents/Projects/YoctoVideo/backend/node_modules/path-to-regexp/dist/index.js:91:27)
    at lexer.next (<anonymous>)
    at Iter.peek (/home/cory/Documents/Projects/YoctoVideo/backend/node_modules/path-to-regexp/dist/index.js:106:38)
    at Iter.tryConsume (/home/cory/Documents/Projects/YoctoVideo/backend/node_modules/path-to-regexp/dist/index.js:112:28)
    at Iter.text (/home/cory/Documents/Projects/YoctoVideo/backend/node_modules/path-to-regexp/dist/index.js:128:30)
    at consume (/home/cory/Documents/Projects/YoctoVideo/backend/node_modules/path-to-regexp/dist/index.js:152:29)
    at parse (/home/cory/Documents/Projects/YoctoVideo/backend/node_modules/path-to-regexp/dist/index.js:183:20)
    at /home/cory/Documents/Projects/YoctoVideo/backend/node_modules/path-to-regexp/dist/index.js:294:74
    at Array.map (<anonymous>)

Node.js v23.11.0
Command failed with exit code 1: /usr/bin/node ./src/index.js

Through bisecting, the offending code in my app is

import express from 'express';
import session from 'express-session';
import { sql } from 'slonik';
import { dbPool } from '../database/index.js';
const router = express.Router();

// API route for profile info
router.get('/api/users/:nickname', async (request, response) => {
  try {
    let nickname = request.params.nickname;
    // If no nickname is provided, check if the user is logged in
    if (!nickname && request.session.nickname) {
      nickname = request.session.nickname;
    }

    // If no nickname and no session, return an error
    if (!nickname) {
      response.status(401).json({ success: false, message: 'Not logged in or no nickname provided.' });
      return;
    }

    // Fetch the profile details for the given nickname
    const user = await dbPool.maybeOne(sql.unsafe`
			SELECT email, nickname, full_name, birthday FROM users WHERE nickname = ${nickname};
		`);

    // Check if the logged-in user is viewing their own profile
    const isOwner = request.session.nickname === nickname;

    // If the logged-in user is the owner, send full profile details
    if (isOwner) {
      response.status(200).json({
        success: true,
        profile: {
          email: user.email,
          nickname: user.nickname,
          fullName: user.full_name,
          dateOfBirth: user.birthday,
          isOwner: true,
        },
      });
      return;
    } else {
      // If it's a public view, send limited profile details
      response.status(200).json({
        success: true,
        data: {
          email: user.email,
          nickname: user.nickname,
          isOwner: false,
        },
      });
      return;
    }
  } catch (error) {
    console.error('Error fetching profile:', error);
    response.status(500).json({ success: false, message: 'Internal server error.' });
    return;
  }
});

export default router;

It was originally '/api/users/:nickname?', but when I updated to express 5.1.0, I changed the route path to '/api/users/{:nickname}' like the migration guide suggests. When that didn't work I got rid of the braces ('/api/users/:nickname') to make it a required parameter, but that still did not work.

Environment information

Version:

Platform:

Node.js version:

Any other relevant information:

What steps will reproduce the bug?

After trying a few arbitrary paths, it seems that any replaceable param in a 5.x router.get() path will do this.

CoryAlbrecht avatar Apr 16 '25 07:04 CoryAlbrecht

Are you sure that this path is the problem? Don't you maybe have /* somewhere?

If you want to see which path causes the exception you can increase the stack trace limit (e.g. node --stack-trace-limit=50 index.js) and find the last line pointing at your source, before entering node_modules/express or node_modules/router.

krzysdz avatar Apr 16 '25 09:04 krzysdz

I am facing the same error with express.static, with previous version the code below worked fine but it no longer works with the 5.x version

app.get("*", (req, res) => {
      res.sendFile("index.html", { root: "build/dist/" });
    });

AB-70 avatar Apr 22 '25 21:04 AB-70

I am facing the same error with express.static, with previous version the code below worked fine but it no longer works with the 5.x version

app.get("*", (req, res) => {
      res.sendFile("index.html", { root: "build/dist/" });
    });

try this instead

app.get(/(.*)/, (req, res) =>  { 
    res.sendFile("index.html", { root: "build/dist/" });
});

gianlucca avatar May 09 '25 01:05 gianlucca

Thanku brooo

aibhavesh avatar May 16 '25 14:05 aibhavesh

I am facing the same error with express.static, with previous version the code below worked fine but it no longer works with the 5.x version

app.get("*", (req, res) => {
      res.sendFile("index.html", { root: "build/dist/" });
});

Just change * to /{*something}

bjohansebas avatar May 16 '25 14:05 bjohansebas

Ah yeah, new path-to-regexp version breaking everything, let's break people's code for some made up by committee reason, good going guys.

RedShift1 avatar May 22 '25 12:05 RedShift1

@RedShift1 you can read more about the reason here and here. And if you still think we made up a reason then feel free to head on over to path-to-regexp and open up a PR bringing back the old path semantics without a ReDOS, we will happily accept your work.

wesleytodd avatar May 22 '25 13:05 wesleytodd

I wasn't aware this was an actual security issue, my bad, I apologize.

RedShift1 avatar May 22 '25 18:05 RedShift1