From 7a868074831eb763d47af053769f30f1db234cc0 Mon Sep 17 00:00:00 2001 From: aarontravass Date: Sun, 8 Oct 2023 19:30:26 -0400 Subject: [PATCH 1/8] test: added tests for app, ip-filter, proxy-addr, rate-limit and req --- package.json | 1 + packages/app/src/app.ts | 9 +-- packages/app/src/request.ts | 12 ++-- packages/proxy-addr/src/index.ts | 7 +-- packages/req/src/fresh.ts | 2 +- pnpm-lock.yaml | 47 ++++++++++++++-- tests/core/app.test.ts | 97 +++++++++++++++++++++++++++----- tests/core/request.test.ts | 35 ++++++++++++ tests/fixtures/views/error.ejs | 1 + tests/fixtures/views/index.ejs | 1 + tests/modules/proxy-addr.test.ts | 15 ++++- tests/modules/req.test.ts | 43 +++++++++++++- 12 files changed, 233 insertions(+), 37 deletions(-) create mode 100644 tests/fixtures/views/error.ejs create mode 100644 tests/fixtures/views/index.ejs diff --git a/package.json b/package.json index 90c8e8f2..4588ca26 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ "c8": "^8.0.1", "colorette": "2.0.20", "dirname-filename-esm": "1.1.1", + "ejs": "^3.1.9", "eslint": "8.46.0", "eslint-config-prettier": "8.10.0", "eslint-plugin-prettier": "5.0.0", diff --git a/packages/app/src/app.ts b/packages/app/src/app.ts index cd60f633..1596c424 100644 --- a/packages/app/src/app.ts +++ b/packages/app/src/app.ts @@ -310,9 +310,9 @@ export class App const exts = this.applyExtensions || extendMiddleware(this) - req.originalUrl = req.url || req.originalUrl + req.originalUrl = req.originalUrl || req.url - const pathname = getPathname(req.originalUrl) + const pathname = getPathname(req.url) const matched = this.#find(pathname) @@ -338,13 +338,11 @@ export class App path: '/' }) } - mw.push({ handler: this.noMatchHandler, type: 'mw', path: '/' }) - const handle = (mw: Middleware) => async (req: Req, res: Res, next?: NextFunction) => { const { path, handler, regex } = mw @@ -353,9 +351,8 @@ export class App try { params = regex ? getURLParams(regex, pathname) : {} } catch (e) { - console.error(e) if (e instanceof URIError) return res.sendStatus(400) // Handle malformed URI - else throw e + return res.sendStatus(500) } req.params = { ...req.params, ...params } diff --git a/packages/app/src/request.ts b/packages/app/src/request.ts index f6d27690..be486c41 100644 --- a/packages/app/src/request.ts +++ b/packages/app/src/request.ts @@ -16,16 +16,20 @@ export { getURLParams } from '@tinyhttp/req' const trustRemoteAddress = ({ socket }: Pick) => { const val = socket.remoteAddress - if (typeof val === 'string') return compile(val.split(',').map((x) => x.trim())) + if (typeof val === 'string') { + return compile(val.split(',').map((x) => x.trim())) + /* c8 ignore next */ + } return compile(val || []) } export const getProtocol = (req: Request): Protocol => { const proto = `http${req.secure ? 's' : ''}` + /* c8 ignore next */ if (!trustRemoteAddress(req)) return proto - const header = (req.headers['X-Forwarded-Proto'] as string) || proto + const header = (req.get('X-Forwarded-Proto') as string) || proto const index = header.indexOf(',') @@ -34,9 +38,7 @@ export const getProtocol = (req: Request): Protocol => { export const getHostname = (req: Request): string | undefined => { let host: string = req.get('X-Forwarded-Host') as string - - if (!host || !trustRemoteAddress(req)) host = req.get('Host') as string - + if (!host) host = req.get('Host') as string if (!host) return // IPv6 literal support diff --git a/packages/proxy-addr/src/index.ts b/packages/proxy-addr/src/index.ts index 8f7ebb91..2dee1a99 100644 --- a/packages/proxy-addr/src/index.ts +++ b/packages/proxy-addr/src/index.ts @@ -119,6 +119,7 @@ export function parseIPNotation(note: string): [IPv4 | IPv6, string | number] { if (typeof range === 'number' && (range <= 0 || range > max)) throw new TypeError('invalid range on address: ' + note) return [ip, range] + /* c8 ignore next */ } /** * Parse netmask string into CIDR range. @@ -137,7 +138,7 @@ function parseNetmask(netmask: string) { * @param trust * @public */ -export function proxyaddr(req: Req, trust: Trust): string { +function proxyaddr(req: Req, trust: Trust): string { const addrs = alladdrs(req, trust) return addrs[addrs.length - 1] @@ -160,7 +161,6 @@ function trustMulti(subnets: (IPv4 | IPv6)[]) { let trusted = ip if (kind !== subnetkind) { if (subnetkind === 'ipv4' && !(ip as IPv6).isIPv4MappedAddress()) continue - if (!ipconv) ipconv = subnetkind === 'ipv4' ? (ip as IPv6).toIPv4Address() : (ip as IPv4).toIPv4MappedAddress() trusted = ipconv @@ -193,5 +193,4 @@ function trustSingle(subnet: IPv4 | IPv6) { } } -export { alladdrs as all } -export { compile } +export { compile, alladdrs as all, proxyaddr } diff --git a/packages/req/src/fresh.ts b/packages/req/src/fresh.ts index a5d8f629..58329084 100644 --- a/packages/req/src/fresh.ts +++ b/packages/req/src/fresh.ts @@ -34,7 +34,7 @@ function isStale(etag: string, noneMatch: string) { export function fresh(reqHeaders: IncomingHttpHeaders, resHeaders: OutgoingHttpHeaders) { const modifiedSince = reqHeaders['if-modified-since'] const noneMatch = reqHeaders['if-none-match'] - + console.log(reqHeaders) if (!modifiedSince && !noneMatch) return false const cacheControl = reqHeaders['cache-control'] diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c4d12682..6b97c02f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1,9 +1,5 @@ lockfileVersion: '6.0' -settings: - autoInstallPeers: true - excludeLinksFromLockfile: false - importers: .: @@ -38,6 +34,9 @@ importers: dirname-filename-esm: specifier: 1.1.1 version: 1.1.1 + ejs: + specifier: ^3.1.9 + version: 3.1.9 eslint: specifier: 8.46.0 version: 8.46.0 @@ -1672,6 +1671,10 @@ packages: resolution: {integrity: sha512-jgsaNduz+ndvGyFt3uSuWqvy4lCnIJiovtouQN5JZHOKCS2QuhEdbcQHFhVksz2N2U9hXJo8odG7ETyWlEeuDw==} dev: true + /async@3.2.4: + resolution: {integrity: sha512-iAB+JbDEGXhyIUavoDl9WP/Jj106Kz9DEn1DPgYw5ruDn0e3Wgi3sKFm55sASdGBNOQB8F59d9qQ7deqrHA8wQ==} + dev: true + /asynckit@0.4.0: resolution: {integrity: sha512-Oei9OH4tRh0YqU3GxhX79dM/mwVgvbZJaSNaRk+bshkj0S5cfHcgYakreBjrHwatXKbz+IoIdYLxrKim2MjW0Q==} dev: true @@ -2156,6 +2159,14 @@ packages: safe-buffer: 5.2.1 dev: true + /ejs@3.1.9: + resolution: {integrity: sha512-rC+QVNMJWv+MtPgkt0y+0rVEIdbtxVADApW9JXrUVlzHetgcyczP/E7DJmWJ4fJCZF2cPcBk0laWO9ZHMG3DmQ==} + engines: {node: '>=0.10.0'} + hasBin: true + dependencies: + jake: 10.8.7 + dev: true + /emoji-regex@8.0.0: resolution: {integrity: sha512-MSjYzcWNOA0ewAHpz0MxpYFvwg6yjy1NG3xteoqz644VCo/RPgnr1/GGt+ic3iJTzQ8Eu3TdM14SawnVUmGE6A==} dev: true @@ -2513,6 +2524,12 @@ packages: flat-cache: 3.0.4 dev: true + /filelist@1.0.4: + resolution: {integrity: sha512-w1cEuf3S+DrLCQL7ET6kz+gmlJdbq9J7yXCSjK/OZCPA+qEN1WyF4ZAf0YYJa4/shHJra2t/d/r8SV4Ji+x+8Q==} + dependencies: + minimatch: 5.1.6 + dev: true + /fill-range@7.0.1: resolution: {integrity: sha512-qOo9F+dMUmC2Lcb4BbVvnKJxTPjCm+RRpe4gDuGrzkL7mEVl/djYSu2OdQ2Pa302N4oqkSg9ir6jaLWJ2USVpQ==} engines: {node: '>=8'} @@ -3154,6 +3171,17 @@ packages: istanbul-lib-report: 3.0.1 dev: true + /jake@10.8.7: + resolution: {integrity: sha512-ZDi3aP+fG/LchyBzUM804VjddnwfSfsdeYkwt8NcbKRvo4rFkjhs456iLFn3k2ZUWvNe4i48WACDbza8fhq2+w==} + engines: {node: '>=10'} + hasBin: true + dependencies: + async: 3.2.4 + chalk: 4.1.2 + filelist: 1.0.4 + minimatch: 3.1.2 + dev: true + /jju@1.4.0: resolution: {integrity: sha512-8wb9Yw966OSxApiCt0K3yNJL8pnNeIv+OEq2YMidz4FKP6nonSRoOXc80iXY4JaN2FC11B9qsNmDsm+ZOfMROA==} dev: true @@ -3485,6 +3513,13 @@ packages: brace-expansion: 1.1.11 dev: true + /minimatch@5.1.6: + resolution: {integrity: sha512-lKwV/1brpG6mBUFHtb7NUmtABCb2WZZmm2wNiOA5hAb8VdCS4B3dtMWyvcoViccwAW/COERjXLt0zP1zXUN26g==} + engines: {node: '>=10'} + dependencies: + brace-expansion: 2.0.1 + dev: true + /minimatch@9.0.3: resolution: {integrity: sha512-RHiac9mvaRw0x3AYRgDC1CxAP7HTcNrrECeA8YYJeWnpo+2Q5CegtZjaotWTWxDG3UeGA1coE05iH1mPjT/2mg==} engines: {node: '>=16 || 14 >=14.17'} @@ -4980,3 +5015,7 @@ packages: optionalDependencies: commander: 9.5.0 dev: true + +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false diff --git a/tests/core/app.test.ts b/tests/core/app.test.ts index e9944f31..81fbdc70 100644 --- a/tests/core/app.test.ts +++ b/tests/core/app.test.ts @@ -1,13 +1,12 @@ /* eslint-disable @typescript-eslint/no-unused-vars */ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import http from 'node:http' import { readFile } from 'node:fs/promises' import { App } from '../../packages/app/src/index' import { renderFile } from 'eta' -import type { PartialConfig } from 'eta/dist/types/config' import { InitAppAndTest } from '../../test_helpers/initAppAndTest' import { makeFetch } from 'supertest-fetch' -import { vi } from 'vitest' +import { renderFile as ejsRenderFile } from 'ejs' import { View } from '../../packages/app/src/view' describe('Testing App', () => { @@ -53,6 +52,20 @@ describe('Testing App', () => { await fetch('/').expect(500, 'Ouch, you hurt me on / page.') }) + it('Default onError with testing', async () => { + vi.stubEnv('TESTING', '') + const app = new App() + + app.use((_req, _res, _next) => { + throw new Error('you') + }) + + const server = app.listen() + const fetch = makeFetch(server) + + await fetch('/').expect(500, 'you') + vi.unstubAllEnvs() + }) it('App works with HTTP 1.1', async () => { const app = new App() @@ -513,6 +526,14 @@ describe('HTTP methods', () => { await fetch('/hello', { method: 'HEAD' }).expect(404) }) + it('Returns statusCode 204 when no handler is present and the request is `HEAD`', async () => { + const app = new App() + app.get('/', (_, res, next) => { + next() + }) + const fetch = makeFetch(app.listen()) + await fetch('/', { method: 'HEAD' }).expectStatus(204) + }) }) describe('Route handlers', () => { @@ -737,18 +758,16 @@ describe('Subapps', () => { app.route('/path').get((_, res) => res.send('Hello World')) }) - /* it('req.originalUrl does not change', async () => { + it('req.originalUrl does not change', async () => { const app = new App() const subApp = new App() - subApp.get('/route', (req, res) => + subApp.get('/route', (req, res) => { res.send({ - origUrl: req.originalUrl, - url: req.url, - path: req.path + origUrl: req.originalUrl }) - ) + }) app.use('/subapp', subApp) @@ -757,11 +776,9 @@ describe('Subapps', () => { const fetch = makeFetch(server) await fetch('/subapp/route').expect(200, { - origUrl: '/subapp/route', - url: '/route', - path: '/route' + origUrl: '/subapp/route' }) - }) */ + }) it('lets other wares handle the URL if subapp doesnt have that path', async () => { const app = new App() @@ -909,6 +926,14 @@ describe('Subapps', () => { const fetch = makeFetch(server) await fetch('/%').expect(400, 'Bad Request') }) + it('Should throw an error when url regex throws an error', async () => { + global.decodeURIComponent = (...args) => { + throw new Error('an error was throw here') + } + const app = new App() + app.get('/:id', (_, res) => res.send('hello')) + await makeFetch(app.listen())('/123').expect(500) + }) it('handles errors by parent when no onError specified', async () => { const app = new App({ onError: (err, req, res) => res.status(500).end(`Ouch, ${err} hurt me on ${req.path} page.`) @@ -990,6 +1015,19 @@ describe('Template engines', () => { expect(str).toEqual('Hello from v1rtl') }) }) + it('should expose app._locals', () => { + const app = new App({ + settings: { + views: `${process.cwd()}/tests/fixtures/views` + } + }) + app.engine('eta', renderFile) + + app.render('index.eta', {}, { _locals: { name: 'world' } }, (err, str) => { + if (err) throw err + expect(str).toEqual('Hello from world') + }) + }) it('should support index files', () => { const app = new App({ settings: { @@ -1007,19 +1045,22 @@ describe('Template engines', () => { }) describe('errors', () => { it('should catch errors', () => { + expect.assertions(1) const app = new App({ settings: { views: `${process.cwd()}/tests/fixtures` } }) - class TestView { + class ErrorTestView { + path = 'something' + constructor(...args) {} render() { throw new Error('oops') } } - app.set('view', TestView as unknown as typeof View) + app.set('view', ErrorTestView as unknown as typeof View) app.render('nothing', {}, {}, (err) => { expect((err as Error).message, 'err!') @@ -1176,6 +1217,32 @@ describe('Template engines', () => { }) }) }) + + it('can render without options and throws error if template renderer throws error', async () => { + const app = new App() + app.engine('ejs', ejsRenderFile) + + const server = app.listen() + + const fetch = makeFetch(server) + try { + app.get('/', (_, res) => res.render('error.ejs').end()) + await fetch('/') + } catch (err) { + expect((err as Error).message).toBe('Could not find matching close tag for "<%=".') + } + }) + it('uses the default engine to render', () => { + const app = new App() + app.set('view engine', '.eta') + app.engine('eta', renderFile) + app.locals.name = 'v1rtl' + + app.render(`${process.cwd()}/tests/fixtures/views/index`, {}, {}, (err, str) => { + if (err) throw err + expect(str).toEqual('Hello from v1rtl') + }) + }) }) describe('App settings', () => { diff --git a/tests/core/request.test.ts b/tests/core/request.test.ts index e9731a3f..25b0b5c3 100644 --- a/tests/core/request.test.ts +++ b/tests/core/request.test.ts @@ -3,6 +3,7 @@ import { InitAppAndTest } from '../../test_helpers/initAppAndTest' import { App } from '../../packages/app/src/app' import { makeFetch } from 'supertest-fetch' import { Agent } from 'node:http' +import { getProtocol, getSubdomains } from '../../packages/app/src' describe('Request properties', () => { it('should have default HTTP Request properties', async () => { @@ -115,6 +116,40 @@ describe('Request properties', () => { await fetch('/').expect(200, `subdomains: `) }) + describe('custom imports', () => { + it('should test getSubdomains when host is null', async () => { + const app = new App() + app.get('/', (req, res) => { + req.headers.host = undefined + res.send(getSubdomains(req)) + }) + await makeFetch(app.listen())('/').expectStatus(200) + }) + it('should test getSubdomains when host is an IP', async () => { + const app = new App() + app.get('/', (req, res) => { + req.headers.host = '127.0.0.1' + res.send(getSubdomains(req)) + }) + await makeFetch(app.listen())('/').expectStatus(200) + }) + it('should test getSubdomains when host is an array', async () => { + const app = new App() + app.get('/', (req, res) => { + req.headers.host = '[127.0.0.1]' + res.send(getSubdomains(req)) + }) + await makeFetch(app.listen())('/').expectStatus(200) + }) + it('should test getProtocol', async () => { + const app = new App() + app.get('/', (req, res) => { + req.secure = true + return res.send(getProtocol(req)) + }) + await makeFetch(app.listen())('/', { headers: { 'X-Forwarded-Proto': 'https, http' } }).expectStatus(200) + }) + }) }) it('req.xhr is false because of node-superagent', async () => { diff --git a/tests/fixtures/views/error.ejs b/tests/fixtures/views/error.ejs new file mode 100644 index 00000000..867a88c8 --- /dev/null +++ b/tests/fixtures/views/error.ejs @@ -0,0 +1 @@ +Hello from <%= name \ No newline at end of file diff --git a/tests/fixtures/views/index.ejs b/tests/fixtures/views/index.ejs new file mode 100644 index 00000000..cccdab79 --- /dev/null +++ b/tests/fixtures/views/index.ejs @@ -0,0 +1 @@ +Hello from <%= name %> \ No newline at end of file diff --git a/tests/modules/proxy-addr.test.ts b/tests/modules/proxy-addr.test.ts index 557076c9..d5d38829 100644 --- a/tests/modules/proxy-addr.test.ts +++ b/tests/modules/proxy-addr.test.ts @@ -25,6 +25,11 @@ describe('proxyaddr(req, trust)', () => { expect(proxyaddr(req, [])).toBe('127.0.0.1') }) + it('should return addr when trust is null', () => { + const req = createReq('127.0.0.1') as IncomingMessage + + expect(proxyaddr(req, undefined)).toBe('127.0.0.1') + }) it('should reject a number', () => { const req = createReq('127.0.0.1') as IncomingMessage @@ -165,6 +170,13 @@ describe('proxyaddr(req, trust)', () => { ['10.0.0.1', 1] ]) }) + it('should not trust non-IP addresses', () => { + const req = createReq('10.0.0.1', { + 'x-forwarded-for': '192.168.0.1, 10.0.0.2, localhost' + }) as IncomingMessage + + expect(proxyaddr(req, '10.0.0.1')).toBe('localhost') + }) }) describe('with all trusted', () => { it('should return socket address with no headers', () => { @@ -354,7 +366,8 @@ describe('proxyaddr(req, trust)', () => { 'x-forwarded-for': '192.168.0.1, 10.0.0.2' }) as IncomingMessage - expect(proxyaddr(req, ['::ffff:a00:1', '::ffff:a00:2'])).toBe('192.168.0.1') + expect(proxyaddr(req, ['::1', '::2'])).toBe('10.0.0.1') + expect(proxyaddr(req, '::1')).toBe('10.0.0.1') }) }) }) diff --git a/tests/modules/req.test.ts b/tests/modules/req.test.ts index 681e3713..2ad56f91 100644 --- a/tests/modules/req.test.ts +++ b/tests/modules/req.test.ts @@ -26,7 +26,7 @@ describe('Request extensions', () => { }) it('should handle "referer"', async () => { const app = runServer((req, res) => { - res.end(getRequestHeader(req)('referrer')) + res.end(getRequestHeader(req)('referer')) }) await makeFetch(app)('/', { @@ -240,6 +240,47 @@ describe('Request extensions', () => { await makeFetch(app)('/').expect('stale') }) + it('returns false if headers are not found', async () => { + const app = runServer((req, res) => { + const fresh = getFreshOrStale(req, res) + + res.end(fresh ? 'fresh' : 'stale') + }) + + await makeFetch(app)('/', { + method: 'GET' + }).expect('stale') + }) + it('returns false if cache control header is set to no-cache', async () => { + const app = runServer((req, res) => { + const fresh = getFreshOrStale(req, res) + + res.end(fresh ? 'fresh' : 'stale') + }) + + await makeFetch(app)('/', { + method: 'GET', + headers: { + 'cache-control': 'no-cache', + 'If-None-Match': '12345' + } + }).expect('stale') + }) + it('returns false if last modified is set', async () => { + const app = runServer((req, res) => { + const fresh = getFreshOrStale(req, res) + + res.end(fresh ? 'fresh' : 'stale') + }) + + await makeFetch(app)('/', { + method: 'GET', + headers: { + 'If-None-Match': '*', + 'if-modified-since': new Date().toDateString() + } + }).expect('stale') + }) }) describe('req.range', () => { From 9937460c840d29a8ab3018431c8748c44c372bce Mon Sep 17 00:00:00 2001 From: aarontravass Date: Sun, 8 Oct 2023 19:34:06 -0400 Subject: [PATCH 2/8] refactor: removed console.log --- packages/req/src/fresh.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/req/src/fresh.ts b/packages/req/src/fresh.ts index 58329084..84fa78db 100644 --- a/packages/req/src/fresh.ts +++ b/packages/req/src/fresh.ts @@ -34,7 +34,6 @@ function isStale(etag: string, noneMatch: string) { export function fresh(reqHeaders: IncomingHttpHeaders, resHeaders: OutgoingHttpHeaders) { const modifiedSince = reqHeaders['if-modified-since'] const noneMatch = reqHeaders['if-none-match'] - console.log(reqHeaders) if (!modifiedSince && !noneMatch) return false const cacheControl = reqHeaders['cache-control'] From 717f183117252cabc4a946d82085b2c205fac94c Mon Sep 17 00:00:00 2001 From: aarontravass Date: Tue, 10 Oct 2023 00:06:04 -0400 Subject: [PATCH 3/8] fix: addressed comments --- package.json | 1 - packages/app/src/app.ts | 2 +- pnpm-lock.yaml | 39 -------------------------------------- tests/core/app.test.ts | 27 +++++++------------------- tests/core/request.test.ts | 2 +- 5 files changed, 9 insertions(+), 62 deletions(-) diff --git a/package.json b/package.json index 4588ca26..90c8e8f2 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,6 @@ "c8": "^8.0.1", "colorette": "2.0.20", "dirname-filename-esm": "1.1.1", - "ejs": "^3.1.9", "eslint": "8.46.0", "eslint-config-prettier": "8.10.0", "eslint-plugin-prettier": "5.0.0", diff --git a/packages/app/src/app.ts b/packages/app/src/app.ts index 1596c424..3fbf05dd 100644 --- a/packages/app/src/app.ts +++ b/packages/app/src/app.ts @@ -352,7 +352,7 @@ export class App params = regex ? getURLParams(regex, pathname) : {} } catch (e) { if (e instanceof URIError) return res.sendStatus(400) // Handle malformed URI - return res.sendStatus(500) + return this.onError(e, req, res) } req.params = { ...req.params, ...params } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6b97c02f..8297cf58 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -34,9 +34,6 @@ importers: dirname-filename-esm: specifier: 1.1.1 version: 1.1.1 - ejs: - specifier: ^3.1.9 - version: 3.1.9 eslint: specifier: 8.46.0 version: 8.46.0 @@ -1671,10 +1668,6 @@ packages: resolution: {integrity: sha512-jgsaNduz+ndvGyFt3uSuWqvy4lCnIJiovtouQN5JZHOKCS2QuhEdbcQHFhVksz2N2U9hXJo8odG7ETyWlEeuDw==} dev: true - /async@3.2.4: - resolution: {integrity: sha512-iAB+JbDEGXhyIUavoDl9WP/Jj106Kz9DEn1DPgYw5ruDn0e3Wgi3sKFm55sASdGBNOQB8F59d9qQ7deqrHA8wQ==} - dev: true - /asynckit@0.4.0: resolution: {integrity: sha512-Oei9OH4tRh0YqU3GxhX79dM/mwVgvbZJaSNaRk+bshkj0S5cfHcgYakreBjrHwatXKbz+IoIdYLxrKim2MjW0Q==} dev: true @@ -2159,14 +2152,6 @@ packages: safe-buffer: 5.2.1 dev: true - /ejs@3.1.9: - resolution: {integrity: sha512-rC+QVNMJWv+MtPgkt0y+0rVEIdbtxVADApW9JXrUVlzHetgcyczP/E7DJmWJ4fJCZF2cPcBk0laWO9ZHMG3DmQ==} - engines: {node: '>=0.10.0'} - hasBin: true - dependencies: - jake: 10.8.7 - dev: true - /emoji-regex@8.0.0: resolution: {integrity: sha512-MSjYzcWNOA0ewAHpz0MxpYFvwg6yjy1NG3xteoqz644VCo/RPgnr1/GGt+ic3iJTzQ8Eu3TdM14SawnVUmGE6A==} dev: true @@ -2524,12 +2509,6 @@ packages: flat-cache: 3.0.4 dev: true - /filelist@1.0.4: - resolution: {integrity: sha512-w1cEuf3S+DrLCQL7ET6kz+gmlJdbq9J7yXCSjK/OZCPA+qEN1WyF4ZAf0YYJa4/shHJra2t/d/r8SV4Ji+x+8Q==} - dependencies: - minimatch: 5.1.6 - dev: true - /fill-range@7.0.1: resolution: {integrity: sha512-qOo9F+dMUmC2Lcb4BbVvnKJxTPjCm+RRpe4gDuGrzkL7mEVl/djYSu2OdQ2Pa302N4oqkSg9ir6jaLWJ2USVpQ==} engines: {node: '>=8'} @@ -3171,17 +3150,6 @@ packages: istanbul-lib-report: 3.0.1 dev: true - /jake@10.8.7: - resolution: {integrity: sha512-ZDi3aP+fG/LchyBzUM804VjddnwfSfsdeYkwt8NcbKRvo4rFkjhs456iLFn3k2ZUWvNe4i48WACDbza8fhq2+w==} - engines: {node: '>=10'} - hasBin: true - dependencies: - async: 3.2.4 - chalk: 4.1.2 - filelist: 1.0.4 - minimatch: 3.1.2 - dev: true - /jju@1.4.0: resolution: {integrity: sha512-8wb9Yw966OSxApiCt0K3yNJL8pnNeIv+OEq2YMidz4FKP6nonSRoOXc80iXY4JaN2FC11B9qsNmDsm+ZOfMROA==} dev: true @@ -3513,13 +3481,6 @@ packages: brace-expansion: 1.1.11 dev: true - /minimatch@5.1.6: - resolution: {integrity: sha512-lKwV/1brpG6mBUFHtb7NUmtABCb2WZZmm2wNiOA5hAb8VdCS4B3dtMWyvcoViccwAW/COERjXLt0zP1zXUN26g==} - engines: {node: '>=10'} - dependencies: - brace-expansion: 2.0.1 - dev: true - /minimatch@9.0.3: resolution: {integrity: sha512-RHiac9mvaRw0x3AYRgDC1CxAP7HTcNrrECeA8YYJeWnpo+2Q5CegtZjaotWTWxDG3UeGA1coE05iH1mPjT/2mg==} engines: {node: '>=16 || 14 >=14.17'} diff --git a/tests/core/app.test.ts b/tests/core/app.test.ts index 81fbdc70..0fc50908 100644 --- a/tests/core/app.test.ts +++ b/tests/core/app.test.ts @@ -6,7 +6,6 @@ import { App } from '../../packages/app/src/index' import { renderFile } from 'eta' import { InitAppAndTest } from '../../test_helpers/initAppAndTest' import { makeFetch } from 'supertest-fetch' -import { renderFile as ejsRenderFile } from 'ejs' import { View } from '../../packages/app/src/view' describe('Testing App', () => { @@ -926,13 +925,15 @@ describe('Subapps', () => { const fetch = makeFetch(server) await fetch('/%').expect(400, 'Bad Request') }) - it('Should throw an error when url regex throws an error', async () => { + it('should return status of 500 if `getURLParams` has an error', async () => { global.decodeURIComponent = (...args) => { throw new Error('an error was throw here') } - const app = new App() + const app = new App({ + onError: (err, req, res) => res.status(500).end(err.message) + }) app.get('/:id', (_, res) => res.send('hello')) - await makeFetch(app.listen())('/123').expect(500) + await makeFetch(app.listen())('/123').expect(500, 'an error was throw here') }) it('handles errors by parent when no onError specified', async () => { const app = new App({ @@ -1052,7 +1053,7 @@ describe('Template engines', () => { } }) - class ErrorTestView { + class TestView { path = 'something' constructor(...args) {} render() { @@ -1060,7 +1061,7 @@ describe('Template engines', () => { } } - app.set('view', ErrorTestView as unknown as typeof View) + app.set('view', TestView as unknown as typeof View) app.render('nothing', {}, {}, (err) => { expect((err as Error).message, 'err!') @@ -1218,20 +1219,6 @@ describe('Template engines', () => { }) }) - it('can render without options and throws error if template renderer throws error', async () => { - const app = new App() - app.engine('ejs', ejsRenderFile) - - const server = app.listen() - - const fetch = makeFetch(server) - try { - app.get('/', (_, res) => res.render('error.ejs').end()) - await fetch('/') - } catch (err) { - expect((err as Error).message).toBe('Could not find matching close tag for "<%=".') - } - }) it('uses the default engine to render', () => { const app = new App() app.set('view engine', '.eta') diff --git a/tests/core/request.test.ts b/tests/core/request.test.ts index 25b0b5c3..c035f03c 100644 --- a/tests/core/request.test.ts +++ b/tests/core/request.test.ts @@ -116,7 +116,7 @@ describe('Request properties', () => { await fetch('/').expect(200, `subdomains: `) }) - describe('custom imports', () => { + describe('internal function tests', () => { it('should test getSubdomains when host is null', async () => { const app = new App() app.get('/', (req, res) => { From ea4bf7b75e96df1e9ae57fcd4b5782ab347027d6 Mon Sep 17 00:00:00 2001 From: aarontravass Date: Mon, 23 Oct 2023 11:35:50 -0400 Subject: [PATCH 4/8] test: removed c8 ignore and added test to check socket destruction --- packages/app/src/request.ts | 8 ++------ packages/proxy-addr/src/index.ts | 1 - tests/core/app.test.ts | 2 +- tests/core/request.test.ts | 16 ++++++++++++++-- tests/fixtures/views/error.ejs | 1 - tests/fixtures/views/index.ejs | 1 - 6 files changed, 17 insertions(+), 12 deletions(-) delete mode 100644 tests/fixtures/views/error.ejs delete mode 100644 tests/fixtures/views/index.ejs diff --git a/packages/app/src/request.ts b/packages/app/src/request.ts index be486c41..0df29082 100644 --- a/packages/app/src/request.ts +++ b/packages/app/src/request.ts @@ -16,17 +16,13 @@ export { getURLParams } from '@tinyhttp/req' const trustRemoteAddress = ({ socket }: Pick) => { const val = socket.remoteAddress - if (typeof val === 'string') { - return compile(val.split(',').map((x) => x.trim())) - /* c8 ignore next */ - } - return compile(val || []) + if (typeof val !== 'string') return compile(val || []) + return compile(val.split(',').map((x) => x.trim())) } export const getProtocol = (req: Request): Protocol => { const proto = `http${req.secure ? 's' : ''}` - /* c8 ignore next */ if (!trustRemoteAddress(req)) return proto const header = (req.get('X-Forwarded-Proto') as string) || proto diff --git a/packages/proxy-addr/src/index.ts b/packages/proxy-addr/src/index.ts index 2dee1a99..18fdccca 100644 --- a/packages/proxy-addr/src/index.ts +++ b/packages/proxy-addr/src/index.ts @@ -119,7 +119,6 @@ export function parseIPNotation(note: string): [IPv4 | IPv6, string | number] { if (typeof range === 'number' && (range <= 0 || range > max)) throw new TypeError('invalid range on address: ' + note) return [ip, range] - /* c8 ignore next */ } /** * Parse netmask string into CIDR range. diff --git a/tests/core/app.test.ts b/tests/core/app.test.ts index 0fc50908..0afaeb31 100644 --- a/tests/core/app.test.ts +++ b/tests/core/app.test.ts @@ -51,7 +51,7 @@ describe('Testing App', () => { await fetch('/').expect(500, 'Ouch, you hurt me on / page.') }) - it('Default onError with testing', async () => { + it('Defaults to onError when `TESTING` env is enabled', async () => { vi.stubEnv('TESTING', '') const app = new App() diff --git a/tests/core/request.test.ts b/tests/core/request.test.ts index c035f03c..59f20ea8 100644 --- a/tests/core/request.test.ts +++ b/tests/core/request.test.ts @@ -1,4 +1,4 @@ -import { describe, it } from 'vitest' +import { describe, expect, it } from 'vitest' import { InitAppAndTest } from '../../test_helpers/initAppAndTest' import { App } from '../../packages/app/src/app' import { makeFetch } from 'supertest-fetch' @@ -107,7 +107,7 @@ describe('Request properties', () => { it('req.subdomains is empty by default', async () => { const { fetch } = InitAppAndTest( (req, res) => { - res.send(`subdomains: ${req.subdomains.join(', ')}`) + res.send(`subdomains: ${req.subdomains?.join(', ')}`) }, '/', 'GET', @@ -149,6 +149,18 @@ describe('Request properties', () => { }) await makeFetch(app.listen())('/', { headers: { 'X-Forwarded-Proto': 'https, http' } }).expectStatus(200) }) + it('should use a default value if socket is destroyed', async () => { + const app = new App() + app.get('/', (req, res) => { + req.socket.destroy() + return res.send(getProtocol(req)) + }) + try { + await makeFetch(app.listen())('/') + } catch (error) { + expect(error).toBeDefined() + } + }) }) }) diff --git a/tests/fixtures/views/error.ejs b/tests/fixtures/views/error.ejs deleted file mode 100644 index 867a88c8..00000000 --- a/tests/fixtures/views/error.ejs +++ /dev/null @@ -1 +0,0 @@ -Hello from <%= name \ No newline at end of file diff --git a/tests/fixtures/views/index.ejs b/tests/fixtures/views/index.ejs deleted file mode 100644 index cccdab79..00000000 --- a/tests/fixtures/views/index.ejs +++ /dev/null @@ -1 +0,0 @@ -Hello from <%= name %> \ No newline at end of file From 560ac63b8aa4345816019a0a7c5b8abb9e50d964 Mon Sep 17 00:00:00 2001 From: aarontravass Date: Sun, 12 Nov 2023 16:43:12 -0500 Subject: [PATCH 5/8] test: improved test names --- tests/core/request.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/core/request.test.ts b/tests/core/request.test.ts index 59f20ea8..3209bcd6 100644 --- a/tests/core/request.test.ts +++ b/tests/core/request.test.ts @@ -117,7 +117,7 @@ describe('Request properties', () => { await fetch('/').expect(200, `subdomains: `) }) describe('internal function tests', () => { - it('should test getSubdomains when host is null', async () => { + it('should test `getSubdomains` function when host is null', async () => { const app = new App() app.get('/', (req, res) => { req.headers.host = undefined @@ -125,7 +125,7 @@ describe('Request properties', () => { }) await makeFetch(app.listen())('/').expectStatus(200) }) - it('should test getSubdomains when host is an IP', async () => { + it('should test `getSubdomains` function when host is an IP', async () => { const app = new App() app.get('/', (req, res) => { req.headers.host = '127.0.0.1' @@ -133,7 +133,7 @@ describe('Request properties', () => { }) await makeFetch(app.listen())('/').expectStatus(200) }) - it('should test getSubdomains when host is an array', async () => { + it('should test `getSubdomains` function when host is an array', async () => { const app = new App() app.get('/', (req, res) => { req.headers.host = '[127.0.0.1]' @@ -141,7 +141,7 @@ describe('Request properties', () => { }) await makeFetch(app.listen())('/').expectStatus(200) }) - it('should test getProtocol', async () => { + it('should test `getProtocol` function', async () => { const app = new App() app.get('/', (req, res) => { req.secure = true @@ -149,7 +149,7 @@ describe('Request properties', () => { }) await makeFetch(app.listen())('/', { headers: { 'X-Forwarded-Proto': 'https, http' } }).expectStatus(200) }) - it('should use a default value if socket is destroyed', async () => { + it('should test `getProtocol` function by using a default value if socket is destroyed', async () => { const app = new App() app.get('/', (req, res) => { req.socket.destroy() From 8c707393c2209f77fcc1fbe72ba2904fd0259af6 Mon Sep 17 00:00:00 2001 From: aarontravass Date: Wed, 15 Nov 2023 22:37:49 -0500 Subject: [PATCH 6/8] test: merged upstream with master and fixed failing tests --- packages/app/src/app.ts | 2 +- tests/core/request.test.ts | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/app/src/app.ts b/packages/app/src/app.ts index 2da2e8f7..b6715713 100644 --- a/packages/app/src/app.ts +++ b/packages/app/src/app.ts @@ -358,7 +358,7 @@ export class App // Warning: users should not use :wild as a pattern let prefix = path if (regex) { - for (const key of regex.keys) { + for (const key of regex.keys as string[]) { if (key === 'wild') { prefix = prefix.replace('*', params.wild) } else { diff --git a/tests/core/request.test.ts b/tests/core/request.test.ts index fc1d014d..ec5ced74 100644 --- a/tests/core/request.test.ts +++ b/tests/core/request.test.ts @@ -64,12 +64,12 @@ describe('Request properties', () => { }) await fetch('/s/t/u/a1/b/c').expect(200, { - url: '/a1/b/c', + url: '/s/t/u/a1/b/c', params: { pat1: 't', pat2: 'u', wild: 'c' } }) await fetch('/s/t/u/a2/b/c').expect(200, { - url: '/a2/b/c', + url: '/s/t/u/a2/b/c', params: { pat1: 't', pat2: 'u', pat: 'c' } }) }) @@ -84,7 +84,7 @@ describe('Request properties', () => { new App() .get('/', echo) .use('/a1/b', echo) - .use('/a2/b', mw, mw, mw, (req, res) => res.send({ urls: req.urls, params: req.params })) + .use('/a2/b', mw, mw, mw, (req, res) => res.send({ urls: req['urls'], params: req.params })) .use('/a3/:pat1/:pat2', echo) .use('/a4/:pat1/*', echo) @@ -113,22 +113,22 @@ describe('Request properties', () => { }) await fetch('/s/t/a1/b/c').expect(200, { - url: '/c', + url: '/a1/b/c', params: { pat: 't' } }) await fetch('/s/t/a2/b/c').expect(200, { - urls: ['/c', '/c', '/c'], + urls: ['/a2/b/c', '/a2/b/c', '/a2/b/c'], params: { pat: 't' } }) await fetch('/s/t/a3/b/c/d').expect(200, { - url: '/d', + url: '/b/c/d', params: { pat: 't', pat1: 'b', pat2: 'c' } }) await fetch('/s/t/a4/b/c/d').expect(200, { - url: '/', + url: '/c/d', params: { pat: 't', pat1: 'b', wild: 'c/d' } }) }) @@ -140,11 +140,11 @@ describe('Request properties', () => { const app = new App().use('/s1/*', subAppRoute).use('/s2/*', subAppMw) const fetch = makeFetch(app.listen()) await fetch('/s1/a/b/c/d').expect(200, { - url: '/', + url: '/s1/a/b/c/d', params: { wild: 'a/b/c/d' } }) await fetch('/s2/a/b/c/d').expect(200, { - url: '/', + url: '/s2/a/b/c/d', params: { wild: 'a/b/c/d' } }) }) From a264327180b51d93e6781ae716620cec25eac72a Mon Sep 17 00:00:00 2001 From: aarontravass Date: Mon, 15 Jan 2024 23:00:04 -0500 Subject: [PATCH 7/8] test: reverted tests and fixed some code --- packages/app/src/app.ts | 10 +++++++--- tests/core/request.test.ts | 17 +++++++++-------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/packages/app/src/app.ts b/packages/app/src/app.ts index b6715713..42865234 100644 --- a/packages/app/src/app.ts +++ b/packages/app/src/app.ts @@ -314,6 +314,8 @@ export class App const pathname = getPathname(req.url) + const reqUrlCopy = req.url + const matched = this.#find(pathname) const mw: Middleware[] = [ @@ -346,6 +348,8 @@ export class App const handle = (mw: Middleware) => async (req: Req, res: Res, next?: NextFunction) => { const { path, handler, regex } = mw + req.url = reqUrlCopy // reset req.url since it is changed in the handle fn + let params: URLParams try { @@ -357,8 +361,8 @@ export class App // Warning: users should not use :wild as a pattern let prefix = path - if (regex) { - for (const key of regex.keys as string[]) { + if (regex?.keys) { + for (const key of regex.keys) { if (key === 'wild') { prefix = prefix.replace('*', params.wild) } else { @@ -370,7 +374,7 @@ export class App req.params = { ...req.params, ...params } if (mw.type === 'mw') { - req.url = lead(req.originalUrl.substring(prefix.length)) + req.url = lead(req.url.substring(prefix.length)) } if (!req.path) req.path = getPathname(req.url) diff --git a/tests/core/request.test.ts b/tests/core/request.test.ts index ec5ced74..f31645a6 100644 --- a/tests/core/request.test.ts +++ b/tests/core/request.test.ts @@ -64,18 +64,19 @@ describe('Request properties', () => { }) await fetch('/s/t/u/a1/b/c').expect(200, { - url: '/s/t/u/a1/b/c', + url: '/a1/b/c', params: { pat1: 't', pat2: 'u', wild: 'c' } }) await fetch('/s/t/u/a2/b/c').expect(200, { - url: '/s/t/u/a2/b/c', + url: '/a2/b/c', params: { pat1: 't', pat2: 'u', pat: 'c' } }) }) it('should set the correct req.url on middlewares even in a subapp', async () => { const echo = (req, res) => res.send({ url: req.url, params: req.params }) const mw = (req, res, next) => { + console.log(req.url, req.originalUrl) req.urls ||= [] req.urls.push(req.url) next() @@ -113,22 +114,22 @@ describe('Request properties', () => { }) await fetch('/s/t/a1/b/c').expect(200, { - url: '/a1/b/c', + url: '/c', params: { pat: 't' } }) await fetch('/s/t/a2/b/c').expect(200, { - urls: ['/a2/b/c', '/a2/b/c', '/a2/b/c'], + urls: ['/c', '/c', '/c'], params: { pat: 't' } }) await fetch('/s/t/a3/b/c/d').expect(200, { - url: '/b/c/d', + url: '/d', params: { pat: 't', pat1: 'b', pat2: 'c' } }) await fetch('/s/t/a4/b/c/d').expect(200, { - url: '/c/d', + url: '/', params: { pat: 't', pat1: 'b', wild: 'c/d' } }) }) @@ -140,11 +141,11 @@ describe('Request properties', () => { const app = new App().use('/s1/*', subAppRoute).use('/s2/*', subAppMw) const fetch = makeFetch(app.listen()) await fetch('/s1/a/b/c/d').expect(200, { - url: '/s1/a/b/c/d', + url: '/', params: { wild: 'a/b/c/d' } }) await fetch('/s2/a/b/c/d').expect(200, { - url: '/s2/a/b/c/d', + url: '/', params: { wild: 'a/b/c/d' } }) }) From e34bc0f533cf3a45a779862e3bb20b6f3b6042c2 Mon Sep 17 00:00:00 2001 From: aarontravass Date: Thu, 25 Jan 2024 12:16:43 -0500 Subject: [PATCH 8/8] refactor: removed extra if in rustRemoteAddress and fixed tests --- packages/app/src/request.ts | 7 ++----- tests/core/request.test.ts | 4 +++- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/app/src/request.ts b/packages/app/src/request.ts index 0df29082..3d90cd8f 100644 --- a/packages/app/src/request.ts +++ b/packages/app/src/request.ts @@ -14,11 +14,8 @@ import type { TLSSocket } from 'node:tls' export { getURLParams } from '@tinyhttp/req' -const trustRemoteAddress = ({ socket }: Pick) => { - const val = socket.remoteAddress - if (typeof val !== 'string') return compile(val || []) - return compile(val.split(',').map((x) => x.trim())) -} +const trustRemoteAddress = ({ socket }: Pick) => + compile(socket.remoteAddress ? socket.remoteAddress.split(',').map((x) => x.trim()) : []) export const getProtocol = (req: Request): Protocol => { const proto = `http${req.secure ? 's' : ''}` diff --git a/tests/core/request.test.ts b/tests/core/request.test.ts index f31645a6..f7677063 100644 --- a/tests/core/request.test.ts +++ b/tests/core/request.test.ts @@ -220,7 +220,7 @@ describe('Request properties', () => { await fetch('/').expect(200, `subdomains: `) }) - describe('internal function tests', () => { + describe('`getSubdomains` function test', () => { it('should test `getSubdomains` function when host is null', async () => { const app = new App() app.get('/', (req, res) => { @@ -245,6 +245,8 @@ describe('Request properties', () => { }) await makeFetch(app.listen())('/').expectStatus(200) }) + }) + describe('`getProtocol` function tests', () => { it('should test `getProtocol` function', async () => { const app = new App() app.get('/', (req, res) => {