tls: add missing 'new'

[cjihrig]

ERR_INVALID_OPT_VALUE cannot be constructed without new.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

[nodejs-github-bot]

Lite-CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3465

[sam-github]

Sorry about that :-(.

I think this should have a regression test.

[sam-github]

I'm writing a test, I'll post it here.

[sam-github]

@cjihrig how did you find this, by code inspection, or did you actually trigger the bug?

I suggest the following (I can PR, or you add to your commit, as you wish).

diff --git a/lib/_tls_common.js b/lib/_tls_common.js
index 2a4ad9b24e..82ab45f389 100644
--- a/lib/_tls_common.js
+++ b/lib/_tls_common.js
@@ -163,13 +163,14 @@ exports.createSecureContext = function createSecureContext(options) {                                                                                  
   // cipher suites all have a standard name format beginning with TLS_, so split
   // the ciphers and pass them to the appropriate API.
   const ciphers = (options.ciphers || tls.DEFAULT_CIPHERS).split(':');
-  const cipherList = ciphers.filter((_) => !_.match(/^TLS_/)).join(':');
+  const cipherList = ciphers.filter((_) => !_.match(/^TLS_/) &&
+                                    _.length > 0).join(':');
   const cipherSuites = ciphers.filter((_) => _.match(/^TLS_/)).join(':');

   if (cipherSuites === '' && cipherList === '') {
     // Specifying empty cipher suites for both TLS1.2 and TLS1.3 is invalid, its
     // not possible to handshake with no suites.
-    throw ERR_INVALID_OPT_VALUE('ciphers', ciphers);
+    throw new ERR_INVALID_OPT_VALUE('ciphers', ciphers);
   }

   c.context.setCipherSuites(cipherSuites);
diff --git a/test/parallel/test-tls-set-ciphers.js b/test/parallel/test-tls-set-ciphers.js                                                                                   
index 96eb0944f4..fbca83b3d5 100644
--- a/test/parallel/test-tls-set-ciphers.js
+++ b/test/parallel/test-tls-set-ciphers.js
@@ -91,3 +91,13 @@ test('TLS_AES_128_CCM_8_SHA256', U,

 test('TLS_AES_128_CCM_8_SHA256', 'TLS_AES_128_CCM_8_SHA256',
      'TLS_AES_128_CCM_8_SHA256');
+
+// Invalid cipher values
+test(9, 'AES256-SHA', U, 'ERR_INVALID_ARG_TYPE', U);
+test('AES256-SHA', 9, U, U, 'ERR_INVALID_ARG_TYPE');
+test(':', 'AES256-SHA', U, 'ERR_INVALID_OPT_VALUE', U);
+test('AES256-SHA', ':', U, U, 'ERR_INVALID_OPT_VALUE');
+
+// Using '' is synonymous for "use default ciphers"
+test('TLS_AES_256_GCM_SHA384', '', 'TLS_AES_256_GCM_SHA384');
+test('', 'TLS_AES_256_GCM_SHA384', 'TLS_AES_256_GCM_SHA384');

[Trott]

Change LGTM, but I have a (non-blocking) preference that we add a test here too.

[cjihrig]

how did you find this, by code inspection, or did you actually trigger the bug?

@sam-github I just stumbled on it in the code. I didn't trigger it. I'll pull in your other changes as a separate commit with you as the author.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/23030/

EDIT(cjihrig): CI was yellow.

[Trott]

Landed in 95c1cb4...ae749e7