Properly send CORS headers for options request

lib/serve.js

@@ -39,23 +39,18 @@ module.exports = {
 
     app.use(bodyParser.json({ limit: '5mb' }));
 
-    app.use(function optionsHandler(req, res, next) {
-      if(req.method !== 'OPTIONS') {
-        next();
-      } else {
-        res.sendStatus(200);
-      }
-    });
-
     for (let funcConf of funcConfs) {
       for (let httpEvent of funcConf.events) {
         const method = httpEvent.method.toLowerCase();
         const endpoint = `/${this.options.stage}/${httpEvent.path}`;
         const path = endpoint.replace(/\{(.+?)\}/g, ':$1');
         let handler = this._handlerBase(funcConf);
+        let optionsHandler = this._optionsHandler;
         if (httpEvent.cors) {
           handler = this._handlerAddCors(handler);
+          optionsHandler = this._handlerAddCors(optionsHandler);
         }
+        app.options(path, optionsHandler);
         app[method](
           path,
           handler
@@ -122,4 +117,8 @@ module.exports = {
       });
     }
   },
+
+  _optionsHandler(req, res) {
+    res.sendStatus(200);
+  },
 };

tests/express.mock.js

@@ -5,6 +5,7 @@ const appMock = {
   use: sinon.spy(),
   get: sinon.spy(),
   post: sinon.spy(),
+  options: sinon.spy(),
 };
 
 const expressMock = sinon.stub().returns(appMock);
@@ -15,6 +16,7 @@ expressMock._resetSpies = () => {
   appMock.use.reset();
   appMock.get.reset();
   appMock.post.reset();
+  appMock.options.reset();
 };
 
-module.exports = () => expressMock;
+module.exports = () => expressMock;

tests/serve.test.js

@@ -113,6 +113,17 @@ describe('serve', () => {
     });
   });
 
+  describe('_optionsHandler', () => {
+    it('should send a 200 express response', () => {
+      const testRes = {
+        sendStatus: sinon.spy(),
+      };
+      const handler = module._optionsHandler;
+      handler({}, testRes);
+      expect(testRes.sendStatus).to.have.been.calledWith(200);
+    });
+  });
+
   describe('_handlerAddCors', () => {
     it('should retun an express handler', () => {
       const res = module._handlerAddCors();
@@ -234,39 +245,6 @@ describe('serve', () => {
       }));
     });
 
-    describe('OPTIONS handler', () => {
-      let optionsHandler;
-
-      beforeEach(() => {
-        const res = module._newExpressApp([]);
-        const optionsHandlers = res.use.getCalls().filter(c =>
-          typeof c.args[0] === 'function' &&
-          c.args[0].name === 'optionsHandler'
-        );
-        optionsHandler = optionsHandlers.length ? optionsHandlers[0].args[0] : null;
-      });
-
-      it('should add an OPTIONS request handler', () => {
-        expect(optionsHandler).to.exist;
-      });
-
-      it('should continue for non OPTIONS requests', () => {
-        const req = { method: 'GET' };
-        const next = sinon.spy();
-        optionsHandler(req, {}, next);
-        expect(next).to.have.callCount(1);
-      });
-
-      it('should send status 200 for OPTIONS requests', () => {
-        const req = { method: 'OPTIONS' };
-        const res = { sendStatus: sinon.spy() };
-        const next = sinon.spy();
-        optionsHandler(req, res, next);
-        expect(res.sendStatus).to.have.been.calledWith(200);
-        expect(next).to.have.callCount(0);
-      });
-    });
-
     it('should create express handlers for all functions http event', () => {
       const testFuncsConfs = [
         {
@@ -299,7 +277,9 @@ describe('serve', () => {
       module.options.stage = testStage;
       const testHandlerBase = 'testHandlerBase';
       const testHandlerCors = 'testHandlerCors';
+      const testHandlerOptions = 'testHandlerOptions';
       module._handlerBase = sinon.stub().returns(testHandlerBase);
+      module._optionsHandler = testHandlerOptions;
       module._handlerAddCors = sinon.stub().returns(testHandlerCors);
       const app = module._newExpressApp(testFuncsConfs);
       expect(app.get).to.have.callCount(1);
@@ -318,6 +298,15 @@ describe('serve', () => {
       expect(module.serverless.cli.consoleLog).to.have.been.calledWith(
         '  POST - http://localhost:8000/test/func2path/{testParam}'
       );
+      expect(app.options).to.have.callCount(2);
+      expect(app.options.firstCall).to.have.been.calledWith(
+        '/test/func1path',
+        testHandlerCors
+      );
+      expect(app.options.secondCall).to.have.been.calledWith(
+        '/test/func2path/:testParam',
+        testHandlerOptions
+      );
     });
   });