Node: tls.connect() options ciphers no longer accept null as a valid value in node v15.3.0

[T1B0]

• Version:

node v15.3.0

• Platform:

Reproduced on Linux 5.9.0-3-amd64 #1 SMP Debian 5.9.9-1 (2020-11-19) x86_64 GNU/Linux - but probably applicable on all platform

• Subsystem:

tls.connect() options

What steps will reproduce the bug?

Before version 15.3.0 tls.connect (also accepted by https.request() ) option value null was accepted as falsy value for the cipthers option.
As of version 15.3.0, passing option.ciphers = null throw an error.

case.js (tweaked from https://nodejs.org/api/https.html#https_https_request_options_callback )

const https = require('https');

const options = {
  hostname: 'nodejs.org',
  port: 443,
  path: '/en/',
  ciphers: null,
  method: 'GET'
};

const req = https.request(options, (res) => {
  console.log('statusCode:', res.statusCode);
  console.log('headers:', res.headers);

  res.on('data', (d) => {
    process.stdout.write(d);
  });
});

req.on('error', (e) => {
  console.error(e);
});

req.end();

You get a connection that end up with 200 OK using this v15.2.1 dockerfile

FROM node:15.2.1-buster-slim

COPY ./case.js ./

CMD node case.js

but it will throw an error with a v15.3.0 dockerfile

FROM node:15.3.0-buster-slim

COPY ./case.js ./

CMD node case.js

How often does it reproduce? Is there a required condition?

throw an error 100% of the time on v15.3.0 with options.ciphers = null

What is the expected behavior?

I have no doubt it is a changing behavior, but i don't know what was the expected behavior of an undocumented cipher option value in the first place either. i just know that it used to work.

What do you see instead?

behavior changed, it now throw an error :

node:internal/validators:123
    throw new ERR_INVALID_ARG_TYPE(name, 'string', value);
    ^

TypeError [ERR_INVALID_ARG_TYPE]: The "options.ciphers" property must be of type string. Received null
    at new NodeError (node:internal/errors:278:15)
    at validateString (node:internal/validators:123:11)
    at Object.createSecureContext (node:_tls_common:267:5)
    at Object.connect (node:_tls_wrap:1581:48)
    at Agent.createConnection (node:https:129:22)
    at Agent.createSocket (node:_http_agent:323:26)
    at Agent.addRequest (node:_http_agent:274:10)
    at new ClientRequest (node:_http_client:318:16)
    at Object.request (node:https:313:10)
    at Object.<anonymous> (/case.js:11:19) {
  code: 'ERR_INVALID_ARG_TYPE'

Additional information

• I stumbled upon this because it breaks real world npm module elasticsearch-legacy-client on v15.3.0 build (see https://github.com/elastic/elasticsearch-js-legacy/blob/16.x/src/lib/host.js#L44 )
• the new version's error message is clear and seems well-adapted to the given context.
• this change seem to be introduced by https://github.com/nodejs/node/pull/35368/files

The point of this report is to warn about this non-obvious breaking behavior change and not to say it's not acceptable/legit api change.

ps: Thanks to @jasnell for encouraging me to write an issue :+1:

[T1B0]

cc @spalger for the https://github.com/elastic/elasticsearch-js-legacy/ part ^^

[Trott]

I did a bisect and the commit that introduced this issue is 35274cbddf7206687b2e30d32e57d293750e19a0.

[jasnell]

Yep, already planning on working on it on Monday.

[Trott]

Yep, already planning on working on it on Monday.

It's a one-line fix so I hope I'm not ruining your Monday plans/strategy.: https://github.com/nodejs/node/pull/36318

[tmadeira]

Yep, already planning on working on it on Monday.

It's a one-line fix so I hope I'm not ruining your Monday plans/strategy.: #36318

I think the same fix is needed for dhparam !== undefined, crl !== undefined, sessionIdContext !== undefined, pfx !== undefined. Even setting ciphers here I'm getting "Error: Unable to load PFX certificate" in a code that used to work before node v15.3.0.