rejectUnauthorized should be undefined or null by default

[andrew-aladev]

This commit breaks websocket transport.

this.rejectUnauthorized = opts.rejectUnauthorized === undefined ? true : opts.rejectUnauthorized;

So by default socket.io have invalid agent options.

This code is from ws:

if (options.isDefinedAndNonNull('pfx')
   || options.isDefinedAndNonNull('key')
   || options.isDefinedAndNonNull('passphrase')
   || options.isDefinedAndNonNull('cert')
   || options.isDefinedAndNonNull('ca')
   || options.isDefinedAndNonNull('ciphers')
   || options.isDefinedAndNonNull('rejectUnauthorized')) {

    if (options.isDefinedAndNonNull('pfx')) requestOptions.pfx = options.value.pfx;
    if (options.isDefinedAndNonNull('key')) requestOptions.key = options.value.key;
    if (options.isDefinedAndNonNull('passphrase')) requestOptions.passphrase = options.value.passphrase;
    if (options.isDefinedAndNonNull('cert')) requestOptions.cert = options.value.cert;
    if (options.isDefinedAndNonNull('ca')) requestOptions.ca = options.value.ca;
    if (options.isDefinedAndNonNull('ciphers')) requestOptions.ciphers = options.value.ciphers;
    if (options.isDefinedAndNonNull('rejectUnauthorized')) requestOptions.rejectUnauthorized = options.value.rejectUnauthorized;

    if (!agent) {
        // global agent ignores client side certificates
        agent = new httpObj.Agent(requestOptions);
    }
  }

Nodejs throws ENOENT on connect because agent is invalid. Both socket.io and ws don't want to validate http agent. In this case socket.io should have valid default values for agent.

[darrachequesne]

Hi! Thanks for the report, and the detailed analysis. Which version of nodejs is affected? It seems to be working on all but nodejs 0.10 (https://travis-ci.org/socketio/engine.io-client/builds/150137797), do you confirm?

According to the documentation:

rejectUnauthorized: If true the server will reject any connection which is not authorized with the list of supplied CAs. This option only has an effect if requestCert is true. Default: false.

true looks like a valid default value, doesn't it?

[andrew-aladev]

Yes, we are using jxcore. This is a fork of nodejs 0.10.40. I am very busy this week. I will provide more information about this issue later.

[mikermcneil]

@darrachequesne @andrew-aladev We're seeing this in Sails too (and only on Node v0.10). For the time being, we went ahead and pinned our dependency in sails.io.js to sio client@1.4.5 (which @sgress454 confirmed works on Node v0.10), and we're publishing a patch that way.

[mikermcneil]

@darrachequesne in case it helps: here's the failing test report from sails.io.js with trace: https://travis-ci.org/balderdashy/sails.io.js/builds/161471333 (see the log under the failing build for node v0.10)

And here is the commit that fixes it (rolling back and pinning to 1.4.5): https://travis-ci.org/balderdashy/sails.io.js/builds/163916390

[darrachequesne]

Closed by #496

[andrew-aladev]

I've checked now. It works fine, thank you!