Skip to content

Commit

Permalink
Don't store request specific data into the Layer object, should be im…
Browse files Browse the repository at this point in the history
…mutable
  • Loading branch information
Colin GILLE committed Feb 2, 2024
1 parent 86a5f76 commit ff0facf
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 45 deletions.
41 changes: 12 additions & 29 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,20 +233,19 @@ Router.prototype.handle = function handle(req, res, callback) {

// find next matching layer
var layer
var match
var match = false
var route

while (match !== true && idx < stack.length) {
while (match === false && idx < stack.length) {
layer = stack[idx++]
match = matchLayer(layer, path)
route = layer.route

if (typeof match !== 'boolean') {
// hold on to layerError
layerError = layerError || match
try {
match = layer.match(path)
} catch (err) {
layerError = layerError || err
}
route = layer.route

if (match !== true) {
if (match === false) {
continue
}

Expand Down Expand Up @@ -277,7 +276,7 @@ Router.prototype.handle = function handle(req, res, callback) {
}

// no match
if (match !== true) {
if (match === false) {
return done(layerError)
}

Expand All @@ -288,9 +287,9 @@ Router.prototype.handle = function handle(req, res, callback) {

// Capture one-time layer values
req.params = self.mergeParams
? mergeParams(layer.params, parentParams)
: layer.params
var layerPath = layer.path
? mergeParams(match.params, parentParams)
: match.params
var layerPath = match.path

// this should be done for the layer
self.process_params(layer, paramcalled, req, res, function (err) {
Expand Down Expand Up @@ -603,22 +602,6 @@ function getProtohost(url) {
: undefined
}

/**
* Match path to a layer.
*
* @param {Layer} layer
* @param {string} path
* @private
*/

function matchLayer(layer, path) {
try {
return layer.match(path)
} catch (err) {
return err
}
}

/**
* Merge params with parent params
*
Expand Down
29 changes: 13 additions & 16 deletions lib/layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ function Layer(path, options, fn) {

this.handle = fn
this.name = fn.name || '<anonymous>'
this.params = undefined
this.path = undefined
this.regexp = pathRegexp(path, this.keys = [], opts)

// set fast path flags
Expand Down Expand Up @@ -111,35 +109,31 @@ Layer.prototype.match = function match(path) {
if (path != null) {
// fast path non-ending match for / (any path matches)
if (this.regexp.fast_slash) {
this.params = {}
this.path = ''
return true
return {
path: '',
params: {}
}
}

// fast path for * (everything matched in a param)
if (this.regexp.fast_star) {
this.params = {'0': decode_param(path)}
this.path = path
return true
return {
path: path,
params: {'0': decode_param(path)}
}
}

// match the path
match = this.regexp.exec(path)
}

if (!match) {
this.params = undefined
this.path = undefined
return false
}

// store values
this.params = {}
this.path = match[0]

// iterate matches
var keys = this.keys
var params = this.params
var params = {}

for (var i = 1; i < match.length; i++) {
var key = keys[i - 1]
Expand All @@ -151,7 +145,10 @@ Layer.prototype.match = function match(path) {
}
}

return true
return {
path: match[0],
params: params
}
}

/**
Expand Down
22 changes: 22 additions & 0 deletions test/req.params.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ var Router = require('..')
var utils = require('./support/utils')

var createServer = utils.createServer
var assert = utils.assert
var request = utils.request

describe('req.params', function () {
Expand Down Expand Up @@ -67,6 +68,27 @@ describe('req.params', function () {
.expect(200, '{"foo":"bar"}', done)
})

it('should not keep parameters in memory', function (done) {
var router = Router()
var server = createServer(function (req, res, next) {
router(req, res, function (err) {
if (err) return next(err)
sawParams(req, res)
})
})

router.get('/:fizz', hitParams(1))

request(server)
.get('/buzz')
.end(function(err) {
if (err) return done(err)

assert.strictEqual(router.stack[0].params, undefined)
done()
})
})

describe('when "mergeParams: true"', function () {
it('should merge outside object with params', function (done) {
var router = Router({ mergeParams: true })
Expand Down

0 comments on commit ff0facf

Please sign in to comment.