nest: MiddlewareConsumer routing is broken

Bug Report

Current behavior

The MiddlewareConsumer routing is broken when you define home route: / All routes mapped to the same path ‘/’

Input Code

Routing module:

export class RoutingModule implements NestModule {
  configure(consumer: MiddlewareConsumer) {
    consumer
      .apply((req, res) => res.send('hit path /'))
      .forRoutes({ path: '/', method: RequestMethod.GET })
      .apply((req, res) => res.send('hit path /test/app'))
      .forRoutes({ path: '/test/app', method: RequestMethod.GET })
      .apply((req, res) => res.send('hit path /:username'))
      .forRoutes({ path: '/:username', method: RequestMethod.GET });
  }
}

App module:

import { Module } from '@nestjs/common';
import { RoutingModule } from './routing.module';

@Module({
  imports: [RoutingModule],
})
export class AppModule {}

Get request to the ‘/test/app’ and ‘/:username’ prints ‘hit path /’ Feels like ‘/’ defined as ‘*’

Expected behavior

The /test/app and /:username routes should be accessible

Possible Solution

Environment


"@nestjs/common": "^8.0.6",
"@nestjs/core": "^8.0.6",
"@nestjs/platform-express": "^8.0.6"

 
For Tooling issues:
- Node version: 14  
- Platform:  Mac/Linux 

Others:

This issue is not reproduced in NestJS 7.X.X - started in 8.X.X.

This flow works as expected with the Controllers:

@Controller()
export class AppController {
  @Get('/')
  homepage(): string {
    return 'hit /';
  }

  @Get('/test/app')
  username(): string {
    return 'hit /test/app';
  }

  @Get('/:username')
  users(): string {
    return 'hit /:username';
  }
}

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 28 (16 by maintainers)

Most upvoted comments

@jmcdo29 I don’t think you’re right. If I register two routes, let’s say /:username and /users so route /users should be registered first. When I register just / why it should be last? This route regex is not overriding /something. And as I said in the bug description:

Is not reproduced with controllers
Is not reproduced in NestJs 7.X.X
Is not reproduced with a pure Express server
Is not behave like this in any other language and framework

oleg-yudovich on Sep 23, 2021

This should be fixed in 8.0.11

kamilmysliwiec on Oct 6, 2021

@Zclhlmgqzc I am running a simple express server (express 4.17.1, the same version used in NestJS):
import express from 'express'

const app = express()

app.get('/', (req, res) => {
  res.send('home')
})

app.get('/about', (req, res) => {
  res.send('about')
})

let server = app.listen(3000, () => {
  console.log(`server running at port http://localhost/${server.address().port}`)
})
route http://localhost:3000/about hit /about

app.use != app.get post …

Zclhlmgqzc on Sep 24, 2021

@jmcdo29 I don’t think you’re right. If I register two routes, let’s say /:username and /users so route /users should be registered first. When I register just / why it should be last? This route regex is not overriding /something. And as I said in the bug description:

Is not reproduced with controllers

Is not reproduced in NestJs 7.X.X
—> GET /path 7.x app.get(/path) (Router-level middleware)
now app.use( /path req.method GET )

~~Is not reproduced with a pure Express server~~ —> see #7737 app.use( req.method GET )

Is not behave like this in any other language and framework —>fastify middleware fastify.use( req.method GET )

http://127.0.0.1:3001/test => hit path /

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

app.use('/', function (req, res, next) {
    if (req.url === '/') {
        console.log('isRequestMapping')
    }
    if (req.method === 'GET') {
        res.send('hit path /')
    }
})

app.use('/test', function (req, res, next) {
    if (req.url === '/') {
        console.log('isRequestMapping')
    }
    if (req.method === 'GET') {
        res.send('hit path test')
    }
})

app.listen(3001)

http://127.0.0.1:3001/test => hit path /test

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

app.use('/test', function (req, res, next) {
    if (req.url === '/') {
        console.log('isRequestMapping')
    }
    if (req.method === 'GET') {
        res.send('hit path test')
    }
})

app.use('/', function (req, res, next) {
    if (req.url === '/') {
        console.log('isRequestMapping')
    }
    if (req.method === 'GET') {
        res.send('hit path /')
    }
})

app.listen(3001)

the bug description:

Current behavior

forRoutes({ path: ‘/’ }, { path: ‘/test/app’ }, { path: ‘/:username’ }) === forRoutes(AppController)

Expected behavior

forRoutes(AppController) should be equal to [{ path: ‘/$’ }, { path: ‘/test/app$’ }, { path: ‘/:username$’ }]

forRoutes({ path: ‘/’ }, { path: ‘/test/app’ }, { path: ‘/:username’ }) should be not equal to forRoutes(AppController)

Zclhlmgqzc on Sep 24, 2021

Wouldn’t this be due to how regex matches work? i.e. if you want to hit just / then you either need to bind that last or use /$ as the regex to signify the end of the string?

jmcdo29 on Sep 22, 2021