Skip to content

Commit 4aa2e15

Browse files
committed
Enhance performance and security across router and query parameter handling
- Optimize middleware processing in the next function for improved performance. - Refactor cache initialization and management in the sequential router for better memory efficiency. - Introduce advanced prototype pollution tests to ensure security against dangerous properties. - Implement comprehensive performance tests for the router and query parameter parsing. - Update package dependencies for improved stability and performance.
1 parent ad5fd52 commit 4aa2e15

File tree

9 files changed

+584
-68
lines changed

9 files changed

+584
-68
lines changed

lib/next.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,14 @@ function next (middlewares, req, res, index, routers, defaultRoute, errorHandler
1717
return !res.finished && defaultRoute(req, res)
1818
}
1919

20-
// Get current middleware and increment index
21-
const middleware = middlewares[index++]
20+
// Get current middleware
21+
const middleware = middlewares[index]
2222

2323
// Create step function - this is called by middleware to continue the chain
2424
const step = function (err) {
2525
return err
2626
? errorHandler(err, req, res)
27-
: next(middlewares, req, res, index, routers, defaultRoute, errorHandler)
27+
: next(middlewares, req, res, index + 1, routers, defaultRoute, errorHandler)
2828
}
2929

3030
try {
@@ -41,7 +41,7 @@ function next (middlewares, req, res, index, routers, defaultRoute, errorHandler
4141
// Replace pattern in URL - this is a hot path, optimize it
4242
req.url = req.url.replace(pattern, '')
4343

44-
// Ensure URL starts with a slash
44+
// Ensure URL starts with a slash - use charCodeAt for performance
4545
if (req.url.length === 0 || req.url.charCodeAt(0) !== 47) { // 47 is '/'
4646
req.url = '/' + req.url
4747
}

lib/router/sequential.js

Lines changed: 103 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,32 @@ const { Trouter } = require('trouter')
22
const next = require('./../next')
33
const { parse } = require('regexparam')
44
const { LRUCache: Cache } = require('lru-cache')
5-
const queryparams = require('../utils/queryparams')
5+
const queryparams = require('./../utils/queryparams')
66

7-
// Default handlers as constants to avoid creating functions on each router instance
7+
/**
8+
* Default handlers as constants to avoid creating functions on each router instance.
9+
* This reduces memory allocation and improves performance when multiple routers are created.
10+
*/
811
const DEFAULT_ROUTE = (req, res) => {
912
res.statusCode = 404
1013
res.end()
1114
}
1215

1316
const DEFAULT_ERROR_HANDLER = (err, req, res) => {
1417
res.statusCode = 500
18+
// Note: err.message could expose sensitive information in production
1519
res.end(err.message)
1620
}
1721

18-
// Simple ID generator
19-
const generateId = () => Math.random().toString(36).substring(2, 10).toUpperCase()
22+
/**
23+
* Simple ID generator using Math.random for router identification.
24+
* Warning: Not cryptographically secure - suitable only for internal routing logic.
25+
* Optimized to minimize string operations.
26+
*/
27+
const generateId = () => {
28+
// Use a more efficient approach - avoid substring operations
29+
return Math.random().toString(36).slice(2, 10).toUpperCase()
30+
}
2031

2132
module.exports = (config = {}) => {
2233
// Use object destructuring with defaults for cleaner config initialization
@@ -29,88 +40,135 @@ module.exports = (config = {}) => {
2940

3041
const routers = {}
3142

32-
// Initialize cache only once
43+
/**
44+
* Pre-create frozen empty objects for better performance and safety.
45+
* Use Object.create(null) for prototype pollution protection.
46+
* Frozen objects prevent accidental mutation that could cause cross-request state pollution.
47+
* These are reused across requests when no query params or route params are present.
48+
*/
49+
const EMPTY_PARAMS = Object.freeze(Object.create(null))
50+
51+
/**
52+
* Initialize LRU cache for route matching results with optimized settings.
53+
* Cache keys are method+path combinations to speed up repeated lookups.
54+
* - cacheSize > 0: Limited LRU cache with specified max entries
55+
* - cacheSize = 0: No caching (disabled)
56+
* - cacheSize < 0: Large LRU cache (50k entries) for "unlimited" mode
57+
* Optimized cache size for better memory management and performance.
58+
*/
3359
let cache = null
3460
if (cacheSize > 0) {
35-
cache = new Cache({ max: cacheSize })
61+
cache = new Cache({
62+
max: cacheSize,
63+
updateAgeOnGet: false, // Disable age updates for better performance
64+
updateAgeOnHas: false
65+
})
3666
} else if (cacheSize < 0) {
37-
// For unlimited cache, still use LRUCache but with a very high max
38-
// This provides better memory management than an unbounded Map
39-
cache = new Cache({ max: 100000 })
67+
// Reduced from 100k to 50k for better memory efficiency while maintaining performance
68+
cache = new Cache({
69+
max: 50000,
70+
updateAgeOnGet: false,
71+
updateAgeOnHas: false
72+
})
4073
}
4174

4275
const router = new Trouter()
4376
router.id = id
4477

4578
const _use = router.use
4679

80+
/**
81+
* Enhanced router.use method with support for nested routers.
82+
* Handles both middleware functions and nested router instances.
83+
* Automatically handles prefix parsing when first argument is a function.
84+
* Optimized for minimal overhead in the common case.
85+
*/
4786
router.use = (prefix, ...middlewares) => {
4887
if (typeof prefix === 'function') {
4988
middlewares = [prefix, ...middlewares]
5089
prefix = '/'
5190
}
5291
_use.call(router, prefix, middlewares)
5392

54-
if (middlewares[0]?.id) {
55-
// caching router -> pattern relation for urls pattern replacement
93+
// Optimized nested router detection - check first middleware only
94+
const firstMiddleware = middlewares[0]
95+
if (firstMiddleware?.id) {
96+
// Cache router -> pattern relation for URL pattern replacement in nested routing
97+
// This enables efficient URL rewriting when entering nested router contexts
5698
const { pattern } = parse(prefix, true)
57-
routers[middlewares[0].id] = pattern
99+
routers[firstMiddleware.id] = pattern
58100
}
59101

60-
return router // Fix: return router instead of this
102+
return router // Ensure chainable API by returning router instance
61103
}
62104

63-
// Create the cleanup middleware once
64-
const createCleanupMiddleware = (step) => (req, res, next) => {
65-
req.url = req.preRouterUrl
66-
req.path = req.preRouterPath
67-
68-
req.preRouterUrl = undefined
69-
req.preRouterPath = undefined
70-
71-
return step()
105+
/**
106+
* Creates cleanup middleware for nested router restoration.
107+
* This middleware restores the original URL and path after nested router processing.
108+
* Uses property deletion instead of undefined assignment for better performance.
109+
* Optimized to minimize closure creation overhead.
110+
*/
111+
const createCleanupMiddleware = (step) => {
112+
// Pre-create the cleanup function to avoid repeated function creation
113+
return (req, res, next) => {
114+
req.url = req.preRouterUrl
115+
req.path = req.preRouterPath
116+
117+
// Use delete for better performance than setting undefined
118+
delete req.preRouterUrl
119+
delete req.preRouterPath
120+
121+
return step()
122+
}
72123
}
73124

74125
router.lookup = (req, res, step) => {
75-
// Initialize URL and originalUrl if needed
76-
req.url = req.url || '/'
77-
req.originalUrl = req.originalUrl || req.url
126+
// Initialize URL and originalUrl if needed - use nullish coalescing for better performance
127+
req.url ??= '/'
128+
req.originalUrl ??= req.url
78129

79-
// Parse query parameters
130+
// Parse query parameters using optimized utility
80131
queryparams(req, req.url)
81132

82-
// Fast path for cache lookup
83-
const reqCacheKey = cache && (req.method + req.path)
84-
let match = cache && cache.get(reqCacheKey)
133+
// Cache lookup optimization - minimize variable assignments
134+
let match
135+
if (cache) {
136+
// Pre-compute cache key with direct concatenation (fastest approach)
137+
const reqCacheKey = req.method + req.path
138+
match = cache.get(reqCacheKey)
85139

86-
if (!match) {
87-
match = router.find(req.method, req.path)
88-
if (cache && reqCacheKey) {
140+
if (!match) {
141+
match = router.find(req.method, req.path)
89142
cache.set(reqCacheKey, match)
90143
}
144+
} else {
145+
match = router.find(req.method, req.path)
91146
}
92147

93148
const { handlers, params } = match
94149

95-
if (handlers.length > 0) {
96-
// Avoid creating a new array with spread operator
97-
// Use the handlers array directly
150+
if (handlers.length) {
151+
// Optimized middleware array handling
98152
let middlewares
99-
100153
if (step !== undefined) {
101-
// Only create a new array if we need to add the cleanup middleware
154+
// Create new array only when step middleware is needed
102155
middlewares = handlers.slice()
103156
middlewares.push(createCleanupMiddleware(step))
104157
} else {
105158
middlewares = handlers
106159
}
107160

108-
// Initialize params object if needed
161+
// Optimized parameter assignment with minimal overhead
109162
if (!req.params) {
110-
req.params = params
163+
// Use pre-created empty object or provided params directly
164+
req.params = params || EMPTY_PARAMS
111165
} else if (params) {
112-
// Faster than Object.assign for small objects
113-
for (const key in params) {
166+
// Manual property copying - optimized for small objects
167+
// Pre-compute keys and length to avoid repeated calls
168+
const paramKeys = Object.keys(params)
169+
let i = paramKeys.length
170+
while (i--) {
171+
const key = paramKeys[i]
114172
req.params[key] = params[key]
115173
}
116174
}
@@ -121,6 +179,10 @@ module.exports = (config = {}) => {
121179
}
122180
}
123181

182+
/**
183+
* Shorthand method for registering routes with specific HTTP methods.
184+
* Delegates to router.add with the provided method, pattern, and handlers.
185+
*/
124186
router.on = (method, pattern, ...handlers) => router.add(method, pattern, handlers)
125187

126188
return router

lib/utils/object.js

Lines changed: 0 additions & 10 deletions
This file was deleted.

lib/utils/queryparams.js

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,54 @@
1+
// Pre-create Set for dangerous properties - faster O(1) lookup vs string comparisons
2+
const DANGEROUS_PROPERTIES = new Set(['__proto__', 'constructor', 'prototype'])
3+
4+
// Pre-created empty query object to avoid allocations
5+
const EMPTY_QUERY = Object.freeze(Object.create(null))
6+
17
module.exports = (req, url) => {
2-
const query = {}
3-
const indexOfQuestionMark = url.indexOf('?')
4-
const path = indexOfQuestionMark !== -1 ? url.slice(0, indexOfQuestionMark) : url
5-
const search = indexOfQuestionMark !== -1 ? url.slice(indexOfQuestionMark + 1) : ''
8+
// Single indexOf call - more efficient than multiple operations
9+
const questionMarkIndex = url.indexOf('?')
10+
11+
if (questionMarkIndex === -1) {
12+
// Fast path: no query string
13+
req.path = url
14+
req.query = EMPTY_QUERY
15+
return
16+
}
17+
18+
// Use Object.create(null) for prototype pollution protection
19+
const query = Object.create(null)
20+
21+
// Extract path and search in one operation each
22+
req.path = url.slice(0, questionMarkIndex)
23+
const search = url.slice(questionMarkIndex + 1)
624

7-
if (search.length > 0) {
8-
const searchParams = new URLSearchParams(search.replace(/\[\]=/g, '='))
9-
for (const [name, value] of searchParams.entries()) {
10-
if (query[name]) {
11-
Array.isArray(query[name]) ? query[name].push(value) : (query[name] = [query[name], value])
25+
if (search.length === 0) {
26+
// Fast path: empty query string
27+
req.query = query
28+
return
29+
}
30+
31+
// Process query parameters with optimized URLSearchParams handling
32+
const searchParams = new URLSearchParams(search.replace(/\[\]=/g, '='))
33+
34+
for (const [name, value] of searchParams.entries()) {
35+
// Use Set for O(1) dangerous property lookup instead of multiple string comparisons
36+
if (DANGEROUS_PROPERTIES.has(name)) {
37+
continue // Skip dangerous property names
38+
}
39+
40+
const existing = query[name]
41+
if (existing !== undefined) {
42+
// Optimized array handling - check type once, then branch
43+
if (Array.isArray(existing)) {
44+
existing.push(value)
1245
} else {
13-
query[name] = value
46+
query[name] = [existing, value]
1447
}
48+
} else {
49+
query[name] = value
1550
}
1651
}
1752

18-
req.path = path
1953
req.query = query
2054
}

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@
2828
},
2929
"homepage": "https://github.com/BackendStack21/0http#readme",
3030
"devDependencies": {
31-
"0http": "^4.1.0",
31+
"0http": "^4.2.0",
3232
"@types/node": "^22.10.5",
3333
"body-parser": "^1.20.1",
3434
"chai": "^4.3.7",
3535
"cross-env": "^7.0.3",
36-
"mitata": "^1.0.30",
36+
"mitata": "^1.0.34",
3737
"mocha": "^11.0.1",
3838
"nyc": "^17.1.0",
3939
"supertest": "^7.0.0"

0 commit comments

Comments
 (0)