diff --git a/index.js b/index.js index 4358aeb..25831f5 100644 --- a/index.js +++ b/index.js @@ -392,6 +392,17 @@ Router.prototype.use = function use (handler) { throw new TypeError('argument handler must be a function') } + // Detect router cycles + if (fn && typeof fn.handle === 'function' && fn.stack) { + if (detectRouterCycle(this, fn)) { + process.emitWarning( + 'Detected router mounted on itself or in a cycle. ' + + 'This will cause requests to hang indefinitely.', + 'RouterCycleWarning' + ) + } + } + // add the middleware debug('use %o %s', path, fn.name || '') @@ -409,6 +420,51 @@ Router.prototype.use = function use (handler) { return this } +/** + * Detect if mounting a router would create a cycle. + * + * @param {Router} parentRouter - The router being mounted onto + * @param {Router} childRouter - The router being mounted + * @param {Set} visited - Set of visited routers to avoid infinite recursion + * @return {boolean} + * @private + */ + +function detectRouterCycle (parentRouter, childRouter, visited) { + // Direct self-mount + if (parentRouter === childRouter) { + return true + } + + // Initialize visited set on first call + if (!visited) { + visited = new Set() + } + + // Avoid infinite recursion - if we've already checked this router, skip it + if (visited.has(childRouter)) { + return false + } + + visited.add(childRouter) + + // Check if childRouter already contains parentRouter in its stack + if (childRouter.stack) { + for (let i = 0; i < childRouter.stack.length; i++) { + const layer = childRouter.stack[i] + if (layer.handle === parentRouter) { + return true + } + // Check nested routers recursively + if (layer.handle && layer.handle.stack && detectRouterCycle(parentRouter, layer.handle, visited)) { + return true + } + } + } + + return false +} + /** * Create a new Route for the given path. * diff --git a/test/router.js b/test/router.js index b440e40..ce73a6a 100644 --- a/test/router.js +++ b/test/router.js @@ -1487,3 +1487,106 @@ function sawBase (req, res) { res.setHeader('Content-Type', 'text/plain') res.end(msg) } + +describe('Router cycle detection', function () { + it('should emit warning when router is mounted on itself', function (done) { + const router = new Router() + const warnings = [] + const originalEmit = process.emitWarning + + process.emitWarning = function (message, type) { + warnings.push({ message, type }) + } + + router.use('/', router) + + process.emitWarning = originalEmit + + assert.strictEqual(warnings.length, 1) + assert.strictEqual(warnings[0].type, 'RouterCycleWarning') + assert.ok(warnings[0].message.includes('cycle')) + done() + }) + + it('should emit warning for indirect router cycle', function (done) { + const routerA = new Router() + const routerB = new Router() + const warnings = [] + const originalEmit = process.emitWarning + + process.emitWarning = function (message, type) { + warnings.push({ message, type }) + } + + routerA.use('/b', routerB) + routerB.use('/a', routerA) + + process.emitWarning = originalEmit + + assert.strictEqual(warnings.length, 1) + assert.strictEqual(warnings[0].type, 'RouterCycleWarning') + assert.ok(warnings[0].message.includes('cycle')) + done() + }) + + it('should emit warning for nested indirect cycles', function (done) { + const routerA = new Router() + const routerB = new Router() + const routerC = new Router() + const warnings = [] + const originalEmit = process.emitWarning + + process.emitWarning = function (message, type) { + warnings.push({ message, type }) + } + + routerA.use('/b', routerB) + routerB.use('/c', routerC) + routerC.use('/a', routerA) + + process.emitWarning = originalEmit + + assert.strictEqual(warnings.length, 1) + assert.strictEqual(warnings[0].type, 'RouterCycleWarning') + done() + }) + + it('should not emit warning for non-cyclic router mounts', function (done) { + const routerA = new Router() + const routerB = new Router() + const routerC = new Router() + const warnings = [] + const originalEmit = process.emitWarning + + process.emitWarning = function (message, type) { + warnings.push({ message, type }) + } + + routerA.use('/b', routerB) + routerA.use('/c', routerC) + + process.emitWarning = originalEmit + + assert.strictEqual(warnings.length, 0) + done() + }) + + it('should allow cyclic mount but warn (behavior unchanged)', function (done) { + const router = new Router() + const warnings = [] + const originalEmit = process.emitWarning + + process.emitWarning = function (message, type) { + warnings.push({ message, type }) + } + + router.get('/test', helloWorld) + router.use('/', router) + + process.emitWarning = originalEmit + + assert.strictEqual(warnings.length, 1) + assert.strictEqual(router.stack.length, 2) // route + cyclic mount + done() + }) +})