diff --git a/.eslintrc.yml b/.eslintrc.yml index 1eece14..6d49a7d 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -1,2 +1,19 @@ root: true -extends: standard +extends: + - standard + - plugin:markdown/recommended +plugins: + - markdown +overrides: + - files: '**/*.md' + processor: 'markdown/markdown' +rules: + eol-last: error + indent: + - error + - 2 + - SwitchCase: 1 + no-trailing-spaces: error + semi: + - error + - never diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..e96cc35 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,173 @@ +name: ci + +on: +- pull_request +- push + +jobs: + test: + runs-on: ubuntu-latest + strategy: + matrix: + name: + - Node.js 0.10 + - Node.js 0.12 + - io.js 1.x + - io.js 2.x + - io.js 3.x + - Node.js 4.x + - Node.js 5.x + - Node.js 6.x + - Node.js 7.x + - Node.js 8.x + - Node.js 9.x + - Node.js 10.x + - Node.js 11.x + - Node.js 12.x + - Node.js 13.x + - Node.js 14.x + - Node.js 15.x + - Node.js 16.x + - Node.js 17.x + + include: + - name: Node.js 0.10 + node-version: "0.10" + npm-i: mocha@3.5.3 nyc@10.3.2 supertest@2.0.0 + + - name: Node.js 0.12 + node-version: "0.12" + npm-i: mocha@3.5.3 nyc@10.3.2 supertest@2.0.0 + + - name: io.js 1.x + node-version: "1.8" + npm-i: mocha@3.5.3 nyc@10.3.2 supertest@2.0.0 + + - name: io.js 2.x + node-version: "2.5" + npm-i: mocha@3.5.3 nyc@10.3.2 supertest@2.0.0 + + - name: io.js 3.x + node-version: "3.3" + npm-i: mocha@3.5.3 nyc@10.3.2 supertest@2.0.0 + + - name: Node.js 4.x + node-version: "4.9" + npm-i: mocha@5.2.0 nyc@11.9.0 supertest@3.4.2 + + - name: Node.js 5.x + node-version: "5.12" + npm-i: mocha@5.2.0 nyc@11.9.0 supertest@3.4.2 + + - name: Node.js 6.x + node-version: "6.17" + npm-i: mocha@6.2.2 nyc@14.1.1 + + - name: Node.js 7.x + node-version: "7.10" + npm-i: mocha@6.2.2 nyc@14.1.1 + + - name: Node.js 8.x + node-version: "8.17" + npm-i: mocha@7.2.0 + + - name: Node.js 9.x + node-version: "9.11" + npm-i: mocha@7.2.0 + + - name: Node.js 10.x + node-version: "10.24" + npm-i: mocha@8.4.0 + + - name: Node.js 11.x + node-version: "11.15" + npm-i: mocha@8.4.0 + + - name: Node.js 12.x + node-version: "12.22" + + - name: Node.js 13.x + node-version: "13.14" + + - name: Node.js 14.x + node-version: "14.18" + + - name: Node.js 15.x + node-version: "15.14" + + - name: Node.js 16.x + node-version: "16.13" + + - name: Node.js 17.x + node-version: "17.1" + + steps: + - uses: actions/checkout@v2 + + - name: Install Node.js ${{ matrix.node-version }} + shell: bash -eo pipefail -l {0} + run: | + nvm install --default ${{ matrix.node-version }} + dirname "$(nvm which ${{ matrix.node-version }})" >> "$GITHUB_PATH" + + - name: Configure npm + run: npm config set shrinkwrap false + + - name: Install npm module(s) ${{ matrix.npm-i }} + run: npm install --save-dev ${{ matrix.npm-i }} + if: matrix.npm-i != '' + + - name: Setup Node.js version-specific dependencies + shell: bash + run: | + # eslint for linting + # - remove on Node.js < 10 + if [[ "$(cut -d. -f1 <<< "${{ matrix.node-version }}")" -lt 10 ]]; then + node -pe 'Object.keys(require("./package").devDependencies).join("\n")' | \ + grep -E '^eslint(-|$)' | \ + sort -r | \ + xargs -n1 npm rm --silent --save-dev + fi + + - name: Install Node.js dependencies + run: npm install + + - name: List environment + id: list_env + shell: bash + run: | + echo "node@$(node -v)" + echo "npm@$(npm -v)" + npm -s ls ||: + (npm -s ls --depth=0 ||:) | awk -F'[ @]' 'NR>1 && $2 { print "::set-output name=" $2 "::" $3 }' + + - name: Run tests + shell: bash + run: | + if npm -ps ls nyc | grep -q nyc; then + npm run test-ci + else + npm test + fi + + - name: Lint code + if: steps.list_env.outputs.eslint != '' + run: npm run lint + + - name: Collect code coverage + uses: coverallsapp/github-action@master + if: steps.list_env.outputs.nyc != '' + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + flag-name: run-${{ matrix.test_number }} + parallel: true + + coverage: + needs: test + runs-on: ubuntu-latest + steps: + - name: Upload code coverage + uses: coverallsapp/github-action@master + with: + github-token: ${{ secrets.github_token }} + parallel-finished: true diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 357893a..0000000 --- a/.travis.yml +++ /dev/null @@ -1,103 +0,0 @@ -language: node_js -node_js: - - "0.10" - - "0.12" - - "1.8" - - "2.5" - - "3.3" - - "4.9" - - "5.12" - - "6.17" - - "7.10" - - "8.17" - - "9.11" - - "10.19" - - "11.15" - - "12.16" - - "13.11" -cache: - directories: - - node_modules -before_install: - - | - # Setup utility functions - function node_version_lt () { - [[ "$(v "$TRAVIS_NODE_VERSION")" -lt "$(v "${1}")" ]] - } - function npm_module_installed () { - npm -lsp ls | grep -Fq "$(pwd)/node_modules/${1}:${1}@" - } - function npm_remove_module_re () { - node -e ' - fs = require("fs"); - p = JSON.parse(fs.readFileSync("package.json", "utf8")); - r = RegExp(process.argv[1]); - for (k in p.devDependencies) { - if (r.test(k)) delete p.devDependencies[k]; - } - fs.writeFileSync("package.json", JSON.stringify(p, null, 2) + "\n"); - ' "$@" - } - function npm_use_module () { - node -e ' - fs = require("fs"); - p = JSON.parse(fs.readFileSync("package.json", "utf8")); - p.devDependencies[process.argv[1]] = process.argv[2]; - fs.writeFileSync("package.json", JSON.stringify(p, null, 2) + "\n"); - ' "$@" - } - function v () { - tr '.' '\n' <<< "${1}" \ - | awk '{ printf "%03d", $0 }' \ - | sed 's/^0*//' - } - # Configure npm - - | - # Skip updating shrinkwrap / lock - npm config set shrinkwrap false - # Setup Node.js version-specific dependencies - - | - # Configure eslint for linting - if node_version_lt '8.0'; then npm_remove_module_re '^eslint(-|$)' - fi - - | - # Configure mocha for testing - if node_version_lt '4.0'; then npm_use_module 'mocha' '3.5.3' - elif node_version_lt '6.0'; then npm_use_module 'mocha' '5.2.0' - elif node_version_lt '8.0'; then npm_use_module 'mocha' '6.2.2' - fi - - | - # Configure nyc for testing - if node_version_lt '4.0'; then npm_use_module 'nyc' '10.3.2' - elif node_version_lt '6.0'; then npm_use_module 'nyc' '11.9.0' - elif node_version_lt '8.0'; then npm_use_module 'nyc' '14.1.1' - fi - - | - # Configure supertest for http calls - if node_version_lt '4.0'; then npm_use_module 'supertest' '2.0.0' - elif node_version_lt '6.0'; then npm_use_module 'supertest' '3.4.2' - fi - # Update Node.js modules - - | - # Prune and rebuild node_modules - if [[ -d node_modules ]]; then - npm prune - npm rebuild - fi -script: - # Run test script, depending on nyc install - - | - if npm_module_installed 'nyc'; then npm run-script test-travis - else npm test - fi - # Run linting, depending on eslint install - - | - if npm_module_installed 'eslint'; then npm run-script lint - fi -after_script: - # Upload coverage to coveralls if exists - - | - if [[ -d .nyc_output ]]; then - npm install --save-dev coveralls@2 - nyc report --reporter=text-lcov | coveralls - fi diff --git a/HISTORY.md b/HISTORY.md index 55b18bd..438dfaa 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,3 +1,8 @@ +2.x +=== + +This incorporates all changes after 1.3.5 up to 1.3.6. + 2.0.0-beta.1 / 2020-03-29 ========================= @@ -33,6 +38,12 @@ This incorporates all changes after 1.3.3 up to 1.3.5. - Remove `DEBUG_FD` environment variable support - Support 256 namespace colors +1.3.6 / 2021-11-15 +================== + + * Fix handling very large stacks of sync middleware + * deps: safe-buffer@5.2.1 + 1.3.5 / 2020-03-24 ================== diff --git a/README.md b/README.md index a498379..b105931 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ [![NPM Version][npm-image]][npm-url] [![NPM Downloads][downloads-image]][downloads-url] [![Node.js Version][node-version-image]][node-version-url] -[![Build Status][travis-image]][travis-url] +[![Build Status][ci-image]][ci-url] [![Test Coverage][coveralls-image]][coveralls-url] Simple middleware-style router @@ -399,12 +399,12 @@ server.listen(8080) [MIT](LICENSE) +[ci-image]: https://badgen.net/github/checks/pillarjs/router/master?label=ci +[ci-url]: https://github.com/pillarjs/router/actions?query=workflow%3Aci [npm-image]: https://img.shields.io/npm/v/router.svg [npm-url]: https://npmjs.org/package/router [node-version-image]: https://img.shields.io/node/v/router.svg [node-version-url]: http://nodejs.org/download/ -[travis-image]: https://img.shields.io/travis/pillarjs/router/master.svg -[travis-url]: https://travis-ci.org/pillarjs/router [coveralls-image]: https://img.shields.io/coveralls/pillarjs/router/master.svg [coveralls-url]: https://coveralls.io/r/pillarjs/router?branch=master [downloads-image]: https://img.shields.io/npm/dm/router.svg diff --git a/index.js b/index.js index 45495b3..6dbb540 100644 --- a/index.js +++ b/index.js @@ -155,6 +155,7 @@ Router.prototype.handle = function handle (req, res, callback) { var removed = '' var self = this var slashAdded = false + var sync = 0 var paramcalled = {} // middleware and routes @@ -210,6 +211,11 @@ Router.prototype.handle = function handle (req, res, callback) { return } + // max sync stack + if (++sync > 100) { + return setImmediate(next, err) + } + // get pathname of request var path = getPathname(req) @@ -329,6 +335,8 @@ Router.prototype.handle = function handle (req, res, callback) { } else { layer.handleRequest(req, res, next) } + + sync = 0 } } diff --git a/lib/route.js b/lib/route.js index 7656403..b89b8e3 100644 --- a/lib/route.js +++ b/lib/route.js @@ -23,6 +23,11 @@ var methods = require('methods') var slice = Array.prototype.slice +/* istanbul ignore next */ +var defer = typeof setImmediate === 'function' + ? setImmediate + : function (fn) { process.nextTick(fn.bind.apply(fn, arguments)) } + /** * Expose `Route`. */ @@ -93,6 +98,8 @@ Route.prototype._methods = function _methods () { Route.prototype.dispatch = function dispatch (req, res, done) { var idx = 0 var stack = this.stack + var sync = 0 + if (stack.length === 0) { return done() } @@ -122,6 +129,11 @@ Route.prototype.dispatch = function dispatch (req, res, done) { return done(err) } + // max sync stack + if (++sync > 100) { + return defer(next, err) + } + var layer var match @@ -141,6 +153,8 @@ Route.prototype.dispatch = function dispatch (req, res, done) { } else { layer.handleRequest(req, res, next) } + + sync = 0 } } diff --git a/package.json b/package.json index 9b6306c..fa25e86 100644 --- a/package.json +++ b/package.json @@ -18,18 +18,18 @@ }, "devDependencies": { "after": "0.8.2", - "eslint": "6.8.0", + "eslint": "7.32.0", "eslint-config-standard": "14.1.1", - "eslint-plugin-import": "2.20.1", - "eslint-plugin-markdown": "1.0.2", - "eslint-plugin-node": "11.0.0", - "eslint-plugin-promise": "4.2.1", - "eslint-plugin-standard": "4.0.1", + "eslint-plugin-import": "2.25.3", + "eslint-plugin-markdown": "2.2.1", + "eslint-plugin-node": "11.1.0", + "eslint-plugin-promise": "4.3.1", + "eslint-plugin-standard": "4.1.0", "finalhandler": "1.1.2", - "mocha": "7.1.1", - "nyc": "15.0.0", - "safe-buffer": "5.2.0", - "supertest": "4.0.2" + "mocha": "9.1.3", + "nyc": "15.1.0", + "safe-buffer": "5.2.1", + "supertest": "6.1.6" }, "files": [ "lib/", @@ -42,10 +42,10 @@ "node": ">= 0.10" }, "scripts": { - "lint": "eslint --plugin markdown --ext js,md .", + "lint": "eslint .", "test": "mocha --reporter spec --bail --check-leaks test/", + "test-ci": "nyc --reporter=lcov --reporter=text npm test", "test-cov": "nyc --reporter=text npm test", - "test-travis": "nyc --reporter=html --reporter=text npm test", "version": "node scripts/version-history.js && git add HISTORY.md" } } diff --git a/test/req.params.js b/test/req.params.js index 334b805..4a0e01b 100644 --- a/test/req.params.js +++ b/test/req.params.js @@ -68,7 +68,7 @@ describe('req.params', function () { }) describe('when "mergeParams: true"', function () { - it('should merge outsite object with params', function (done) { + it('should merge outside object with params', function (done) { var router = Router({ mergeParams: true }) var server = createServer(function (req, res, next) { req.params = { foo: 'bar' } @@ -87,7 +87,7 @@ describe('req.params', function () { .expect(200, '{"foo":"bar"}', done) }) - it('should ignore non-object outsite object', function (done) { + it('should ignore non-object outside object', function (done) { var router = Router({ mergeParams: true }) var server = createServer(function (req, res, next) { req.params = 42 @@ -126,7 +126,7 @@ describe('req.params', function () { }) describe('with numeric properties in req.params', function () { - it('should merge numeric properies by offsetting', function (done) { + it('should merge numeric properties by offsetting', function (done) { var router = Router({ mergeParams: true }) var server = createServer(function (req, res, next) { req.params = { 0: 'foo', 1: 'bar' } diff --git a/test/route.js b/test/route.js index 69dd5ac..ee3d6d7 100644 --- a/test/route.js +++ b/test/route.js @@ -107,6 +107,24 @@ describe('Router', function () { .expect(404, done) }) + it('should not stack overflow with a large sync stack', function (done) { + this.timeout(5000) // long-running test + + var router = new Router() + var route = router.route('/foo') + var server = createServer(router) + + for (var i = 0; i < 6000; i++) { + route.all(function (req, res, next) { next() }) + } + + route.get(helloWorld) + + request(server) + .get('/foo') + .expect(200, 'hello, world', done) + }) + describe('.all(...fn)', function () { it('should reject no arguments', function () { var router = new Router() diff --git a/test/router.js b/test/router.js index 74308f7..3b2a268 100644 --- a/test/router.js +++ b/test/router.js @@ -563,6 +563,23 @@ describe('Router', function () { .expect(404, done) }) + it('should not stack overflow with a large sync stack', function (done) { + this.timeout(5000) // long-running test + + var router = new Router() + var server = createServer(router) + + for (var i = 0; i < 6000; i++) { + router.use(function (req, res, next) { next() }) + } + + router.use(helloWorld) + + request(server) + .get('/') + .expect(200, 'hello, world', done) + }) + describe('error handling', function () { it('should invoke error function after next(err)', function (done) { var router = new Router()