Skip to content

Commit 5b6feb6

Browse files
committed
test: add comprehensive test coverage for all 43 security fixes
- Body parser: RAW_BODY_SYMBOL, empty body, nesting depth, streaming limits - JWT auth: token type validation, algorithm confusion, timing-safe comparison - Rate limit: minimal headers, unique unknown keys, IP spoofing prevention - CORS: allowedHeaders caching, null origin rejection - Router: prototype pollution, LRU cache, frozen params - Security: dedicated prototype pollution test suite - Integration & regression: updated for new security behaviors 483 tests passing, 1906 expect() calls.
1 parent a46859e commit 5b6feb6

File tree

11 files changed

+463
-85
lines changed

11 files changed

+463
-85
lines changed

test/integration/router.test.js

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,12 @@ describe('Router Integration Tests', () => {
164164

165165
describe('Error Handling', () => {
166166
it('should return a 500 response for a route that throws an error', async () => {
167+
const originalError = console.error
168+
console.error = () => {}
167169
const response = await router.fetch(createTestRequest('GET', '/error'))
170+
console.error = originalError
168171
expect(response.status).toBe(500)
169-
expect(await response.text()).toEqual('Unexpected error')
172+
expect(await response.text()).toEqual('Internal Server Error')
170173
})
171174

172175
it('should return a 404 response for a non-existent route', async () => {
@@ -346,12 +349,12 @@ describe('Router Integration Tests', () => {
346349
const testCases = [
347350
{
348351
url: '/search/hello%20world?filter=test%20value',
349-
expectedTerm: 'hello%20world',
352+
expectedTerm: 'hello world',
350353
expectedQuery: {filter: 'test value'}, // Query parser decodes spaces
351354
},
352355
{
353356
url: '/search/café?type=beverage',
354-
expectedTerm: 'caf%C3%A9', // URL parameters are URL-encoded
357+
expectedTerm: 'café',
355358
expectedQuery: {type: 'beverage'},
356359
},
357360
{
@@ -426,9 +429,14 @@ describe('Router Integration Tests', () => {
426429
return {success: true}
427430
})
428431

432+
const originalError = console.error
433+
console.error = () => {}
434+
429435
const req = createTestRequest('GET', '/api/error/test')
430436
const result = await router.fetch(req)
431437

438+
console.error = originalError
439+
432440
// Should get error response with error handling
433441
expect(result.status).toBe(500)
434442
expect(req.step1).toBe(true)

test/performance/regression.test.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,13 +214,19 @@ describe('Performance Regression Tests', () => {
214214
throw new Error('Test error')
215215
})
216216

217+
// Suppress console.error during perf test (default error handler logs errors)
218+
const originalError = console.error
219+
console.error = () => {}
220+
217221
const iterations = 500
218222
const {time} = await measureTime(async () => {
219223
for (let i = 0; i < iterations; i++) {
220224
await router.fetch(createTestRequest('GET', '/error-test'))
221225
}
222226
})
223227

228+
console.error = originalError
229+
224230
const avgTime = time / iterations
225231
expect(avgTime).toBeLessThan(3) // Error handling should not be too slow
226232
})

test/security/prototype-pollution.test.js

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,4 +207,77 @@ describe('Prototype Pollution Security Tests', () => {
207207
expect({}.polluted).toBeUndefined()
208208
})
209209
})
210+
211+
describe('Query Prototype Pollution Protection (L-1)', () => {
212+
it('should filter __proto__ from query parameters', async () => {
213+
routerInstance.get('/search', (req) => {
214+
return Response.json({query: req.query})
215+
})
216+
217+
const testReq = {
218+
method: 'GET',
219+
url: 'http://localhost/search?__proto__[polluted]=true&safe=value',
220+
headers: {},
221+
}
222+
223+
await routerInstance.fetch(testReq)
224+
225+
// __proto__ key should be filtered from query
226+
expect(testReq.query['__proto__']).toBeUndefined()
227+
expect(testReq.query.safe).toBe('value')
228+
expect({}.polluted).toBeUndefined()
229+
})
230+
231+
it('should filter constructor from query parameters', async () => {
232+
routerInstance.get('/search', (req) => {
233+
return Response.json({query: req.query})
234+
})
235+
236+
const testReq = {
237+
method: 'GET',
238+
url: 'http://localhost/search?constructor=malicious&name=test',
239+
headers: {},
240+
}
241+
242+
await routerInstance.fetch(testReq)
243+
244+
expect(testReq.query['constructor']).toBeUndefined()
245+
expect(testReq.query.name).toBe('test')
246+
})
247+
248+
it('should filter prototype from query parameters', async () => {
249+
routerInstance.get('/search', (req) => {
250+
return Response.json({query: req.query})
251+
})
252+
253+
const testReq = {
254+
method: 'GET',
255+
url: 'http://localhost/search?prototype=bad&ok=yes',
256+
headers: {},
257+
}
258+
259+
await routerInstance.fetch(testReq)
260+
261+
expect(testReq.query['prototype']).toBeUndefined()
262+
expect(testReq.query.ok).toBe('yes')
263+
})
264+
265+
it('should still allow normal query parameters', async () => {
266+
routerInstance.get('/search', (req) => {
267+
return Response.json({query: req.query})
268+
})
269+
270+
const testReq = {
271+
method: 'GET',
272+
url: 'http://localhost/search?page=1&limit=10&filter=active',
273+
headers: {},
274+
}
275+
276+
await routerInstance.fetch(testReq)
277+
278+
expect(testReq.query.page).toBe('1')
279+
expect(testReq.query.limit).toBe('10')
280+
expect(testReq.query.filter).toBe('active')
281+
})
282+
})
210283
})

test/unit/body-parser.test.js

Lines changed: 77 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ describe('Body Parser Middleware', () => {
395395
const middleware = bodyParser()
396396
await middleware(req, next)
397397

398-
expect(req.body).toEqual({})
398+
expect(req.body).toBeUndefined()
399399
expect(next).toHaveBeenCalled()
400400
})
401401

@@ -899,8 +899,8 @@ describe('Body Parser Middleware', () => {
899899
})
900900

901901
it('should handle non-string, non-number limit values', () => {
902-
expect(parseLimit(null)).toBe(1024 * 1024) // Default 1MB
903-
expect(parseLimit(undefined)).toBe(1024 * 1024) // Default 1MB
902+
expect(() => parseLimit(null)).toThrow(TypeError)
903+
expect(() => parseLimit(undefined)).toThrow(TypeError)
904904
})
905905

906906
it('should handle custom JSON parser in main body parser', async () => {
@@ -1261,9 +1261,9 @@ describe('Body Parser Middleware', () => {
12611261

12621262
const {createTextParser} = require('../../lib/middleware/body-parser')
12631263
const middleware = createTextParser()
1264-
await expect(middleware(mockReq, () => {})).rejects.toThrow(
1265-
'Text parsing failed',
1266-
)
1264+
const response = await middleware(mockReq, () => {})
1265+
expect(response.status).toBe(400)
1266+
expect(await response.text()).toBe('Failed to read request body')
12671267
})
12681268

12691269
test('should handle invalid content-length in URL-encoded parser', async () => {
@@ -1322,9 +1322,9 @@ describe('Body Parser Middleware', () => {
13221322
createURLEncodedParser,
13231323
} = require('../../lib/middleware/body-parser')
13241324
const middleware = createURLEncodedParser()
1325-
await expect(middleware(mockReq, () => {})).rejects.toThrow(
1326-
'URL-encoded parsing failed',
1327-
)
1325+
const response = await middleware(mockReq, () => {})
1326+
expect(response.status).toBe(400)
1327+
expect(await response.text()).toBe('Failed to read request body')
13281328
})
13291329

13301330
test('should handle invalid content-length in multipart parser', async () => {
@@ -1607,9 +1607,9 @@ describe('Body Parser Middleware', () => {
16071607

16081608
const {createTextParser} = require('../../lib/middleware/body-parser')
16091609
const middleware = createTextParser()
1610-
await expect(middleware(mockReq, () => {})).rejects.toThrow(
1611-
'Text reading failed',
1612-
)
1610+
const response = await middleware(mockReq, () => {})
1611+
expect(response.status).toBe(400)
1612+
expect(await response.text()).toBe('Failed to read request body')
16131613
})
16141614

16151615
test('should handle URL-encoded early return for non-matching content type (lines 360-361)', async () => {
@@ -1701,10 +1701,7 @@ describe('Body Parser Middleware', () => {
17011701
// This might not be possible due to the restrictive regex
17021702
expect(() => parseLimit('0.b')).toThrow('Invalid limit format')
17031703
} catch (e) {
1704-
// If this doesn't work, the line might be unreachable
1705-
console.log(
1706-
'Line 40 may be mathematically unreachable due to regex constraints',
1707-
)
1704+
// Line 40 may be mathematically unreachable due to regex constraints
17081705
}
17091706
})
17101707

@@ -1720,7 +1717,6 @@ describe('Body Parser Middleware', () => {
17201717
['content-type', 'application/x-www-form-urlencoded'],
17211718
]),
17221719
text: async () => 'simpleKey=simpleValue', // Simple key without brackets
1723-
_rawBodyText: 'simpleKey=simpleValue',
17241720
}
17251721

17261722
const urlEncodedParser = createURLEncodedParser({
@@ -1741,7 +1737,6 @@ describe('Body Parser Middleware', () => {
17411737
method: 'POST',
17421738
headers: new Map([['content-type', 'application/json']]),
17431739
text: async () => '', // Empty string should be handled
1744-
_rawBodyText: '',
17451740
}
17461741

17471742
const jsonParser = createJSONParser()
@@ -1815,7 +1810,6 @@ describe('Body Parser Middleware', () => {
18151810
['content-length', bodyContent.length.toString()],
18161811
]),
18171812
text: async () => bodyContent,
1818-
_rawBodyText: bodyContent,
18191813
}
18201814

18211815
const urlEncodedParser = createURLEncodedParser({limit: 500}) // Small limit
@@ -1870,3 +1864,67 @@ describe('Body Parser Middleware', () => {
18701864
})
18711865
})
18721866
})
1867+
1868+
describe('RAW_BODY_SYMBOL Security (L-3)', () => {
1869+
const {
1870+
createJSONParser,
1871+
createTextParser,
1872+
createURLEncodedParser,
1873+
RAW_BODY_SYMBOL,
1874+
} = require('../../lib/middleware/body-parser')
1875+
const {createTestRequest} = require('../helpers')
1876+
1877+
it('should export RAW_BODY_SYMBOL', () => {
1878+
expect(RAW_BODY_SYMBOL).toBeDefined()
1879+
expect(typeof RAW_BODY_SYMBOL).toBe('symbol')
1880+
expect(RAW_BODY_SYMBOL.toString()).toBe('Symbol(0http.rawBody)')
1881+
})
1882+
1883+
it('should store raw body via Symbol in JSON parser', async () => {
1884+
const req = createTestRequest('POST', '/api/data', {
1885+
headers: {'Content-Type': 'application/json'},
1886+
body: '{"key": "value"}',
1887+
})
1888+
1889+
const parser = createJSONParser()
1890+
const next = jest.fn()
1891+
await parser(req, next)
1892+
1893+
// Raw body should be accessible via Symbol
1894+
expect(req[RAW_BODY_SYMBOL]).toBe('{"key": "value"}')
1895+
// Raw body should NOT be accessible as a regular string property
1896+
expect(req._rawBodyText).toBeUndefined()
1897+
})
1898+
1899+
it('should store raw body via Symbol in text parser', async () => {
1900+
const req = createTestRequest('POST', '/api/data', {
1901+
headers: {'Content-Type': 'text/plain'},
1902+
body: 'hello world',
1903+
})
1904+
1905+
const parser = createTextParser()
1906+
const next = jest.fn()
1907+
await parser(req, next)
1908+
1909+
expect(req[RAW_BODY_SYMBOL]).toBe('hello world')
1910+
expect(req._rawBodyText).toBeUndefined()
1911+
})
1912+
1913+
it('should not expose raw body as enumerable property', async () => {
1914+
const req = createTestRequest('POST', '/api/data', {
1915+
headers: {'Content-Type': 'application/json'},
1916+
body: '{"secret": "password123"}',
1917+
})
1918+
1919+
const parser = createJSONParser()
1920+
const next = jest.fn()
1921+
await parser(req, next)
1922+
1923+
// Symbol-keyed properties should not appear in Object.keys
1924+
const keys = Object.keys(req)
1925+
expect(keys).not.toContain('_rawBodyText')
1926+
// The raw body should only be accessible via Symbol
1927+
expect(req._rawBodyText).toBeUndefined()
1928+
expect(req[RAW_BODY_SYMBOL]).toBe('{"secret": "password123"}')
1929+
})
1930+
})

test/unit/config.test.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,16 @@ describe('Router Configuration', () => {
9696
throw new Error('Test error')
9797
})
9898

99+
const originalError = console.error
100+
console.error = () => {}
101+
99102
const response = await router.fetch(
100103
new Request('http://localhost:3000/test-error', {
101104
method: 'GET',
102105
}),
103106
)
107+
108+
console.error = originalError
104109
expect(response.status).toBe(500)
105110
})
106111
})

test/unit/cors.test.js

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,10 @@ describe('CORS Middleware', () => {
667667

668668
expect(response.status).toBe(204)
669669
expect(headersFunction).toHaveBeenCalledWith(req)
670-
expect(response.headers.get('Access-Control-Allow-Headers')).toBe('')
670+
// I-2: string return values are now correctly handled as single-element arrays
671+
expect(response.headers.get('Access-Control-Allow-Headers')).toBe(
672+
'Content-Type',
673+
)
671674
expect(next).not.toHaveBeenCalled()
672675
})
673676

@@ -701,4 +704,49 @@ describe('CORS Middleware', () => {
701704
)
702705
})
703706
})
707+
708+
describe('Preflight allowedHeaders Function Caching (I-2)', () => {
709+
it('should call allowedHeaders function only once during preflight', async () => {
710+
req = createTestRequest('OPTIONS', '/api/test')
711+
req.headers = new Headers({
712+
Origin: 'https://example.com',
713+
'Access-Control-Request-Method': 'POST',
714+
'Access-Control-Request-Headers': 'Content-Type',
715+
})
716+
717+
const headersFunction = jest.fn(() => ['Content-Type', 'Authorization'])
718+
719+
const middleware = cors({
720+
origin: 'https://example.com',
721+
allowedHeaders: headersFunction,
722+
})
723+
724+
const response = await middleware(req, next)
725+
726+
expect(response.status).toBe(204)
727+
// I-2: should be called exactly once, not 3 times
728+
expect(headersFunction).toHaveBeenCalledTimes(1)
729+
expect(response.headers.get('Access-Control-Allow-Headers')).toBe(
730+
'Content-Type, Authorization',
731+
)
732+
})
733+
734+
it('should still call allowedHeaders function for non-preflight requests', async () => {
735+
req.headers = new Headers({Origin: 'https://example.com'})
736+
737+
const headersFunction = jest.fn(() => ['Content-Type'])
738+
739+
const middleware = cors({
740+
origin: 'https://example.com',
741+
allowedHeaders: headersFunction,
742+
})
743+
744+
const response = await middleware(req, next)
745+
746+
expect(headersFunction).toHaveBeenCalledTimes(1)
747+
expect(response.headers.get('Access-Control-Allow-Headers')).toBe(
748+
'Content-Type',
749+
)
750+
})
751+
})
704752
})

test/unit/edge-cases.test.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,9 +286,14 @@ describe('Router Edge Cases and Boundary Conditions', () => {
286286
// This should not break the router
287287
router.get('/test', null, (req) => ({message: 'success'}))
288288

289+
const originalError = console.error
290+
console.error = () => {}
291+
289292
const req = createTestRequest('GET', '/test')
290293
const result = await router.fetch(req)
291294

295+
console.error = originalError
296+
292297
// Should handle gracefully
293298
expect(typeof result).toBe('object')
294299
})

0 commit comments

Comments
 (0)