From 200158daa326e77480db1d13c4cb5e53d39c0b2c Mon Sep 17 00:00:00 2001 From: Kevin Whitley Date: Wed, 3 Apr 2024 12:13:17 -0500 Subject: [PATCH 1/4] adds default headers reflection to cors() --- examples/runtimes/node-autorouter.js | 33 ++-------------------------- src/cors.spec.ts | 9 ++++++++ src/cors.ts | 5 ++++- 3 files changed, 15 insertions(+), 32 deletions(-) diff --git a/examples/runtimes/node-autorouter.js b/examples/runtimes/node-autorouter.js index de7a315..5d0713f 100644 --- a/examples/runtimes/node-autorouter.js +++ b/examples/runtimes/node-autorouter.js @@ -1,41 +1,12 @@ -const { error } = require("../../dist/index") const { AutoRouter } = require("../../dist/AutoRouter") const { createServerAdapter } = require("@whatwg-node/server") const http = require("http") -const router = AutoRouter({ - catch: (err) => { - console.log('ERROR', err.message, err.stack) - - return error(500, err.stack) - } -}) +const router = AutoRouter() router.get('*', () => "Hello itty-router v5") -const serverAdapter = createServerAdapter(async (request) => { - // this works - - const url = new URL(request.url) - - console.log({ - url: request.url, - newURL: url, - method: request.method, - }) - - request.query = {} - request.params = {} - request.route = '/foo' - request.proxy = new Proxy(request, {}) - - const slimRequest = { - method: request.method, - url: request.url, - } - - return await router.fetch(request) -}) +const serverAdapter = createServerAdapter(router.fetch) const httpServer = http.createServer(serverAdapter) diff --git a/src/cors.spec.ts b/src/cors.spec.ts index f9e81cd..8eda13f 100644 --- a/src/cors.spec.ts +++ b/src/cors.spec.ts @@ -32,6 +32,9 @@ const REGEXP_DENY_ORIGIN = /^https:\/\/google.com$/ const BASIC_OPTIONS_REQUEST = toReq('OPTIONS /', { headers: { origin: TEST_ORIGIN }, }) +const REQUEST_HEADERS_REQUEST = toReq('OPTIONS /', { + headers: { 'access-control-request-headers': 'x-foo' }, +}) const BASIC_REQUEST = toReq('/', { headers: { origin: TEST_ORIGIN }, }) @@ -179,6 +182,12 @@ describe('cors(options?: CorsOptions)', () => { const response = await DEFAULT_ROUTER.fetch(BASIC_OPTIONS_REQUEST) expect(response.status).toBe(204) }) + + it('reflects requested headers by default', async () => { + const response = await DEFAULT_ROUTER.fetch(REQUEST_HEADERS_REQUEST) + expect(response.status).toBe(204) + expect(response.headers.get('access-control-allow-headers')).toBe('x-foo') + }) }) }) diff --git a/src/cors.ts b/src/cors.ts index 9f6dbe7..ab3b53b 100644 --- a/src/cors.ts +++ b/src/cors.ts @@ -31,7 +31,6 @@ export const cors = (options: CorsOptions = {}) => { // create generic CORS headers const corsHeaders: Record = { - 'access-control-allow-headers': allowHeaders?.join?.(',') ?? allowHeaders, // include allowed headers // @ts-expect-error 'access-control-expose-headers': exposeHeaders?.join?.(',') ?? exposeHeaders, // include allowed headers // @ts-expect-error @@ -64,6 +63,7 @@ export const cors = (options: CorsOptions = {}) => { status: 204, headers: Object.entries({ 'access-control-allow-origin': getAccessControlOrigin(request), + 'access-control-allow-headers': allowHeaders?.join?.(',') ?? allowHeaders ?? request.headers.get('access-control-request-headers'), // include allowed headers ...corsHeaders, }).filter(v => v[1]), }) @@ -77,6 +77,9 @@ export const cors = (options: CorsOptions = {}) => { || response.status == 101 ) return response + // clone the response + // response = response.clone() + const origin = getAccessControlOrigin(request) if (origin) response.headers.append('access-control-allow-origin', origin) From c94e33432f6576be7822335158c25905425fcc0b Mon Sep 17 00:00:00 2001 From: Kevin Whitley Date: Wed, 3 Apr 2024 12:32:32 -0500 Subject: [PATCH 2/4] code-golfing --- src/cors.ts | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/cors.ts b/src/cors.ts index ab3b53b..7181686 100644 --- a/src/cors.ts +++ b/src/cors.ts @@ -30,7 +30,7 @@ export const cors = (options: CorsOptions = {}) => { } = options // create generic CORS headers - const corsHeaders: Record = { + const corsHeaders = { // @ts-expect-error 'access-control-expose-headers': exposeHeaders?.join?.(',') ?? exposeHeaders, // include allowed headers // @ts-expect-error @@ -57,15 +57,21 @@ export const cors = (options: CorsOptions = {}) => { : origin } + const appendHeadersAndReturn = (response: Response, headers: Record) => { + for (const [key, value] of Object.entries(headers)) { + if (value) response.headers.append(key, value) + } + return response + } + const preflight = (request: Request) => { if (request.method == 'OPTIONS') { - return new Response(null, { - status: 204, - headers: Object.entries({ - 'access-control-allow-origin': getAccessControlOrigin(request), - 'access-control-allow-headers': allowHeaders?.join?.(',') ?? allowHeaders ?? request.headers.get('access-control-request-headers'), // include allowed headers - ...corsHeaders, - }).filter(v => v[1]), + const response = new Response(null, { status: 204 }) + + return appendHeadersAndReturn(response, { + 'access-control-allow-origin': getAccessControlOrigin(request), + 'access-control-allow-headers': allowHeaders?.join?.(',') ?? allowHeaders ?? request.headers.get('access-control-request-headers'), // include allowed headers + ...corsHeaders, }) } // otherwise ignore } @@ -80,14 +86,16 @@ export const cors = (options: CorsOptions = {}) => { // clone the response // response = response.clone() - const origin = getAccessControlOrigin(request) - if (origin) response.headers.append('access-control-allow-origin', origin) + // const origin = getAccessControlOrigin(request) + // if (origin) response.headers.append('access-control-allow-origin', origin) - for (const [key, value] of Object.entries(corsHeaders)) { - if (value) response.headers.append(key, value) - } - - return response + // for (const [key, value] of Object.entries(corsHeaders)) { + // if (value) response.headers.append(key, value) + // } + return appendHeadersAndReturn(response, { + 'access-control-allow-origin': getAccessControlOrigin(request), + ...corsHeaders + }) } // Return corsify and preflight methods. From 6e27a87c9d67b09640cb706ae70e57bff68ff649 Mon Sep 17 00:00:00 2001 From: Kevin Whitley Date: Wed, 3 Apr 2024 12:36:38 -0500 Subject: [PATCH 3/4] simplification and code-golfing --- src/cors.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/cors.ts b/src/cors.ts index 7181686..479078b 100644 --- a/src/cors.ts +++ b/src/cors.ts @@ -57,7 +57,7 @@ export const cors = (options: CorsOptions = {}) => { : origin } - const appendHeadersAndReturn = (response: Response, headers: Record) => { + const appendHeadersAndReturn = (response: Response, headers: Record): Response => { for (const [key, value] of Object.entries(headers)) { if (value) response.headers.append(key, value) } @@ -86,12 +86,6 @@ export const cors = (options: CorsOptions = {}) => { // clone the response // response = response.clone() - // const origin = getAccessControlOrigin(request) - // if (origin) response.headers.append('access-control-allow-origin', origin) - - // for (const [key, value] of Object.entries(corsHeaders)) { - // if (value) response.headers.append(key, value) - // } return appendHeadersAndReturn(response, { 'access-control-allow-origin': getAccessControlOrigin(request), ...corsHeaders From 17091f6377fc394199818bd9d9bf0074aaf07819 Mon Sep 17 00:00:00 2001 From: Kevin Whitley Date: Wed, 3 Apr 2024 12:46:20 -0500 Subject: [PATCH 4/4] final output --- src/cors.spec.ts | 16 +++++++++++++++- src/cors.ts | 19 +++++++------------ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/cors.spec.ts b/src/cors.spec.ts index 8eda13f..934884c 100644 --- a/src/cors.spec.ts +++ b/src/cors.spec.ts @@ -198,7 +198,6 @@ describe('cors(options?: CorsOptions)', () => { const response = corsify(new Response(null)) const response2 = corsify(new Response(null), BASIC_REQUEST) expect(response.headers.get('access-control-allow-origin')).toBe('*') - expect(response.headers.get('access-control-allow-methods')).toBe('*') expect(response2.headers.get('access-control-allow-origin')).toBe('*') }) @@ -234,6 +233,21 @@ describe('cors(options?: CorsOptions)', () => { expect(response2.headers.get('access-control-allow-origin')).toBe(TEST_ORIGIN) }) + it('will not NOT include preflight headers', async () => { + const { corsify } = cors({ + allowHeaders: 'foo', + allowMethods: 'GET', + exposeHeaders: 'foo', + maxAge: 3600, + }) + const corsified = corsify(new Response(null)) + + expect(corsified.headers.get('access-control-allow-methods')).toBeNull() + expect(corsified.headers.get('access-control-allow-headers')).toBeNull() + expect(corsified.headers.get('access-control-expose-headers')).toBeNull() + expect(corsified.headers.get('access-control-max-age')).toBeNull() + }) + it('will safely preserve multiple cookies (or other identical header names)', async () => { const { corsify } = cors() const response = new Response(null) diff --git a/src/cors.ts b/src/cors.ts index 479078b..4559562 100644 --- a/src/cors.ts +++ b/src/cors.ts @@ -29,16 +29,6 @@ export const cors = (options: CorsOptions = {}) => { maxAge, } = options - // create generic CORS headers - const corsHeaders = { - // @ts-expect-error - 'access-control-expose-headers': exposeHeaders?.join?.(',') ?? exposeHeaders, // include allowed headers - // @ts-expect-error - 'access-control-allow-methods': allowMethods?.join?.(',') ?? allowMethods, // include allowed methods - 'access-control-max-age': maxAge, - 'access-control-allow-credentials': credentials, - } - const getAccessControlOrigin = (request?: Request): string => { const requestOrigin = request?.headers.get('origin') // may be null if no request passed @@ -70,8 +60,13 @@ export const cors = (options: CorsOptions = {}) => { return appendHeadersAndReturn(response, { 'access-control-allow-origin': getAccessControlOrigin(request), + // @ts-ignore + 'access-control-allow-methods': allowMethods?.join?.(',') ?? allowMethods, // include allowed methods + // @ts-ignore + 'access-control-expose-headers': exposeHeaders?.join?.(',') ?? exposeHeaders, // include allowed headers 'access-control-allow-headers': allowHeaders?.join?.(',') ?? allowHeaders ?? request.headers.get('access-control-request-headers'), // include allowed headers - ...corsHeaders, + 'access-control-max-age': maxAge, + 'access-control-allow-credentials': credentials, }) } // otherwise ignore } @@ -88,7 +83,7 @@ export const cors = (options: CorsOptions = {}) => { return appendHeadersAndReturn(response, { 'access-control-allow-origin': getAccessControlOrigin(request), - ...corsHeaders + 'access-control-allow-credentials': credentials, }) }