CORS functionality inaccessible

[LMulvey]

In #263, the option to enable CORS headers was switched to disabling CORS. The problem is that the server configuration is still fed with opts.cors, which no longer exists (it would be opts.noCors).

https://github.com/maptiler/tileserver-gl/blob/5585f49396452a974a4ccd7c61b777fb073b7e75/src/main.js#L43-L92

So, this leads to a further issue that inside server.js, the usage of the cors() package for Express is guarded by whether or not opts.cors is truth-y (which it never will be!).

https://github.com/maptiler/tileserver-gl/blob/5585f49396452a974a4ccd7c61b777fb073b7e75/src/server.js#L103-L105

The issue is currently two-fold: 1) There is no CORS functionality enabled in the server 2) The no-cors flag currently does nothing

I'm happy to work on a PR for this if someone isn't already addressing this issue.

[zvaraondrej]

@LMulvey it seems like this is a build-in feature of commander:

https://www.npmjs.com/package/commander#other-option-types-negatable-boolean-and-flagvalue

[LMulvey]

The issue isn't with commander. The commander.option will add a property to opts based on the parameter name:

The options can be accessed as properties on the Command object. Multi-word options such as "--template-engine" are camel-cased, becoming program.templateEngine etc. See also optional new behaviour to avoid name clashes.

The option is setup like this in the code above:

 .option( 
     '-C|--no-cors', 
     'Disable Cross-origin resource sharing headers' 
   ) 

So, the option would be accessed via opts.noCors. However, the guard that checks whether or not CORS is disabled currently uses opts.cors–an option that doesn't exist so it will always be undefined.

[zvaraondrej]

As far as I understand the docs correctly, it should work as expected...

https://www.npmjs.com/package/commander#other-option-types-negatable-boolean-and-flagvalue:

Other option types, negatable boolean and flag|value
You can specify a boolean option long name with a leading no- to set the option value to false when used. Defined alone this also makes the option true by default.

const { program } = require('commander');

program
  .option('--no-sauce', 'Remove sauce')
  .option('--cheese <flavour>', 'cheese flavour', 'mozzarella')
  .option('--no-cheese', 'plain with no cheese')
  .parse(process.argv);

const sauceStr = program.sauce ? 'sauce' : 'no sauce';
const cheeseStr = (program.cheese === false) ? 'no cheese' : `${program.cheese} cheese`
console.log(`You ordered a pizza with ${sauceStr} and ${cheeseStr}`);

$ pizza-options
You ordered a pizza with sauce and mozzarella cheese
$ pizza-options --sauce
error: unknown option '--sauce'
$ pizza-options --cheese=blue
You ordered a pizza with sauce and blue cheese
$ pizza-options --no-sauce --no-cheese
You ordered a pizza with no sauce and no cheese

Besides this, I have tested the v.3.0.0 image run with --no-cors and the Access-Control-Allow-Origin: * have not been sent anymore...

[LMulvey]

I think you're misunderstanding. In the current state, CORS headers will never be sent:

 if (opts.cors) { 
   app.use(cors()); 
 } 

opts.cors will always be undefined therefore always false-y. The fix would be this:

if  (!opts.noCors) {
  app.use(cors());
}

[zvaraondrej]

Well I just double checked and it works correctly :) (v3.0.0)

[L3o-pold]

From what I can see using the 3.0.0 version I agree with @LMulvey. Seems that the option is --no-cors but cors are not enable by default so no way to enable it.

[manix]

This is still not working even with the suggested fix, fortunately for me I could just comment out the if statement. Thank you @LMulvey for pointing it out.