diff --git a/index.js b/index.js index 747ff33..0dbe740 100644 --- a/index.js +++ b/index.js @@ -576,7 +576,6 @@ function processParams (params, layer, called, req, res, done) { } let i = 0 - let name let paramIndex = 0 let key let paramVal @@ -596,10 +595,9 @@ function processParams (params, layer, called, req, res, done) { paramIndex = 0 key = keys[i++] - name = key.name - paramVal = req.params[name] - paramCallbacks = params[name] - paramCalled = called[name] + paramVal = req.params[key] + paramCallbacks = params[key] + paramCalled = called[key] if (paramVal === undefined || !paramCallbacks) { return param() @@ -609,13 +607,13 @@ function processParams (params, layer, called, req, res, done) { if (paramCalled && (paramCalled.match === paramVal || (paramCalled.error && paramCalled.error !== 'route'))) { // restore value - req.params[name] = paramCalled.value + req.params[key] = paramCalled.value // next param return param(paramCalled.error) } - called[name] = paramCalled = { + called[key] = paramCalled = { error: null, match: paramVal, value: paramVal @@ -629,7 +627,7 @@ function processParams (params, layer, called, req, res, done) { const fn = paramCallbacks[paramIndex++] // store updated value - paramCalled.value = req.params[key.name] + paramCalled.value = req.params[key] if (err) { // store error @@ -641,7 +639,7 @@ function processParams (params, layer, called, req, res, done) { if (!fn) return param() try { - const ret = fn(req, res, paramCallback, paramVal, key.name) + const ret = fn(req, res, paramCallback, paramVal, key) if (isPromise(ret)) { ret.then(null, function (error) { paramCallback(error || new Error('Rejected promise')) diff --git a/lib/layer.js b/lib/layer.js index a96eb10..ec9ac7a 100644 --- a/lib/layer.js +++ b/lib/layer.js @@ -20,8 +20,8 @@ const pathRegexp = require('path-to-regexp') * @private */ -const hasOwnProperty = Object.prototype.hasOwnProperty const TRAILING_SLASH_REGEXP = /\/+$/ +const MATCHING_GROUP_REGEXP = /\((?:\?<(.*?)>)?(?!\?)/g /** * Expose `Layer`. @@ -41,10 +41,53 @@ function Layer (path, options, fn) { this.name = fn.name || '' this.params = undefined this.path = undefined - this.regexp = pathRegexp((opts.strict ? path : loosen(path)), this.keys, opts) + this.slash = path === '/' && opts.end === false + + function matcher (_path) { + if (_path instanceof RegExp) { + const keys = [] + let name = 0 + let m + // eslint-disable-next-line no-cond-assign + while (m = MATCHING_GROUP_REGEXP.exec(_path.source)) { + keys.push({ + name: m[1] || name++, + offset: m.index + }) + } + + return function regexpMatcher (p) { + const match = _path.exec(p) + if (!match) { + return false + } + + const params = {} + for (let i = 1; i < match.length; i++) { + const key = keys[i - 1] + const prop = key.name + const val = decodeParam(match[i]) + + if (val !== undefined) { + params[prop] = val + } + } + + return { + params, + path: p + } + } + } - // set fast path flags - this.regexp._slash = path === '/' && opts.end === false + return pathRegexp.match((opts.strict ? _path : loosen(_path)), { + sensitive: opts.sensitive, + end: opts.end, + trailing: !opts.strict, + decode: decodeParam + }) + } + this.matchers = Array.isArray(path) ? path.map(matcher) : [matcher(path)] } /** @@ -126,14 +169,18 @@ Layer.prototype.match = function match (path) { if (path != null) { // fast path non-ending match for / (any path matches) - if (this.regexp._slash) { + if (this.slash) { this.params = {} this.path = '' return true } - // match the path - match = this.regexp.exec(path) + let i = 0 + while (!match && i < this.matchers.length) { + // match the path + match = this.matchers[i](path) + i++ + } } if (!match) { @@ -143,22 +190,9 @@ Layer.prototype.match = function match (path) { } // store values - this.params = {} - this.path = match[0] - - // iterate matches - const keys = this.keys - const params = this.params - - for (let i = 1; i < match.length; i++) { - const key = keys[i - 1] - const prop = key.name - const val = decodeParam(match[i]) - - if (val !== undefined || !(hasOwnProperty.call(params, prop))) { - params[prop] = val - } - } + this.params = match.params + this.path = match.path + this.keys = Object.keys(match.params) return true } @@ -192,7 +226,7 @@ function decodeParam (val) { * Loosens the given path for path-to-regexp matching. */ function loosen (path) { - if (path instanceof RegExp) { + if (path instanceof RegExp || path === '/') { return path } diff --git a/package.json b/package.json index 900cc9e..35de1ac 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "is-promise": "4.0.0", "methods": "~1.1.2", "parseurl": "~1.3.3", - "path-to-regexp": "3.2.0", + "path-to-regexp": "^8.0.0", "setprototypeof": "1.2.0", "utils-merge": "1.0.1" }, @@ -41,6 +41,7 @@ "scripts": { "lint": "standard", "test": "mocha --reporter spec --bail --check-leaks test/", + "test:debug": "mocha --reporter spec --bail --check-leaks test/ --inspect --inspect-brk", "test-ci": "nyc --reporter=lcov --reporter=text npm test", "test-cov": "nyc --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 e5b1b6b..8f7200b 100644 --- a/test/req.params.js +++ b/test/req.params.js @@ -124,46 +124,6 @@ describe('req.params', function () { .expect('x-params-1', '{"foo":"buzz"}') .expect(200, '{"foo":"bar"}', done) }) - - describe('with numeric properties in req.params', function () { - it('should merge numeric properties by offsetting', function (done) { - const router = Router({ mergeParams: true }) - const server = createServer(function (req, res, next) { - req.params = { 0: 'foo', 1: 'bar' } - - router(req, res, function (err) { - if (err) return next(err) - sawParams(req, res) - }) - }) - - router.get('/(.*)', hitParams(1)) - - request(server) - .get('/buzz') - .expect('x-params-1', '{"0":"foo","1":"bar","2":"buzz"}') - .expect(200, '{"0":"foo","1":"bar"}', done) - }) - - it('should merge with same numeric properties', function (done) { - const router = Router({ mergeParams: true }) - const server = createServer(function (req, res, next) { - req.params = { 0: 'foo' } - - router(req, res, function (err) { - if (err) return next(err) - sawParams(req, res) - }) - }) - - router.get('/(.*)', hitParams(1)) - - request(server) - .get('/bar') - .expect('x-params-1', '{"0":"foo","1":"bar"}') - .expect(200, '{"0":"foo"}', done) - }) - }) }) }) diff --git a/test/route.js b/test/route.js index e32e1d4..b1c2ba4 100644 --- a/test/route.js +++ b/test/route.js @@ -716,27 +716,6 @@ describe('Router', function () { .expect(200, { foo: 'fizz', bar: 'buzz' }, done) }) - it('should work following a partial capture group', function (done) { - const router = new Router() - const route = router.route('/user(s?)/:user/:op') - const server = createServer(router) - - route.all(sendParams) - - series([ - function (cb) { - request(server) - .get('/user/tj/edit') - .expect(200, { 0: '', user: 'tj', op: 'edit' }, cb) - }, - function (cb) { - request(server) - .get('/users/tj/edit') - .expect(200, { 0: 's', user: 'tj', op: 'edit' }, cb) - } - ], done) - }) - it('should work inside literal paranthesis', function (done) { const router = new Router() const route = router.route('/:user\\(:op\\)') @@ -770,10 +749,10 @@ describe('Router', function () { }) }) - describe('using ":name?"', function () { + describe('using "{:name}"', function () { it('should name an optional parameter', function (done) { const router = new Router() - const route = router.route('/:foo?') + const route = router.route('{/:foo}') const server = createServer(router) route.all(sendParams) @@ -793,7 +772,7 @@ describe('Router', function () { it('should work in any segment', function (done) { const router = new Router() - const route = router.route('/user/:foo?/delete') + const route = router.route('/user{/:foo}/delete') const server = createServer(router) route.all(sendParams) @@ -812,10 +791,10 @@ describe('Router', function () { }) }) - describe('using ":name*"', function () { + describe('using "*name"', function () { it('should name a zero-or-more repeated parameter', function (done) { const router = new Router() - const route = router.route('/:foo*') + const route = router.route('{/*foo}') const server = createServer(router) route.all(sendParams) @@ -828,19 +807,19 @@ describe('Router', function () { function (cb) { request(server) .get('/bar') - .expect(200, { foo: 'bar' }, cb) + .expect(200, { foo: [ 'bar' ] }, cb) }, function (cb) { request(server) .get('/fizz/buzz') - .expect(200, { foo: 'fizz/buzz' }, cb) + .expect(200, { foo: [ 'fizz', 'buzz' ] }, cb) } ], done) }) it('should work in any segment', function (done) { const router = new Router() - const route = router.route('/user/:foo*/delete') + const route = router.route('/user{/*foo}/delete') const server = createServer(router) route.all(sendParams) @@ -853,135 +832,12 @@ describe('Router', function () { function (cb) { request(server) .get('/user/bar/delete') - .expect(200, { foo: 'bar' }, cb) - }, - function (cb) { - request(server) - .get('/user/fizz/buzz/delete') - .expect(200, { foo: 'fizz/buzz' }, cb) - } - ], done) - }) - }) - - describe('using ":name+"', function () { - it('should name a one-or-more repeated parameter', function (done) { - const router = new Router() - const route = router.route('/:foo+') - const server = createServer(router) - - route.all(sendParams) - - series([ - function (cb) { - request(server) - .get('/') - .expect(404, cb) - }, - function (cb) { - request(server) - .get('/bar') - .expect(200, { foo: 'bar' }, cb) - }, - function (cb) { - request(server) - .get('/fizz/buzz') - .expect(200, { foo: 'fizz/buzz' }, cb) - } - ], done) - }) - - it('should work in any segment', function (done) { - const router = new Router() - const route = router.route('/user/:foo+/delete') - const server = createServer(router) - - route.all(sendParams) - series([ - function (cb) { - request(server) - .get('/user/delete') - .expect(404, cb) - }, - function (cb) { - request(server) - .get('/user/bar/delete') - .expect(200, { foo: 'bar' }, cb) + .expect(200, { foo: [ 'bar' ] }, cb) }, function (cb) { request(server) .get('/user/fizz/buzz/delete') - .expect(200, { foo: 'fizz/buzz' }, cb) - } - ], done) - }) - }) - - describe('using ":name(regexp)"', function () { - it('should limit capture group to regexp match', function (done) { - const router = new Router() - const route = router.route('/:foo([0-9]+)') - const server = createServer(router) - - route.all(sendParams) - - series([ - function (cb) { - request(server) - .get('/foo') - .expect(404, cb) - }, - function (cb) { - request(server) - .get('/42') - .expect(200, { foo: '42' }, cb) - } - ], done) - }) - }) - - describe('using "(regexp)"', function () { - it('should add capture group using regexp', function (done) { - const router = new Router() - const route = router.route('/page_([0-9]+)') - const server = createServer(router) - - route.all(sendParams) - series([ - function (cb) { - request(server) - .get('/page_foo') - .expect(404, cb) - }, - function (cb) { - request(server) - .get('/page_42') - .expect(200, { 0: '42' }, cb) - } - ], done) - }) - - it('should treat regexp as literal regexp', function (done) { - const router = new Router() - const route = router.route('/([a-z]+:n[0-9]+)') - const server = createServer(router) - - route.all(sendParams) - series([ - function (cb) { - request(server) - .get('/foo:bar') - .expect(404, cb) - }, - function (cb) { - request(server) - .get('/foo:n') - .expect(404, cb) - }, - function (cb) { - request(server) - .get('/foo:n42') - .expect(200, { 0: 'foo:n42' }, cb) + .expect(200, { foo: [ 'fizz', 'buzz' ] }, cb) } ], done) }) diff --git a/test/router.js b/test/router.js index 01cc4ff..664ab5b 100644 --- a/test/router.js +++ b/test/router.js @@ -356,35 +356,7 @@ describe('Router', function () { const server = createServer(router) router[method](['/foo', '/bar'], createHitHandle(1), helloWorld) - series([ - function (cb) { - request(server)[method]('/') - .expect(404) - .expect(shouldNotHitHandle(1)) - .end(cb) - }, - function (cb) { - request(server)[method]('/foo') - .expect(200) - .expect(shouldHitHandle(1)) - .expect(body) - .end(cb) - }, - function (cb) { - request(server)[method]('/bar') - .expect(200) - .expect(shouldHitHandle(1)) - .expect(body) - .end(cb) - } - ], done) - }) - it('should support regexp path', function (done) { - const router = new Router() - const server = createServer(router) - - router[method](/^\/[a-z]oo$/, createHitHandle(1), helloWorld) series([ function (cb) { request(server)[method]('/') @@ -400,7 +372,7 @@ describe('Router', function () { .end(cb) }, function (cb) { - request(server)[method]('/zoo') + request(server)[method]('/bar') .expect(200) .expect(shouldHitHandle(1)) .expect(body) @@ -1008,7 +980,7 @@ describe('Router', function () { const router = new Router() const server = createServer(router) - router.use(/^\/[a-z]oo/, saw) + router.use(/^\/[a-z]oo$/, saw) series([ function (cb) { request(server) @@ -1028,7 +1000,56 @@ describe('Router', function () { function (cb) { request(server) .get('/zoo/bear') - .expect(200, 'saw GET /bear', cb) + .expect(404, cb) + }, + function (cb) { + request(server) + .get('/get/zoo') + .expect(404, cb) + } + ], done) + }) + + it('should support regexp path with params', function (done) { + const router = new Router() + const server = createServer(router) + + router.use(/^\/([a-z]oo)$/, function (req, res, next) { + createHitHandle(req.params[0])(req, res, next) + }, saw) + + router.use(/^\/([a-z]oo)\/(?bear)$/, function (req, res, next) { + createHitHandle(req.params[0] + req.params.animal)(req, res, next) + }, saw) + + series([ + function (cb) { + request(server) + .get('/') + .expect(404, cb) + }, + function (cb) { + request(server) + .get('/foo') + .expect(shouldHitHandle('foo')) + .expect(200, 'saw GET /', cb) + }, + function (cb) { + request(server) + .get('/zoo') + .expect(shouldHitHandle('zoo')) + .expect(200, 'saw GET /', cb) + }, + function (cb) { + request(server) + .get('/fooo') + .expect(404, cb) + }, + function (cb) { + request(server) + .get('/zoo/bear') + .expect(shouldHitHandle('zoobear')) + .expect(200, cb) }, function (cb) { request(server) @@ -1049,8 +1070,8 @@ describe('Router', function () { request(server) .get('/test/api/1234') - .expect(shouldNotHitHandle(1)) - .expect(shouldNotHitHandle(2)) + .expect(shouldHitHandle(1)) + .expect(shouldHitHandle(2)) .expect(shouldHitHandle(3)) .expect(200, done) })