Stricter options (#101)

* feat: stricter options handling

* chore: clarify origin documentation

* test: falsy non false origin option

Author: codyzu

README.md

@@ -32,7 +32,7 @@ You can use it as is without passing any option, or you can configure it as expl
 ### Options
 * `origin`: Configures the **Access-Control-Allow-Origin** CORS header. The value of origin could be of different types:
   - `Boolean` - set `origin` to `true` to reflect the [request origin](http://tools.ietf.org/html/draft-abarth-origin-09), or set it to `false` to disable CORS.
-  - `String` - set `origin` to a specific origin. For example if you set it to `"http://example.com"` only requests from "http://example.com" will be allowed.
+  - `String` - set `origin` to a specific origin. For example if you set it to `"http://example.com"` only requests from "http://example.com" will be allowed. The special `*` value (default) allows any origin.
   - `RegExp` - set `origin` to a regular expression pattern which will be used to test the request origin. If it's a match, the request origin will be reflected. For example the pattern `/example\.com$/` will reflect any request that is coming from an origin ending with "example.com".
   - `Array` - set `origin` to an array of valid origins. Each origin can be a `String` or a `RegExp`. For example `["http://example1.com", /\.example2\.com$/]` will accept any request from "http://example1.com" or from a subdomain of "example2.com".
   - `Function` - set `origin` to a function implementing some custom logic. The function takes the request origin as the first parameter and a callback as a second (which expects the signature `err [Error | null], origin`), where `origin` is a non function value of the origin option. *Async-await* and promises are supported as well. The Fastify instance is bound to function call and you may access via `this`. For example: 

index.js

@@ -52,13 +52,20 @@ function fastifyCors (fastify, opts, next) {
         return next(error)
       }
 
-      req.corsPreflightEnabled = resolvedOriginOption
-
       // Disable CORS and preflight if false
       if (resolvedOriginOption === false) {
+        req.corsPreflightEnabled = false
         return next()
       }
 
+      // Falsy values are invalid
+      if (!resolvedOriginOption) {
+        return next(new Error('Invalid CORS origin option'))
+      }
+
+      // Enable preflight
+      req.corsPreflightEnabled = true
+
       reply.header('Access-Control-Allow-Origin',
         getAccessControlAllowOriginHeader(req.headers.origin, resolvedOriginOption))
 
@@ -78,7 +85,7 @@ function fastifyCors (fastify, opts, next) {
   }
 
   function preflightHandler (req, reply) {
-    // Do not handle preflight requests if the origin was not allowed
+    // Do not handle preflight requests if the origin option disabled CORS
     if (!req.corsPreflightEnabled) {
       reply.code(404).type('text/plain').send('Not Found')
       return
@@ -132,7 +139,7 @@ function fastifyCors (fastify, opts, next) {
 }
 
 function getAccessControlAllowOriginHeader (reqOrigin, originOption) {
-  if (!originOption || originOption === '*') {
+  if (originOption === '*') {
     // allow any origin
     return '*'
   }

test/cors.test.js

@@ -170,6 +170,26 @@ test('Dynamic origin resolution (errored)', t => {
   })
 })
 
+test('Dynamic origin resolution (invalid result)', t => {
+  t.plan(3)
+
+  const fastify = Fastify()
+  const origin = (header, cb) => {
+    t.strictEqual(header, 'example.com')
+    cb(null, undefined)
+  }
+  fastify.register(cors, { origin })
+
+  fastify.inject({
+    method: 'GET',
+    url: '/',
+    headers: { origin: 'example.com' }
+  }, (err, res) => {
+    t.error(err)
+    t.strictEqual(res.statusCode, 500)
+  })
+})
+
 test('Dynamic origin resolution (valid origin - promises)', t => {
   t.plan(5)
 
@@ -308,6 +328,30 @@ test('Should reply 404 without cors headers other than `vary` when origin is fal
   })
 })
 
+test('Server error if origin option is falsy but not false', t => {
+  t.plan(4)
+
+  const fastify = Fastify()
+  fastify.register(cors, { origin: '' })
+
+  fastify.inject({
+    method: 'GET',
+    url: '/',
+    headers: { origin: 'example.com' }
+  }, (err, res) => {
+    t.error(err)
+    delete res.headers.date
+    t.strictEqual(res.statusCode, 500)
+    t.deepEqual(res.json(), { statusCode: 500, error: 'Internal Server Error', message: 'Invalid CORS origin option' })
+    t.deepEqual(res.headers, {
+      'content-length': '89',
+      'content-type': 'application/json; charset=utf-8',
+      connection: 'keep-alive',
+      vary: 'Origin'
+    })
+  })
+})
+
 test('Allow only request from a specific origin', t => {
   t.plan(4)