CORS hook throws errors

[kevinburkeshyp]

Sails version: latest
Node version: irrelevant
NPM version: irrelevant
Operating system: irrelevant

---

If I define the following in config/routes.js

    '*': {
        cors: true,
    },

A preflight request with an Access-Control-Request-Method header (sent by the latest version of Chrome) triggers the following thrown error in cors/index.js:

TypeError: Cannot read property 'origin' of undefined
  at _sendHeaders (/Users/burke/code/api/node_modules/sails/lib/hooks/cors/index.js:159:35)
  at routeTargetFnWrapper (/Users/burke/code/api/node_modules/sails/lib/router/bind.js:178:5)
  at Layer.handle [as handle_request] (/Users/burke/code/api/node_modules/express/lib/router/layer.js:95:5)
  at next (/Users/burke/code/api/node_modules/express/lib/router/route.js:131:13)
  at Route.dispatch (/Users/burke/code/api/node_modules/express/lib/router/route.js:112:3)
  at Layer.handle [as handle_request] (/Users/burke/code/api/node_modules/express/lib/router/layer.js:95:5)
  at /Users/burke/code/api/node_modules/express/lib/router/index.js:279:22
  at param (/Users/burke/code/api/node_modules/express/lib/router/index.js:351:14)
  at param (/Users/burke/code/api/node_modules/express/lib/router/index.js:367:14)
  at Function.process_params (/Users/burke/code/api/node_modules/express/lib/router/index.js:412:3)

The relevant Sails code is here: https://github.com/balderdashy/sails/blob/master/lib/hooks/cors/to-prepare-send-headers.js#L40-L41

routeCorsConfig is undefined after that, which throws the TypeError later on.

This is only OK because Express 3 catches thrown errors, and the Express 3 options handler ignores errors. I've added braces and a console.log to make this clear (in express/lib/router/index.js):

    // implied OPTIONS
    if (!route && 'OPTIONS' == req.method) {
      console.log('error', err);
      return self._options(req, res, next);
    }

This behavior changes in Express 4 - it generates a 500 server error, which is how I noticed it. It seems bad to be relying on Express's fallback behavior like this, especially because whatever callback that gets registered isn't going to be run.

[kevinburkeshyp]

Here's how we've fixed this on our fork, Shyp#36, you should be able to pull in the test, but you'll need to apply the patch to to-prepare-send-headers.js instead of cors/index.js.

[sgress454]

Thanks @kevinburkeshyp, taking a look!