Repository: RobinTail/express-zod-api
Pull request #48 (2021-06-15 20:57:35)

Fix possible CORS issue with OPTIONS pre-flight request.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 739d56a9..006b585f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,14 @@
 
 ## Version 1
 
+### v1.3.1
+
+- Improving the coverage I found a bug and fixed it.
+  In some cases there was an issue with CORS requests: [preflight OPTIONS requests](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#preflighted_requests).
+  Despite the enabled configuration option `cors: true` the OPTIONS requests have not been handled properly.
+  This was leading to the 404 error with a message "Can not OPTIONS \<route\>".
+  The issue has been fixed and covered by multiple tests.
+
 ### v1.3.0
 
 - Zod version is 3.2.0.
diff --git a/coverage.svg b/coverage.svg
index 76ab83bc..57a4c264 100644
--- a/coverage.svg
+++ b/coverage.svg
@@ -1,1 +1,1 @@
-<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="116" height="20" role="img" aria-label="Coverage: 98.49%"><title>Coverage: 98.49%</title><linearGradient id="s" x2="0" y2="100%"><stop offset="0" stop-color="#bbb" stop-opacity=".1"/><stop offset="1" stop-opacity=".1"/></linearGradient><clipPath id="r"><rect width="116" height="20" rx="3" fill="#fff"/></clipPath><g clip-path="url(#r)"><rect width="63" height="20" fill="#555"/><rect x="63" width="53" height="20" fill="#4c1"/><rect width="116" height="20" fill="url(#s)"/></g><g fill="#fff" text-anchor="middle" font-family="Verdana,Geneva,DejaVu Sans,sans-serif" text-rendering="geometricPrecision" font-size="110"><text aria-hidden="true" x="325" y="150" fill="#010101" fill-opacity=".3" transform="scale(.1)" textLength="530">Coverage</text><text x="325" y="140" transform="scale(.1)" fill="#fff" textLength="530">Coverage</text><text aria-hidden="true" x="885" y="150" fill="#010101" fill-opacity=".3" transform="scale(.1)" textLength="430">98.49%</text><text x="885" y="140" transform="scale(.1)" fill="#fff" textLength="430">98.49%</text></g></svg>
\ No newline at end of file
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="116" height="20" role="img" aria-label="Coverage: 98.53%"><title>Coverage: 98.53%</title><linearGradient id="s" x2="0" y2="100%"><stop offset="0" stop-color="#bbb" stop-opacity=".1"/><stop offset="1" stop-opacity=".1"/></linearGradient><clipPath id="r"><rect width="116" height="20" rx="3" fill="#fff"/></clipPath><g clip-path="url(#r)"><rect width="63" height="20" fill="#555"/><rect x="63" width="53" height="20" fill="#4c1"/><rect width="116" height="20" fill="url(#s)"/></g><g fill="#fff" text-anchor="middle" font-family="Verdana,Geneva,DejaVu Sans,sans-serif" text-rendering="geometricPrecision" font-size="110"><text aria-hidden="true" x="325" y="150" fill="#010101" fill-opacity=".3" transform="scale(.1)" textLength="530">Coverage</text><text x="325" y="140" transform="scale(.1)" fill="#fff" textLength="530">Coverage</text><text aria-hidden="true" x="885" y="150" fill="#010101" fill-opacity=".3" transform="scale(.1)" textLength="430">98.53%</text><text x="885" y="140" transform="scale(.1)" fill="#fff" textLength="430">98.53%</text></g></svg>
\ No newline at end of file
diff --git a/src/method.ts b/src/method.ts
index bd80f49d..ff0f1ac8 100644
--- a/src/method.ts
+++ b/src/method.ts
@@ -1,4 +1,5 @@
 export type Method = 'get' | 'post' | 'put' | 'delete' | 'patch';
+export type AuxMethod = 'options';
 
 export type MethodsDefinition<M extends Method> = {
   methods: M[];
diff --git a/src/open-api.ts b/src/open-api.ts
index 6d401dae..3c1df6f5 100644
--- a/src/open-api.ts
+++ b/src/open-api.ts
@@ -7,7 +7,7 @@ import {ReferenceObject} from 'openapi3-ts/src/model/OpenApi';
 import {z} from 'zod';
 import {OpenAPIError} from './errors';
 import {extractObjectSchema} from './helpers';
-import {Routing, routingCycle} from './routing';
+import {Routing, routingCycle, RoutingCycleParams} from './routing';
 import {lookup} from 'mime';
 
 const describeSchema = (value: z.ZodTypeAny, isResponse: boolean): SchemaObject => {
@@ -170,7 +170,7 @@ export class OpenAPI {
     this.builder = new OpenApiBuilder()
       .addInfo({title, version})
       .addServer({url: serverUrl});
-    routingCycle(routing, (endpoint, fullPath, method) => {
+    const cb: RoutingCycleParams['cb'] = (endpoint, fullPath, method) => {
       const responseSchemaRef = this.createRef('responseSchema');
       this.builder.addSchema(responseSchemaRef.name, {
         ...describeSchema(endpoint.getOutputSchema(), true),
@@ -225,6 +225,7 @@ export class OpenAPI {
         ...(this.builder.rootDoc.paths?.[fullPath] || {}),
         [method]: operation,
       });
-    });
+    };
+    routingCycle({routing, cb});
   }
 }
diff --git a/src/routing.ts b/src/routing.ts
index 2e167157..33227162 100644
--- a/src/routing.ts
+++ b/src/routing.ts
@@ -3,7 +3,7 @@ import {Logger} from 'winston';
 import {ConfigType} from './config-type';
 import {AbstractEndpoint, Endpoint} from './endpoint';
 import {DependsOnMethodError, RoutingError} from './errors';
-import {Method} from './method';
+import {AuxMethod, Method} from './method';
 
 export class DependsOnMethod {
   constructor(public readonly methods: {
@@ -40,9 +40,14 @@ export interface Routing {
   [PATH: string]: Routing | DependsOnMethod | AbstractEndpoint;
 }
 
-type RoutingCycleCallback = (endpoint: AbstractEndpoint, fullPath: string, method: Method) => void;
+export interface RoutingCycleParams {
+  routing: Routing;
+  cb: (endpoint: AbstractEndpoint, fullPath: string, method: Method | AuxMethod) => void;
+  parentPath?: string;
+  cors?: boolean;
+}
 
-export const routingCycle = (routing: Routing, cb: RoutingCycleCallback, parentPath?: string) => {
+export const routingCycle = ({routing, cb, parentPath, cors}: RoutingCycleParams) => {
   Object.entries(routing).forEach(([path, element]) => {
     path = path.trim();
     if (path.match(/\//)) {
@@ -53,15 +58,27 @@ export const routingCycle = (routing: Routing, cb: RoutingCycleCallback, parentP
     }
     const fullPath = `${parentPath || ''}${path ? `/${path}` : ''}`;
     if (element instanceof AbstractEndpoint) {
-      element.getMethods().forEach((method) => {
+      const methods: (Method | AuxMethod)[] = element.getMethods();
+      if (cors) {
+        methods.push('options');
+      }
+      methods.forEach((method) => {
         cb(element, fullPath, method);
       });
     } else if (element instanceof DependsOnMethod) {
       Object.entries<AbstractEndpoint>(element.methods).forEach(([method, endpoint]) => {
         cb(endpoint, fullPath, method as Method);
       });
+      if (cors && Object.keys(element.methods).length > 0) {
+        const firstEndpoint = Object.values(element.methods)[0] as AbstractEndpoint;
+        cb(firstEndpoint, fullPath, 'options');
+      }
     } else {
-      routingCycle(element, cb, fullPath);
+      routingCycle({
+        routing: element,
+        cb, cors,
+        parentPath: fullPath
+      });
     }
   });
 };
@@ -72,10 +89,14 @@ export const initRouting = ({app, logger, config, routing}: {
   config: ConfigType,
   routing: Routing
 }) => {
-  routingCycle(routing, (endpoint, fullPath, method) => {
-    app[method](fullPath, async (request, response) => {
-      logger.info(`${request.method}: ${fullPath}`);
-      await endpoint.execute({request, response, logger, config});
-    });
+  routingCycle({
+    routing,
+    cors: config.cors,
+    cb: (endpoint, fullPath, method) => {
+      app[method](fullPath, async (request, response) => {
+        logger.info(`${request.method}: ${fullPath}`);
+        await endpoint.execute({request, response, logger, config});
+      });
+    }
   });
 };


Test patch:
diff --git a/tests/system/example.spec.ts b/tests/system/example.spec.ts
index 2e4198de..9b5a1f38 100644
--- a/tests/system/example.spec.ts
+++ b/tests/system/example.spec.ts
@@ -42,6 +42,22 @@ describe('Example', () => {
       })).toBeTruthy();
     });
 
+    test('Should handle OPTIONS request', async () => {
+      const response = await fetch('http://localhost:8090/v1/setUser?test=123&id=50', {
+        method: 'OPTIONS',
+      });
+      expect(response.status).toBe(200);
+      const text = await response.text();
+      expect(text).toBe('');
+      expect(response.headers).toBeTruthy();
+      expect(response.headers.has('access-control-allow-origin')).toBeTruthy();
+      expect(response.headers.has('access-control-allow-methods')).toBeTruthy();
+      expect(response.headers.has('access-control-allow-headers')).toBeTruthy();
+      expect(response.headers.get('access-control-allow-origin')).toBe('*');
+      expect(response.headers.get('access-control-allow-methods')).toBe('POST, OPTIONS');
+      expect(response.headers.get('access-control-allow-headers')).toBe('content-type');
+    });
+
     test('Should handle valid POST request', async () => {
       const response = await fetch('http://localhost:8090/v1/setUser?test=123&id=50', {
         method: 'POST',
diff --git a/tests/unit/routing.spec.ts b/tests/unit/routing.spec.ts
index bd45c63b..4355b715 100644
--- a/tests/unit/routing.spec.ts
+++ b/tests/unit/routing.spec.ts
@@ -14,7 +14,8 @@ describe('Routing', () => {
         post: jest.fn(),
         put: jest.fn(),
         delete: jest.fn(),
-        patch: jest.fn()
+        patch: jest.fn(),
+        options: jest.fn()
       };
 
       loggerMock = {
@@ -27,7 +28,9 @@ describe('Routing', () => {
 
     test('Should set right methods', () => {
       const handlerMock = jest.fn();
-      const configMock = {};
+      const configMock = {
+        cors: true
+      };
       const getEndpoint = new EndpointsFactory().build({
         methods: ['get'],
         input: z.object({}).nonstrict(),
@@ -66,15 +69,21 @@ describe('Routing', () => {
       expect(appMock.put).toBeCalledTimes(0);
       expect(appMock.delete).toBeCalledTimes(0);
       expect(appMock.patch).toBeCalledTimes(0);
+      expect(appMock.options).toBeCalledTimes(3);
       expect(appMock.get.mock.calls[0][0]).toBe('/v1/user/get');
       expect(appMock.get.mock.calls[1][0]).toBe('/v1/user/universal');
       expect(appMock.post.mock.calls[0][0]).toBe('/v1/user/set');
       expect(appMock.post.mock.calls[1][0]).toBe('/v1/user/universal');
+      expect(appMock.options.mock.calls[0][0]).toBe('/v1/user/get');
+      expect(appMock.options.mock.calls[1][0]).toBe('/v1/user/set');
+      expect(appMock.options.mock.calls[2][0]).toBe('/v1/user/universal');
     });
 
     test('Should accept DependsOnMethod', () => {
       const handlerMock = jest.fn();
-      const configMock = {};
+      const configMock = {
+        cors: true
+      };
       const getEndpoint = new EndpointsFactory().build({
         methods: ['get'],
         input: z.object({}).nonstrict(),
@@ -114,10 +123,12 @@ describe('Routing', () => {
       expect(appMock.put).toBeCalledTimes(1);
       expect(appMock.patch).toBeCalledTimes(1);
       expect(appMock.delete).toBeCalledTimes(0);
+      expect(appMock.options).toBeCalledTimes(1);
       expect(appMock.get.mock.calls[0][0]).toBe('/v1/user');
       expect(appMock.post.mock.calls[0][0]).toBe('/v1/user');
       expect(appMock.put.mock.calls[0][0]).toBe('/v1/user');
       expect(appMock.patch.mock.calls[0][0]).toBe('/v1/user');
+      expect(appMock.options.mock.calls[0][0]).toBe('/v1/user');
     });
 
     test('Should accept parameters', () => {
diff --git a/tests/unit/server.spec.ts b/tests/unit/server.spec.ts
index 3ec6cba4..84ea9f37 100644
--- a/tests/unit/server.spec.ts
+++ b/tests/unit/server.spec.ts
@@ -6,7 +6,8 @@ const newAppMock = () => ({
   use: jest.fn(),
   listen: jest.fn((port, cb) => {cb && cb(); return new http.Server(); }),
   get: jest.fn(),
-  post: jest.fn()
+  post: jest.fn(),
+  options: jest.fn()
 });
 
 const expressMock = jest.mock('express', () => {
@@ -67,6 +68,8 @@ describe('Server', () => {
       expect(appMock.get.mock.calls[0][0]).toBe('/v1/test');
       expect(appMock.post).toBeCalledTimes(1);
       expect(appMock.post.mock.calls[0][0]).toBe('/v1/test');
+      expect(appMock.options).toBeCalledTimes(1);
+      expect(appMock.options.mock.calls[0][0]).toBe('/v1/test');
       expect(appMock.listen).toBeCalledTimes(1);
       expect(appMock.listen.mock.calls[0][0]).toBe(8054);
     });
@@ -113,6 +116,8 @@ describe('Server', () => {
       expect(appMock.get.mock.calls[0][0]).toBe('/v1/test');
       expect(appMock.post).toBeCalledTimes(1);
       expect(appMock.post.mock.calls[0][0]).toBe('/v1/test');
+      expect(appMock.options).toBeCalledTimes(1);
+      expect(appMock.options.mock.calls[0][0]).toBe('/v1/test');
       expect(appMock.listen).toBeCalledTimes(1);
       expect(appMock.listen.mock.calls[0][0]).toBe(8054);
     });
@@ -155,6 +160,8 @@ describe('Server', () => {
       expect(appMock.get.mock.calls[0][0]).toBe('/v1/test');
       expect(appMock.post).toBeCalledTimes(1);
       expect(appMock.post.mock.calls[0][0]).toBe('/v1/test');
+      expect(appMock.options).toBeCalledTimes(1);
+      expect(appMock.options.mock.calls[0][0]).toBe('/v1/test');
       app.listen(8054);
       expect(appMock.listen).toBeCalledTimes(1);
       expect(appMock.listen.mock.calls[0][0]).toBe(8054);