diff --git a/lib/next.js b/lib/next.js index 80ae0db..9f9a864 100644 --- a/lib/next.js +++ b/lib/next.js @@ -17,14 +17,14 @@ function next (middlewares, req, res, index, routers, defaultRoute, errorHandler return !res.finished && defaultRoute(req, res) } - // Get current middleware and increment index - const middleware = middlewares[index++] + // Get current middleware + const middleware = middlewares[index] // Create step function - this is called by middleware to continue the chain const step = function (err) { return err ? errorHandler(err, req, res) - : next(middlewares, req, res, index, routers, defaultRoute, errorHandler) + : next(middlewares, req, res, index + 1, routers, defaultRoute, errorHandler) } try { @@ -41,7 +41,7 @@ function next (middlewares, req, res, index, routers, defaultRoute, errorHandler // Replace pattern in URL - this is a hot path, optimize it req.url = req.url.replace(pattern, '') - // Ensure URL starts with a slash + // Ensure URL starts with a slash - use charCodeAt for performance if (req.url.length === 0 || req.url.charCodeAt(0) !== 47) { // 47 is '/' req.url = '/' + req.url } diff --git a/lib/router/sequential.js b/lib/router/sequential.js index 3bbf283..8a2e884 100644 --- a/lib/router/sequential.js +++ b/lib/router/sequential.js @@ -2,9 +2,12 @@ const { Trouter } = require('trouter') const next = require('./../next') const { parse } = require('regexparam') const { LRUCache: Cache } = require('lru-cache') -const queryparams = require('../utils/queryparams') +const queryparams = require('./../utils/queryparams') -// Default handlers as constants to avoid creating functions on each router instance +/** + * Default handlers as constants to avoid creating functions on each router instance. + * This reduces memory allocation and improves performance when multiple routers are created. + */ const DEFAULT_ROUTE = (req, res) => { res.statusCode = 404 res.end() @@ -12,11 +15,19 @@ const DEFAULT_ROUTE = (req, res) => { const DEFAULT_ERROR_HANDLER = (err, req, res) => { res.statusCode = 500 + // Note: err.message could expose sensitive information in production res.end(err.message) } -// Simple ID generator -const generateId = () => Math.random().toString(36).substring(2, 10).toUpperCase() +/** + * Simple ID generator using Math.random for router identification. + * Warning: Not cryptographically secure - suitable only for internal routing logic. + * Optimized to minimize string operations. + */ +const generateId = () => { + // Use a more efficient approach - avoid substring operations + return Math.random().toString(36).slice(2, 10).toUpperCase() +} module.exports = (config = {}) => { // Use object destructuring with defaults for cleaner config initialization @@ -29,14 +40,28 @@ module.exports = (config = {}) => { const routers = {} - // Initialize cache only once + /** + * Initialize LRU cache for route matching results with optimized settings. + * Cache keys are method+path combinations to speed up repeated lookups. + * - cacheSize > 0: Limited LRU cache with specified max entries + * - cacheSize = 0: No caching (disabled) + * - cacheSize < 0: Large LRU cache (50k entries) for "unlimited" mode + * Optimized cache size for better memory management and performance. + */ let cache = null if (cacheSize > 0) { - cache = new Cache({ max: cacheSize }) + cache = new Cache({ + max: cacheSize, + updateAgeOnGet: false, // Disable age updates for better performance + updateAgeOnHas: false + }) } else if (cacheSize < 0) { - // For unlimited cache, still use LRUCache but with a very high max - // This provides better memory management than an unbounded Map - cache = new Cache({ max: 100000 }) + // Reduced from 100k to 50k for better memory efficiency while maintaining performance + cache = new Cache({ + max: 50000, + updateAgeOnGet: false, + updateAgeOnHas: false + }) } const router = new Trouter() @@ -44,6 +69,12 @@ module.exports = (config = {}) => { const _use = router.use + /** + * Enhanced router.use method with support for nested routers. + * Handles both middleware functions and nested router instances. + * Automatically handles prefix parsing when first argument is a function. + * Optimized for minimal overhead in the common case. + */ router.use = (prefix, ...middlewares) => { if (typeof prefix === 'function') { middlewares = [prefix, ...middlewares] @@ -51,66 +82,85 @@ module.exports = (config = {}) => { } _use.call(router, prefix, middlewares) - if (middlewares[0]?.id) { - // caching router -> pattern relation for urls pattern replacement + // Optimized nested router detection - check first middleware only + const firstMiddleware = middlewares[0] + if (firstMiddleware?.id) { + // Cache router -> pattern relation for URL pattern replacement in nested routing + // This enables efficient URL rewriting when entering nested router contexts const { pattern } = parse(prefix, true) - routers[middlewares[0].id] = pattern + routers[firstMiddleware.id] = pattern } - return router // Fix: return router instead of this + return router // Ensure chainable API by returning router instance } - // Create the cleanup middleware once - const createCleanupMiddleware = (step) => (req, res, next) => { - req.url = req.preRouterUrl - req.path = req.preRouterPath - - req.preRouterUrl = undefined - req.preRouterPath = undefined - - return step() + /** + * Creates cleanup middleware for nested router restoration. + * This middleware restores the original URL and path after nested router processing. + * Uses property deletion instead of undefined assignment for better performance. + * Optimized to minimize closure creation overhead. + */ + const createCleanupMiddleware = (step) => { + // Pre-create the cleanup function to avoid repeated function creation + return (req, res, next) => { + req.url = req.preRouterUrl + req.path = req.preRouterPath + + // Use delete for better performance than setting undefined + delete req.preRouterUrl + delete req.preRouterPath + + return step() + } } router.lookup = (req, res, step) => { - // Initialize URL and originalUrl if needed - req.url = req.url || '/' - req.originalUrl = req.originalUrl || req.url + // Initialize URL and originalUrl if needed - use nullish coalescing for better performance + req.url ??= '/' + req.originalUrl ??= req.url - // Parse query parameters + // Parse query parameters using optimized utility queryparams(req, req.url) - // Fast path for cache lookup - const reqCacheKey = cache && (req.method + req.path) - let match = cache && cache.get(reqCacheKey) + // Cache lookup optimization - minimize variable assignments + let match + if (cache) { + // Pre-compute cache key with direct concatenation (fastest approach) + const reqCacheKey = req.method + req.path + match = cache.get(reqCacheKey) - if (!match) { - match = router.find(req.method, req.path) - if (cache && reqCacheKey) { + if (!match) { + match = router.find(req.method, req.path) cache.set(reqCacheKey, match) } + } else { + match = router.find(req.method, req.path) } const { handlers, params } = match - if (handlers.length > 0) { - // Avoid creating a new array with spread operator - // Use the handlers array directly + if (handlers.length) { + // Optimized middleware array handling let middlewares - if (step !== undefined) { - // Only create a new array if we need to add the cleanup middleware + // Create new array only when step middleware is needed middlewares = handlers.slice() middlewares.push(createCleanupMiddleware(step)) } else { middlewares = handlers } - // Initialize params object if needed + // Optimized parameter assignment with minimal overhead if (!req.params) { - req.params = params + // Use pre-created empty object or provided params directly + req.params = params || Object.create(null) } else if (params) { - // Faster than Object.assign for small objects - for (const key in params) { + // Manual property copying - optimized for small objects + // Pre-compute keys and length to avoid repeated calls + const paramKeys = Object.keys(params) + let i = paramKeys.length + while (i--) { + const key = paramKeys[i] req.params[key] = params[key] } } @@ -121,6 +171,10 @@ module.exports = (config = {}) => { } } + /** + * Shorthand method for registering routes with specific HTTP methods. + * Delegates to router.add with the provided method, pattern, and handlers. + */ router.on = (method, pattern, ...handlers) => router.add(method, pattern, handlers) return router diff --git a/lib/utils/object.js b/lib/utils/object.js deleted file mode 100644 index c7b4623..0000000 --- a/lib/utils/object.js +++ /dev/null @@ -1,10 +0,0 @@ -const forEach = (obj, cb) => { - const keys = Object.keys(obj) - for (let i = 0, length = keys.length; i < length; i++) { - cb(obj[keys[i]], keys[i]) - } -} - -module.exports = { - forEach -} diff --git a/lib/utils/queryparams.js b/lib/utils/queryparams.js index 0909264..14e6c9e 100644 --- a/lib/utils/queryparams.js +++ b/lib/utils/queryparams.js @@ -1,20 +1,58 @@ +// Pre-create Set for dangerous properties - faster O(1) lookup vs string comparisons +const DANGEROUS_PROPERTIES = new Set(['__proto__', 'constructor', 'prototype']) + +// Pre-created empty query object to avoid allocations +const EMPTY_QUERY = Object.freeze(Object.create(null)) + module.exports = (req, url) => { - const query = {} - const indexOfQuestionMark = url.indexOf('?') - const path = indexOfQuestionMark !== -1 ? url.slice(0, indexOfQuestionMark) : url - const search = indexOfQuestionMark !== -1 ? url.slice(indexOfQuestionMark + 1) : '' - - if (search.length > 0) { - const searchParams = new URLSearchParams(search.replace(/\[\]=/g, '=')) - for (const [name, value] of searchParams.entries()) { - if (query[name]) { - Array.isArray(query[name]) ? query[name].push(value) : (query[name] = [query[name], value]) + // Single indexOf call - more efficient than multiple operations + const questionMarkIndex = url.indexOf('?') + + if (questionMarkIndex === -1) { + // Fast path: no query string + req.path = url + req.query = EMPTY_QUERY + return + } + + // Use Object.create(null) for prototype pollution protection + const query = Object.create(null) + + // Extract path and search in one operation each + req.path = url.slice(0, questionMarkIndex) + const search = url.slice(questionMarkIndex + 1) + + if (search.length === 0) { + // Fast path: empty query string + req.query = query + return + } + + // Process query parameters with optimized URLSearchParams handling + const searchParams = new URLSearchParams(search.replace(/\[\]=/g, '=')) + + for (const [name, value] of searchParams.entries()) { + // Split parameter name into segments by dot or bracket notation + /* eslint-disable-next-line */ + const segments = name.split(/[\.\[\]]+/).filter(Boolean) + + // Check each segment against the dangerous properties set + if (segments.some(segment => DANGEROUS_PROPERTIES.has(segment))) { + continue // Skip dangerous property names + } + + const existing = query[name] + if (existing !== undefined) { + // Optimized array handling - check type once, then branch + if (Array.isArray(existing)) { + existing.push(value) } else { - query[name] = value + query[name] = [existing, value] } + } else { + query[name] = value } } - req.path = path req.query = query } diff --git a/package.json b/package.json index 1d63793..a3442a3 100644 --- a/package.json +++ b/package.json @@ -28,12 +28,12 @@ }, "homepage": "https://github.com/BackendStack21/0http#readme", "devDependencies": { - "0http": "^4.1.0", + "0http": "^4.2.0", "@types/node": "^22.10.5", "body-parser": "^1.20.1", "chai": "^4.3.7", "cross-env": "^7.0.3", - "mitata": "^1.0.30", + "mitata": "^1.0.34", "mocha": "^11.0.1", "nyc": "^17.1.0", "supertest": "^7.0.0" diff --git a/tooling/advanced-pollution-test.js b/tooling/advanced-pollution-test.js new file mode 100644 index 0000000..1687ec6 --- /dev/null +++ b/tooling/advanced-pollution-test.js @@ -0,0 +1,98 @@ +#!/usr/bin/env node + +/** + * Advanced test for prototype pollution protection including edge cases + */ + +const sequential = require('../lib/router/sequential') + +console.log('Advanced prototype pollution protection test...\n') + +// Test direct property access attempts +const testCases = [ + // Standard URLSearchParams parsing might decode these differently + '/test?__proto__=polluted', + '/test?constructor=polluted', + '/test?prototype=polluted', + + // URL encoded versions + '/test?%5F%5Fproto%5F%5F=polluted', // __proto__ + '/test?constructor%2Eprototype=polluted', // constructor.prototype + + // Multiple same-name parameters (should create arrays) + '/test?__proto__=value1&__proto__=value2', + '/test?constructor=value1&constructor=value2', + + // Mixed dangerous and safe parameters + '/test?safe=value&__proto__=polluted&another=safe' +] + +console.log('Before tests:') +console.log('- Object.prototype.polluted:', Object.prototype.polluted) +console.log('- Object.prototype.testProp:', Object.prototype.testProp) + +// Mock request/response objects +function createMockReq (url) { + return { + method: 'GET', + url, + headers: {} + } +} + +function createMockRes () { + return { + statusCode: 200, + writeHead: () => {}, + end: () => {}, + finished: false + } +} + +const router = sequential() + +// Mock a simple route handler +router.get('/test', (req, res) => { + console.log('Query keys:', Object.keys(req.query)) + console.log('Query values:', req.query) + console.log('Prototype:', Object.getPrototypeOf(req.query)) + res.end() +}) + +// Test each case +testCases.forEach((testUrl, index) => { + console.log(`\n--- Test ${index + 1}: ${testUrl} ---`) + + const req = createMockReq(testUrl) + const res = createMockRes() + + try { + router.lookup(req, res) + } catch (error) { + console.error('Error during lookup:', error.message) + } +}) + +console.log('\nAfter tests:') +console.log('- Object.prototype.polluted:', Object.prototype.polluted) +console.log('- Object.prototype.testProp:', Object.prototype.testProp) + +// Test if we can access dangerous properties through any means +try { + const testObj = {} + /* eslint-disable no-proto */ + console.log('- testObj.__proto__:', testObj.__proto__) + console.log('- testObj.constructor:', testObj.constructor) +} catch (e) { + console.log('- Error accessing prototype properties:', e.message) +} + +// Final verification +const hasPollution = Object.prototype.polluted || Object.prototype.testProp +if (hasPollution) { + console.log('\nโŒ VULNERABILITY: Prototype pollution detected!') + process.exit(1) +} else { + console.log('\nโœ… SUCCESS: Advanced prototype pollution protection working!') + process.exit(0) +} diff --git a/tooling/performance-test.js b/tooling/performance-test.js new file mode 100644 index 0000000..3894d5b --- /dev/null +++ b/tooling/performance-test.js @@ -0,0 +1,165 @@ +#!/usr/bin/env node + +/** + * Comprehensive Performance Test Suite + * Tests various aspects of the 0http router performance + */ + +const sequential = require('../lib/router/sequential') + +console.log('๐Ÿš€ Running comprehensive performance tests...\n') + +// Test configurations +const testConfigs = [ + { name: 'No Cache', cacheSize: 0 }, + { name: 'Small Cache (100)', cacheSize: 100 }, + { name: 'Large Cache (1000)', cacheSize: 1000 }, + { name: 'Unlimited Cache', cacheSize: -1 } +] + +// Test scenarios +const scenarios = [ + { + name: 'Simple Route', + url: '/users/123', + setup: (router) => router.get('/users/:id', (req, res) => res.end()) + }, + { + name: 'Complex Route with Query', + url: '/api/v1/users/123/posts?limit=10&offset=20&sort=date', + setup: (router) => router.get('/api/v1/users/:id/posts', (req, res) => res.end()) + }, + { + name: 'Not Found Route', + url: '/nonexistent/path', + setup: (router) => router.get('/existing', (req, res) => res.end()) + }, + { + name: 'Multiple Parameters', + url: '/users/123/posts/456/comments/789', + setup: (router) => router.get('/users/:userId/posts/:postId/comments/:commentId', (req, res) => res.end()) + } +] + +// Mock request/response objects +function createMockReq (url) { + return { + method: 'GET', + url, + headers: {} + } +} + +function createMockRes () { + return { + statusCode: 200, + writeHead: () => {}, + end: () => {}, + finished: false + } +} + +// Run performance tests for each configuration +for (const config of testConfigs) { + console.log(`๐Ÿ“Š Testing: ${config.name}`) + console.log('โ”€'.repeat(50)) + + for (const scenario of scenarios) { + const router = sequential({ cacheSize: config.cacheSize }) + scenario.setup(router) + + // Warm up + for (let i = 0; i < 1000; i++) { + const req = createMockReq(scenario.url) + const res = createMockRes() + router.lookup(req, res) + } + + // Benchmark + const iterations = 50000 + const start = process.hrtime.bigint() + + for (let i = 0; i < iterations; i++) { + const req = createMockReq(scenario.url) + const res = createMockRes() + router.lookup(req, res) + } + + const end = process.hrtime.bigint() + const totalTime = Number(end - start) + const avgTime = totalTime / iterations + + console.log(` ${scenario.name.padEnd(25)} ${(avgTime / 1000).toFixed(2).padStart(8)} ยตs/op`) + } + + console.log() +} + +// Test query parameter parsing performance +console.log('๐Ÿ” Query Parameter Parsing Performance') +console.log('โ”€'.repeat(50)) + +const queryparams = require('../lib/utils/queryparams') +const queryTests = [ + '/path', + '/path?simple=value', + '/path?a=1&b=2&c=3&d=4&e=5', + '/path?arr=1&arr=2&arr=3&arr=4', + '/path?complex=val&simple=test&arr=1&arr=2&encoded=%20%21' +] + +for (const url of queryTests) { + const iterations = 100000 + const start = process.hrtime.bigint() + + for (let i = 0; i < iterations; i++) { + const req = {} + queryparams(req, url) + } + + const end = process.hrtime.bigint() + const totalTime = Number(end - start) + const avgTime = totalTime / iterations + + const queryDesc = url.includes('?') ? `${url.split('?')[1].split('&').length} params` : 'no query' + console.log(` ${queryDesc.padEnd(15)} ${(avgTime / 1000).toFixed(2).padStart(8)} ยตs/op`) +} + +// Memory usage test +console.log('\n๐Ÿ’พ Memory Usage Test') +console.log('โ”€'.repeat(50)) + +const router = sequential({ cacheSize: 1000 }) +router.get('/users/:id', (req, res) => res.end()) +router.get('/posts/:id/comments/:commentId', (req, res) => res.end()) + +const initialMemory = process.memoryUsage() + +// Simulate traffic +for (let i = 0; i < 10000; i++) { + const urls = [ + `/users/${i}`, + `/posts/${i}/comments/${i * 2}`, + `/users/${i}?query=test`, + `/nonexistent/${i}` + ] + + for (const url of urls) { + const req = createMockReq(url) + const res = createMockRes() + router.lookup(req, res) + } +} + +const finalMemory = process.memoryUsage() +const memoryIncrease = { + rss: finalMemory.rss - initialMemory.rss, + heapUsed: finalMemory.heapUsed - initialMemory.heapUsed, + heapTotal: finalMemory.heapTotal - initialMemory.heapTotal +} + +console.log(` RSS increase: ${(memoryIncrease.rss / 1024 / 1024).toFixed(2)} MB`) +console.log(` Heap used increase: ${(memoryIncrease.heapUsed / 1024 / 1024).toFixed(2)} MB`) +console.log(` Heap total increase: ${(memoryIncrease.heapTotal / 1024 / 1024).toFixed(2)} MB`) + +console.log('\nโœ… Performance testing complete!') diff --git a/tooling/prototype-pollution-test.js b/tooling/prototype-pollution-test.js new file mode 100644 index 0000000..12e2041 --- /dev/null +++ b/tooling/prototype-pollution-test.js @@ -0,0 +1,87 @@ +#!/usr/bin/env node + +/** + * Test script to verify prototype pollution protection + */ + +const sequential = require('../lib/router/sequential') + +console.log('Testing prototype pollution protection...\n') + +// Create a router +const router = sequential() + +// Add a simple route +router.get('/test', (req, res) => { + console.log('Query object keys:', Object.keys(req.query)) + console.log('Query object:', req.query) + console.log('Has __proto__ property:', '__proto__' in req.query) + console.log('Has constructor property:', 'constructor' in req.query) + console.log('Query object prototype:', Object.getPrototypeOf(req.query)) + + res.writeHead(200, { 'Content-Type': 'application/json' }) + res.end(JSON.stringify({ + query: req.query, + hasProto: '__proto__' in req.query, + hasConstructor: 'constructor' in req.query, + prototype: Object.getPrototypeOf(req.query) + })) +}) + +// Test various prototype pollution attempts +const testCases = [ + '/test?normal=value', + '/test?__proto__[polluted]=true', + '/test?constructor[prototype][polluted]=true', + '/test?prototype[polluted]=true', + '/test?__proto__.polluted=true', + '/test?constructor.prototype.polluted=true' +] + +console.log('Before tests - Object.prototype.polluted:', Object.prototype.polluted) + +// Mock request/response objects for testing +function createMockReq (url) { + return { + method: 'GET', + url, + headers: {} + } +} + +function createMockRes () { + let data = '' + return { + statusCode: 200, + writeHead: () => {}, + end: (chunk) => { data += chunk }, + finished: false, + _getData: () => data + } +} + +// Test each case +testCases.forEach((testUrl, index) => { + console.log(`\n--- Test ${index + 1}: ${testUrl} ---`) + + const req = createMockReq(testUrl) + const res = createMockRes() + + try { + router.lookup(req, res) + const responseData = JSON.parse(res._getData()) + console.log('Response:', responseData) + } catch (error) { + console.error('Error:', error.message) + } +}) + +console.log('\nAfter tests - Object.prototype.polluted:', Object.prototype.polluted) + +if (Object.prototype.polluted) { + console.log('\nโŒ VULNERABILITY: Prototype pollution detected!') + process.exit(1) +} else { + console.log('\nโœ… SUCCESS: No prototype pollution detected!') + process.exit(0) +} diff --git a/tooling/queryparams-benchmark.js b/tooling/queryparams-benchmark.js new file mode 100644 index 0000000..5005261 --- /dev/null +++ b/tooling/queryparams-benchmark.js @@ -0,0 +1,81 @@ +#!/usr/bin/env node + +/** + * Micro-benchmark for queryparams optimization testing + */ + +const queryparams = require('../lib/utils/queryparams') + +// Test cases that cover different scenarios +const testCases = [ + '/path', // No query string + '/path?', // Empty query string + '/path?simple=value', // Single parameter + '/path?a=1&b=2&c=3', // Multiple parameters + '/path?arr=1&arr=2&arr=3', // Array parameters + '/path?complex=val1&simple=val2&arr=1&arr=2', // Mixed parameters + '/path?encoded=%20%21%40%23', // URL encoded values + '/path?empty=&another=value', // Empty values + '/path?dangerous=safe&normal=value' // Safe parameters with dangerous-sounding names +] + +console.log('Micro-benchmarking queryparams performance...\n') + +// Warm up +for (let i = 0; i < 1000; i++) { + testCases.forEach(url => { + const req = {} + queryparams(req, url) + }) +} + +// Benchmark each test case +testCases.forEach((url, index) => { + const iterations = 100000 + const req = {} + + console.log(`Test ${index + 1}: ${url}`) + + const start = process.hrtime.bigint() + + for (let i = 0; i < iterations; i++) { + // Create a fresh req object each time to avoid caching effects + const testReq = {} + queryparams(testReq, url) + } + + const end = process.hrtime.bigint() + const totalTime = Number(end - start) + const avgTime = totalTime / iterations + + console.log(` ${iterations.toLocaleString()} iterations`) + console.log(` Average: ${(avgTime / 1000).toFixed(2)} ยตs per operation`) + console.log(` Total: ${(totalTime / 1000000).toFixed(2)} ms`) + + // Verify the result is correct + queryparams(req, url) + console.log(` Result: path="${req.path}", query keys=[${Object.keys(req.query).join(', ')}]`) + console.log() +}) + +// Test security - ensure dangerous properties are filtered +console.log('Security verification:') +const securityTests = [ + '/test?__proto__=polluted', + '/test?constructor=dangerous', + '/test?prototype=unsafe', + '/test?safe=value&__proto__=attack&normal=ok' +] + +securityTests.forEach(url => { + const req = {} + queryparams(req, url) + console.log(`${url}:`) + console.log(` Query keys: [${Object.keys(req.query).join(', ')}]`) + console.log(` Has __proto__: ${'__proto__' in req.query}`) + console.log(` Has constructor: ${'constructor' in req.query}`) + console.log(` Prototype: ${Object.getPrototypeOf(req.query)}`) + console.log() +}) + +console.log('โœ… Micro-benchmark complete!')