From 841b5158ed22205b065da6d19375acdb3a374aee Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Sat, 22 Jun 2024 13:30:47 +0100 Subject: [PATCH 01/14] refactor: create test group for when origin is a `string[]` --- tests/index.test.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/index.test.ts b/tests/index.test.ts index 0a888c1..418fb17 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -50,15 +50,17 @@ describe('CORS headers tests', (it) => { 'http://example.com' ) }) - it('should set origin if it is an array', async () => { - const app = createServer(cors({ origin: ['http://example.com', 'example.com', 'https://example.com'] })) + describe('when origin is an array of strings', (it) => { + it('should set origin when origin header is included in request and whitelisted', async () => { + const app = createServer(cors({ origin: ['http://example.com', 'example.com', 'https://example.com'] })) - const fetch = makeFetch(app) + const fetch = makeFetch(app) - await fetch('/', { headers: { Origin: 'http://example.com' } }).expect( - 'Access-Control-Allow-Origin', - 'http://example.com' - ) + await fetch('/', { headers: { Origin: 'http://example.com' } }).expect( + 'Access-Control-Allow-Origin', + 'http://example.com' + ) + }) }) it('should send an error if it is other object types', () => { try { From d3795b6bcebcf8175c97dbedf7e461ccf05e2365 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Sat, 22 Jun 2024 14:19:20 +0100 Subject: [PATCH 02/14] refactor: `Access-Control-Allow-Origin` handler Type-check the `origin` parameter to `cors` when `cors` is invoked, rather than when requests are handled by its returned middleware. --- src/index.ts | 74 ++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 57 insertions(+), 17 deletions(-) diff --git a/src/index.ts b/src/index.ts index a13e8fe..caae44e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,6 +12,59 @@ export interface AccessControlOptions { preflightContinue?: boolean } +function getOriginHeaderHandler(origin: unknown): (req: Request, res: Response) => void { + function fail() { + throw new TypeError('No other objects allowed. Allowed types is array of strings or RegExp') + } + + if (typeof origin === 'boolean' && origin === true) { + return function (_, res) { + res.setHeader('Access-Control-Allow-Origin', '*') + } + } + + if (typeof origin === 'string') { + return function (_, res) { + res.setHeader('Access-Control-Allow-Origin', origin) + } + } + + if (typeof origin === 'function') { + return function (req, res) { + vary(res, 'Origin') + res.setHeader('Access-Control-Allow-Origin', origin(req, res)) + } + } + + if (typeof origin !== 'object') fail() + + if (Array.isArray(origin) && origin.indexOf('*') !== -1) { + return function (_, res) { + res.setHeader('Access-Control-Allow-Origin', '*') + } + } + + if (Array.isArray(origin)) { + return function (req, res) { + vary(res, 'Origin') + if (req.headers.origin === undefined) return + if (origin.indexOf(req.headers.origin) === -1) return + res.setHeader('Access-Control-Allow-Origin', req.headers.origin) + } + } + + if (origin instanceof RegExp) { + return function (req, res) { + vary(res, 'Origin') + if (req.headers.origin === undefined) return + if (!origin.test(req.headers.origin)) return + res.setHeader('Access-Control-Allow-Origin', req.headers.origin) + } + } + + fail() +} + /** * CORS Middleware */ @@ -26,24 +79,11 @@ export const cors = (opts: AccessControlOptions = {}) => { optionsSuccessStatus = 204, preflightContinue = false } = opts + const originHeaderHandler = getOriginHeaderHandler(origin) + return (req: Request, res: Response, next?: () => void) => { - // Checking the type of the origin property - if (typeof origin === 'boolean' && origin === true) { - res.setHeader('Access-Control-Allow-Origin', '*') - } else if (typeof origin === 'string') { - res.setHeader('Access-Control-Allow-Origin', origin) - } else if (typeof origin === 'function') { - res.setHeader('Access-Control-Allow-Origin', origin(req, res)) - } else if (typeof origin === 'object') { - if (Array.isArray(origin) && (origin.indexOf(req.headers.origin) !== -1 || origin.indexOf('*') !== -1)) { - res.setHeader('Access-Control-Allow-Origin', req.headers.origin) - } else if (origin instanceof RegExp && origin.test(req.headers.origin)) { - res.setHeader('Access-Control-Allow-Origin', req.headers.origin) - } else { - throw new TypeError('No other objects allowed. Allowed types is array of strings or RegExp') - } - } - if ((typeof origin === 'string' && origin !== '*') || typeof origin === 'function') vary(res, 'Origin') + // Setting the Access-Control-Allow-Origin header + originHeaderHandler(req, res) // Setting the Access-Control-Allow-Methods header from the methods array res.setHeader('Access-Control-Allow-Methods', methods.join(', ').toUpperCase()) From 04e93f0de607069555c5d188f8c68f3d41e7b2c6 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Sat, 22 Jun 2024 14:20:06 +0100 Subject: [PATCH 03/14] test: add tests related to #4 --- tests/index.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/index.test.ts b/tests/index.test.ts index 418fb17..efc0fea 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -61,6 +61,20 @@ describe('CORS headers tests', (it) => { 'http://example.com' ) }) + it('should not set origin when origin header is included in request but not whitelisted', async () => { + const app = createServer(cors({ origin: ['http://example.com', 'example.com', 'https://example.com'] })) + + const fetch = makeFetch(app) + + await fetch('/', { headers: { Origin: 'http://not-example.com' } }).expect('Access-Control-Allow-Origin', null) + }) + it('should not set origin when origin header is excluded from request', async () => { + const app = createServer(cors({ origin: ['http://example.com', 'example.com', 'https://example.com'] })) + + const fetch = makeFetch(app) + + await fetch('/').expect('Access-Control-Allow-Origin', null) + }) }) it('should send an error if it is other object types', () => { try { From 393aab1dd4db45ed6dfc34e79b910802b36aca50 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Sat, 22 Jun 2024 14:22:16 +0100 Subject: [PATCH 04/14] test: require `cors` to throw, not returned middleware --- tests/index.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/index.test.ts b/tests/index.test.ts index efc0fea..ad0ef2e 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -79,7 +79,7 @@ describe('CORS headers tests', (it) => { it('should send an error if it is other object types', () => { try { // @ts-ignore - const app = createServer(cors({ origin: { site: 'http://example.com' } })) + const middleware = cors({ origin: { site: 'http://example.com' } }) } catch (e) { assert.strictEqual(e.message, 'No other objects allowed. Allowed types is array of strings or RegExp') } From c8c9e2edea3742c528390cef5b61930e2b387039 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Sat, 22 Jun 2024 16:03:48 +0100 Subject: [PATCH 05/14] fix: setting `origin` to `false` would cause TypeError --- src/index.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/index.ts b/src/index.ts index caae44e..e055b70 100644 --- a/src/index.ts +++ b/src/index.ts @@ -23,6 +23,10 @@ function getOriginHeaderHandler(origin: unknown): (req: Request, res: Response) } } + if (typeof origin === 'boolean' && origin === false) { + return () => undefined + } + if (typeof origin === 'string') { return function (_, res) { res.setHeader('Access-Control-Allow-Origin', origin) From de7e117fcdb2908f333ac1e0745658b6d9a15115 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Sun, 23 Jun 2024 02:05:47 +0100 Subject: [PATCH 06/14] refactor: handle any iterable type and use `Set` to improve lookup performance --- src/index.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/index.ts b/src/index.ts index e055b70..3983a1f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,6 +12,10 @@ export interface AccessControlOptions { preflightContinue?: boolean } +function isIterable(obj: unknown): obj is Iterable { + return typeof obj[Symbol.iterator] === 'function' +} + function getOriginHeaderHandler(origin: unknown): (req: Request, res: Response) => void { function fail() { throw new TypeError('No other objects allowed. Allowed types is array of strings or RegExp') @@ -42,17 +46,19 @@ function getOriginHeaderHandler(origin: unknown): (req: Request, res: Response) if (typeof origin !== 'object') fail() - if (Array.isArray(origin) && origin.indexOf('*') !== -1) { - return function (_, res) { - res.setHeader('Access-Control-Allow-Origin', '*') + if (isIterable(origin)) { + const originSet = new Set(origin) + + if (originSet.has('*')) { + return function (_, res) { + res.setHeader('Access-Control-Allow-Origin', '*') + } } - } - if (Array.isArray(origin)) { return function (req, res) { vary(res, 'Origin') if (req.headers.origin === undefined) return - if (origin.indexOf(req.headers.origin) === -1) return + if (!originSet.has(req.headers.origin)) return res.setHeader('Access-Control-Allow-Origin', req.headers.origin) } } From b120e3feec143506808f0e9479ee29e6e263c419 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Sun, 23 Jun 2024 02:06:39 +0100 Subject: [PATCH 07/14] fix: throw TypeError when iterable is not homogeneous string collection --- src/index.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/index.ts b/src/index.ts index 3983a1f..3b7ae48 100644 --- a/src/index.ts +++ b/src/index.ts @@ -47,6 +47,9 @@ function getOriginHeaderHandler(origin: unknown): (req: Request, res: Response) if (typeof origin !== 'object') fail() if (isIterable(origin)) { + const originArray = Array.from(origin) + if (originArray.some((element) => typeof element !== 'string')) fail() + const originSet = new Set(origin) if (originSet.has('*')) { From eb547d441dc1726cf5ffbcec600aa1f0497bb974 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Sun, 23 Jun 2024 02:09:29 +0100 Subject: [PATCH 08/14] refactor: widen allowed input type --- src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 3b7ae48..920daf5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -2,7 +2,7 @@ import { IncomingMessage as Request, ServerResponse as Response } from 'http' import { vary } from 'es-vary' export interface AccessControlOptions { - origin?: string | boolean | ((req: Request, res: Response) => string) | Array | RegExp + origin?: string | boolean | ((req: Request, res: Response) => string) | Iterable | RegExp methods?: string[] allowedHeaders?: string[] exposedHeaders?: string[] From 9bf69d71ff4b8ada90246e7325db93545f883be3 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Sat, 22 Jun 2024 16:51:11 +0100 Subject: [PATCH 09/14] fix: use login shell to run `pre-commit` the JetBrains git integration uses a non-login non-interactive shell to execute Git commands, which means `pnpm` cannot be found the `-l` option means a login shell is used, so `~/.bashrc` is used as a startup file, which adds `$PNPM_HOME` to `$PATH` --- .husky/pre-commit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.husky/pre-commit b/.husky/pre-commit index 367adc4..35257ce 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -1,4 +1,4 @@ -#!/bin/sh +#!/bin/sh -l . "$(dirname "$0")/_/husky.sh" pnpm format && pnpm lint && pnpm build && pnpm test \ No newline at end of file From b2789defaa585ba52fff9ef219c4b81e6efac1ab Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Sun, 23 Jun 2024 02:13:48 +0100 Subject: [PATCH 10/14] test: add test for `origin` being an iterable containing non-string value --- tests/index.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/index.test.ts b/tests/index.test.ts index ad0ef2e..aa1f671 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -76,6 +76,14 @@ describe('CORS headers tests', (it) => { await fetch('/').expect('Access-Control-Allow-Origin', null) }) }) + it('should send an error if origin is an iterable containing a non-string', async () => { + try { + // @ts-ignore + const middleware = cors({ origin: [{}, 3, 'abc'] }) + } catch (e) { + assert.strictEqual(e.message, 'No other objects allowed. Allowed types is array of strings or RegExp') + } + }) it('should send an error if it is other object types', () => { try { // @ts-ignore From 24e99e885eeb7e77b91ba4b5dee46320d6aaeb78 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Sun, 30 Jun 2024 13:06:42 +0100 Subject: [PATCH 11/14] chore: convert `function` keyword to arrow functions --- src/index.ts | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/index.ts b/src/index.ts index 920daf5..b36e04e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,17 +12,15 @@ export interface AccessControlOptions { preflightContinue?: boolean } -function isIterable(obj: unknown): obj is Iterable { - return typeof obj[Symbol.iterator] === 'function' -} +const isIterable = (obj: unknown): obj is Iterable => typeof obj[Symbol.iterator] === 'function' -function getOriginHeaderHandler(origin: unknown): (req: Request, res: Response) => void { - function fail() { +const getOriginHeaderHandler = (origin: unknown): ((req: Request, res: Response) => void) => { + const fail = () => { throw new TypeError('No other objects allowed. Allowed types is array of strings or RegExp') } if (typeof origin === 'boolean' && origin === true) { - return function (_, res) { + return (_, res) => { res.setHeader('Access-Control-Allow-Origin', '*') } } @@ -32,13 +30,13 @@ function getOriginHeaderHandler(origin: unknown): (req: Request, res: Response) } if (typeof origin === 'string') { - return function (_, res) { + return (_, res) => { res.setHeader('Access-Control-Allow-Origin', origin) } } if (typeof origin === 'function') { - return function (req, res) { + return (req, res) => { vary(res, 'Origin') res.setHeader('Access-Control-Allow-Origin', origin(req, res)) } @@ -53,12 +51,12 @@ function getOriginHeaderHandler(origin: unknown): (req: Request, res: Response) const originSet = new Set(origin) if (originSet.has('*')) { - return function (_, res) { + return (_, res) => { res.setHeader('Access-Control-Allow-Origin', '*') } } - return function (req, res) { + return (req, res) => { vary(res, 'Origin') if (req.headers.origin === undefined) return if (!originSet.has(req.headers.origin)) return @@ -67,7 +65,7 @@ function getOriginHeaderHandler(origin: unknown): (req: Request, res: Response) } if (origin instanceof RegExp) { - return function (req, res) { + return (req, res) => { vary(res, 'Origin') if (req.headers.origin === undefined) return if (!origin.test(req.headers.origin)) return From b78dae2fa0d2f2d15e4fa934a8a1c81c713ff8b6 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Sun, 30 Jun 2024 13:14:42 +0100 Subject: [PATCH 12/14] chore: replace separate `if` branch with ternary expression --- src/index.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/index.ts b/src/index.ts index b36e04e..fabcaa7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -19,14 +19,12 @@ const getOriginHeaderHandler = (origin: unknown): ((req: Request, res: Response) throw new TypeError('No other objects allowed. Allowed types is array of strings or RegExp') } - if (typeof origin === 'boolean' && origin === true) { - return (_, res) => { - res.setHeader('Access-Control-Allow-Origin', '*') - } - } - - if (typeof origin === 'boolean' && origin === false) { - return () => undefined + if (typeof origin === 'boolean') { + return origin + ? (_, res) => { + res.setHeader('Access-Control-Allow-Origin', '*') + } + : () => undefined } if (typeof origin === 'string') { From 7ca7d793bdbbb2cb75ed88015ab932a5b52323b8 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Sun, 30 Jun 2024 13:16:18 +0100 Subject: [PATCH 13/14] chore: remove comment explained by function name --- src/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index fabcaa7..2575240 100644 --- a/src/index.ts +++ b/src/index.ts @@ -91,7 +91,6 @@ export const cors = (opts: AccessControlOptions = {}) => { const originHeaderHandler = getOriginHeaderHandler(origin) return (req: Request, res: Response, next?: () => void) => { - // Setting the Access-Control-Allow-Origin header originHeaderHandler(req, res) // Setting the Access-Control-Allow-Methods header from the methods array From 78645fcdf7149a1f29519d07c91b3f894ea4d705 Mon Sep 17 00:00:00 2001 From: Lordfirespeed <28568841+Lordfirespeed@users.noreply.github.com> Date: Fri, 12 Jul 2024 14:22:30 +0100 Subject: [PATCH 14/14] refactor: move `fail` to outer scope, rename to `failOriginParam` --- src/index.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/index.ts b/src/index.ts index 2575240..4511206 100644 --- a/src/index.ts +++ b/src/index.ts @@ -14,11 +14,11 @@ export interface AccessControlOptions { const isIterable = (obj: unknown): obj is Iterable => typeof obj[Symbol.iterator] === 'function' -const getOriginHeaderHandler = (origin: unknown): ((req: Request, res: Response) => void) => { - const fail = () => { - throw new TypeError('No other objects allowed. Allowed types is array of strings or RegExp') - } +const failOriginParam = () => { + throw new TypeError('No other objects allowed. Allowed types is array of strings or RegExp') +} +const getOriginHeaderHandler = (origin: unknown): ((req: Request, res: Response) => void) => { if (typeof origin === 'boolean') { return origin ? (_, res) => { @@ -40,11 +40,11 @@ const getOriginHeaderHandler = (origin: unknown): ((req: Request, res: Response) } } - if (typeof origin !== 'object') fail() + if (typeof origin !== 'object') failOriginParam() if (isIterable(origin)) { const originArray = Array.from(origin) - if (originArray.some((element) => typeof element !== 'string')) fail() + if (originArray.some((element) => typeof element !== 'string')) failOriginParam() const originSet = new Set(origin) @@ -71,7 +71,7 @@ const getOriginHeaderHandler = (origin: unknown): ((req: Request, res: Response) } } - fail() + failOriginParam() } /**