Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,18 @@ Router.prototype.handle = function handle (req, res, callback) {
return
}

// Validate path includes trailing slash when the mount path had one.
// loosen() strips trailing slashes for non-strict layer matching, which
// allows e.g. mount '/strict/' (loosened to '/strict') to match request
// '/strict'. However, the request should only proceed if it actually
// includes the trailing slash from the original mount path.
if (typeof layer.originalPath === 'string' &&
layer.originalPath === layerPath + '/' &&
!path.startsWith(layer.originalPath)) {
next(layerError)
return
}

// Trim off the part of the url that matches the route
// middleware (.use stuff) needs to have the path stripped
debug('trim prefix (%s) from url %s', layerPath, req.url)
Expand Down
1 change: 1 addition & 0 deletions lib/layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ function Layer (path, options, fn) {
this.params = undefined
this.path = undefined
this.slash = path === '/' && opts.end === false
this.originalPath = path

function matcher (_path) {
if (_path instanceof RegExp) {
Expand Down
201 changes: 201 additions & 0 deletions test/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,11 @@ describe('Router', function () {
function (cb) {
request(server)
.post('/foo')
.expect(404, cb)
},
function (cb) {
request(server)
.post('/foo/')
.expect(200, 'saw POST /', cb)
},
function (cb) {
Expand Down Expand Up @@ -1248,6 +1253,202 @@ describe('Router', function () {
})
})

describe('with trailing slash mount and strict sub-router (issue expressjs/express#2281)', function () {
it('should not match request without trailing slash when mounted with trailing slash', function (done) {
const router = new Router()
const subRouter = new Router({ strict: true })
const server = createServer(router)

subRouter.get('/', saw)

router.use('/strict/', subRouter)

series([
function (cb) {
request(server)
.get('/strict/')
.expect(200, 'saw GET /', cb)
},
function (cb) {
request(server)
.get('/strict')
.expect(404, cb)
}
], done)
})

it('should match sub-paths when mounted with trailing slash', function (done) {
const router = new Router()
const subRouter = new Router({ strict: true })
const server = createServer(router)

subRouter.get('/sub', saw)

router.use('/api/', subRouter)

request(server)
.get('/api/sub')
.expect(200, 'saw GET /sub', done)
})

it('should still match request without trailing slash when mounted without trailing slash', function (done) {
const router = new Router()
const subRouter = new Router({ strict: true })
const server = createServer(router)

subRouter.get('/', saw)

router.use('/foo', subRouter)

request(server)
.get('/foo')
.expect(200, 'saw GET /', done)
})

it('should handle query strings correctly with trailing slash mount', function (done) {
const router = new Router()
const subRouter = new Router({ strict: true })
const server = createServer(router)

subRouter.get('/', saw)

router.use('/api/', subRouter)

series([
function (cb) {
request(server)
.get('/api/?q=1')
.expect(200, cb)
},
function (cb) {
request(server)
.get('/api?q=1')
.expect(404, cb)
}
], done)
})

it('should handle nested mounts with trailing slash', function (done) {
const router = new Router()
const outer = new Router()
const inner = new Router({ strict: true })
const server = createServer(router)

inner.get('/', saw)

outer.use('/inner/', inner)
router.use('/outer/', outer)

series([
function (cb) {
request(server)
.get('/outer/inner/')
.expect(200, 'saw GET /', cb)
},
function (cb) {
request(server)
.get('/outer/inner')
.expect(404, cb)
}
], done)
})

it('should enforce trailing slash for non-strict sub-routers mounted with trailing slash', function (done) {
const router = new Router()
const subRouter = new Router({ strict: false })
const server = createServer(router)

subRouter.get('/', saw)

router.use('/loose/', subRouter)

series([
function (cb) {
request(server)
.get('/loose/')
.expect(200, cb)
},
function (cb) {
request(server)
.get('/loose')
.expect(404, cb)
}
], done)
})

it('should enforce trailing slash for direct handlers mounted with trailing slash', function (done) {
const router = new Router()
const server = createServer(router)

router.use('/handler/', saw)

series([
function (cb) {
request(server)
.get('/handler/')
.expect(200, cb)
},
function (cb) {
request(server)
.get('/handler')
.expect(404, cb)
},
function (cb) {
request(server)
.get('/handler/sub')
.expect(200, cb)
}
], done)
})

it('should set correct baseUrl when trailing slash matches', function (done) {
const router = new Router()
const subRouter = new Router({ strict: true })
const server = createServer(router)

subRouter.get('/', sawBase)

router.use('/api/', subRouter)

request(server)
.get('/api/')
.expect(200, 'saw /api', done)
})

it('should work with all HTTP methods', function (done) {
const router = new Router()
const subRouter = new Router({ strict: true })
const server = createServer(router)

subRouter.all('/', saw)

router.use('/api/', subRouter)

series([
function (cb) {
request(server)
.get('/api/')
.expect(200, cb)
},
function (cb) {
request(server)
.post('/api/')
.expect(200, cb)
},
function (cb) {
request(server)
.get('/api')
.expect(404, cb)
},
function (cb) {
request(server)
.post('/api')
.expect(404, cb)
}
], done)
})
})

describe('next("route")', function () {
it('should invoke next handler', function (done) {
const router = new Router()
Expand Down