Skip to content
Merged
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
7 changes: 5 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@ const http = require('http')
const httpServer = http.Server
const httpsServer = require('https').Server

module.exports = (config = { prioRequestsProcessing: true }) => {
module.exports = (config = {}) => {
const router = config.router || require('./lib/router/sequential')(config)
const server = config.server || http.createServer()

// Default to true even when a partial config is provided. Keeping the default
// in the parameter alone would silently flip it off for any `zero({ ... })` call.
const prioRequestsProcessing = config.prioRequestsProcessing ?? true
server.prioRequestsProcessing =
config.prioRequestsProcessing && (server instanceof httpServer || server instanceof httpsServer)
prioRequestsProcessing && (server instanceof httpServer || server instanceof httpsServer)

if (server.prioRequestsProcessing) {
server.on('request', (req, res) => {
Expand Down
6 changes: 4 additions & 2 deletions lib/router/sequential.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,10 @@ module.exports = (config = {}) => {

// Optimized parameter assignment with minimal overhead
if (!req.params) {
// Use pre-created empty object or provided params directly
req.params = params || Object.create(null)
// Shallow-copy: the match (and its params) may be served from the LRU
// cache and shared across all requests to the same method+path.
// Assigning by reference would let a middleware mutation leak between requests.
req.params = params ? { ...params } : Object.create(null)
} else if (params) {
// Manual property copying - optimized for small objects
// Pre-compute keys and length to avoid repeated calls
Expand Down
74 changes: 74 additions & 0 deletions tests/regression-findings.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/* global describe, it */
const expect = require('chai').expect
const request = require('supertest')

/**
* Regression tests for two findings:
*
* #1 prioRequestsProcessing default flips OFF whenever ANY config object is
* passed without the flag, contradicting the documented `default: true`.
*
* #2 The route-match cache stores trouter's `match` object (including its
* `params`). On a cache hit `req.params` is assigned that object BY
* REFERENCE, so all requests to the same method+path share one mutable
* object — a non-idempotent middleware mutation bleeds into later requests.
*/
describe('0http - Regression: findings validation', () => {
describe('#1 prioRequestsProcessing default', () => {
it('should default prioRequestsProcessing to true when omitted but other config is provided', (done) => {
// A real-world call: user customizes the router but does NOT mention the flag.
const { server } = require('../index')({
router: require('../lib/router/sequential')()
})

// Documented default is `true`; passing unrelated config must not flip it off.
expect(server.prioRequestsProcessing).equals(true)
done()
})

it('should still honor an explicit false', (done) => {
const { server } = require('../index')({
prioRequestsProcessing: false,
router: require('../lib/router/sequential')()
})

expect(server.prioRequestsProcessing).equals(false)
done()
})
})

describe('#2 cached params are not shared across requests', () => {
const baseUrl = `http://localhost:${~~process.env.PORT + 1}`
const { router, server } = require('../index')({
router: require('../lib/router/sequential')()
})

it('should start the service', (done) => {
// Non-idempotent mutation: append a marker to req.params.id on every request.
router.get('/item/:id', (req, res) => {
req.params.id = req.params.id + '!'
res.end(req.params.id)
})

server.listen(~~process.env.PORT + 1, () => done())
})

it('should not leak a mutated param into the next request to the same path', async () => {
// First hit: 'x' -> 'x!'
const first = await request(baseUrl).get('/item/x').expect(200)
expect(first.text).to.equal('x!')

// Second hit to the SAME path: must also be 'x!', NOT 'x!!'.
const second = await request(baseUrl).get('/item/x').expect(200)
expect(second.text).to.equal('x!')

// Third hit, to be sure it does not keep growing.
const third = await request(baseUrl).get('/item/x').expect(200)
expect(third.text).to.equal('x!')
})

it('should stop the service', async () => {
server.close()
})
})
})
Loading