V5 fix cors headers reflection (#235)

* adds default headers reflection to cors() * code-golfing * simplification and code-golfing * final output
kwhitley · Apr 3, 2024 · 3645c6b · 3645c6b
1 parent 38d4448
commit 3645c6b
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 56 deletions.
diff --git 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
@@ -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')
+      })
     })
   })
 
@@ -189,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('*')
       })
 
@@ -225,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
@@ -29,17 +29,6 @@ export const cors = (options: CorsOptions = {}) => {
     maxAge,
   } = options
 
-  // create generic CORS headers
-  const corsHeaders: Record<string, any> = {
-    '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
-    '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
 
@@ -58,14 +47,26 @@ export const cors = (options: CorsOptions = {}) => {
     : origin
   }
 
+  const appendHeadersAndReturn = (response: Response, headers: Record<string, any>): Response => {
+    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),
-          ...corsHeaders,
-        }).filter(v => v[1]),
+      const response = new Response(null, { status: 204 })
+
+      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
+        'access-control-max-age': maxAge,
+        'access-control-allow-credentials': credentials,
       })
     } // otherwise ignore
   }
@@ -77,14 +78,13 @@ export const cors = (options: CorsOptions = {}) => {
       || response.status == 101
     ) return response
 
-    const origin = getAccessControlOrigin(request)
-    if (origin) response.headers.append('access-control-allow-origin', origin)
+    // clone the response
+    // response = response.clone()
 
-    for (const [key, value] of Object.entries(corsHeaders)) {
-      if (value) response.headers.append(key, value)
-    }
-
-    return response
+    return appendHeadersAndReturn(response, {
+      'access-control-allow-origin': getAccessControlOrigin(request),
+      'access-control-allow-credentials': credentials,
+    })
   }
 
   // Return corsify and preflight methods.