https: handle invalid TLS options in proxied requests

lib/https.js

@@ -292,11 +292,20 @@ function establishTunnel(agent, socket, options, tunnelConfig, afterSocket) {
         debug('Propagate error event from tunneled socket to tunnel socket');
         afterSocket(err, tunneldSocket);
       };
-      tunneldSocket = tls.connect(requestOptions, () => {
-        debug('TLS handshake over tunnel succeeded');
-        tunneldSocket.removeListener('error', onTLSHandshakeError);
-        afterSocket(null, tunneldSocket);
-      });
+      // tls.connect() validates options synchronously, and here it runs after
+      // https.request() has returned, so a throw would be uncaught.
+      try {
+        tunneldSocket = tls.connect(requestOptions, () => {
+          debug('TLS handshake over tunnel succeeded');
+          tunneldSocket.removeListener('error', onTLSHandshakeError);
+          afterSocket(null, tunneldSocket);
+        });
+      } catch (err) {
+        debug('tls.connect() over tunnel threw', err);
+        socket.destroy();
+        afterSocket(err, socket);
+        return true;
+      }
       if (requestOptions[kPerRequestCheckServerIdentity])
         tunneldSocket[kPerRequestCheckServerIdentity] = true;
       tunneldSocket.on('free', () => {

test/client-proxy/test-https-proxy-request-invalid-tls-options.mjs

@@ -0,0 +1,75 @@
+// This tests that invalid TLS options do not result in an uncaught exception
+// after an HTTPS proxy tunnel has been established.
+
+import * as common from '../common/index.mjs';
+import assert from 'node:assert';
+import { once } from 'events';
+import fixtures from '../common/fixtures.js';
+import { createProxyServer } from '../common/proxy-server.js';
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+// https must be dynamically imported so that builds without crypto support
+// can skip it.
+const { default: https } = await import('node:https');
+
+const server = https.createServer({
+  cert: fixtures.readKey('agent8-cert.pem'),
+  key: fixtures.readKey('agent8-key.pem'),
+}, common.mustNotCall());
+server.on('error', common.mustNotCall());
+server.listen(0);
+await once(server, 'listening');
+
+const { proxy, logs } = createProxyServer();
+proxy.listen(0);
+await once(proxy, 'listening');
+
+const serverHost = `localhost:${server.address().port}`;
+
+// tls.connect() throws for these options while building the secure context,
+// before any handshake happens.
+const testCases = [
+  { options: { minVersion: 'definitely-invalid' }, code: 'ERR_TLS_INVALID_PROTOCOL_VERSION' },
+  { options: { ciphers: 123 }, code: 'ERR_INVALID_ARG_TYPE' },
+  { options: { secureProtocol: 'definitely-invalid' }, code: 'ERR_TLS_INVALID_PROTOCOL_METHOD' },
+];
+
+for (const { options, code } of testCases) {
+  const agent = new https.Agent({
+    ca: fixtures.readKey('fake-startcom-root-cert.pem'),
+    proxyEnv: {
+      HTTPS_PROXY: `http://localhost:${proxy.address().port}`,
+    },
+  });
+  const req = https.get({
+    host: 'localhost',
+    port: server.address().port,
+    path: '/test',
+    agent,
+    ...options,
+  }, common.mustNotCall());
+  const [err] = await once(req, 'error');
+  assert.strictEqual(err.code, code);
+  agent.destroy();
+}
+
+// Verify that the requests went through the proxy and the tunnel was established.
+const requests = logs.filter((log) => !('error' in log));
+// The client resets the tunnel as soon as tls.connect() throws, which the
+// proxy may observe as an ECONNRESET while still relaying it.
+const errors = logs.filter((log) =>
+  'error' in log && log.error.code !== 'ECONNRESET');
+
+assert.deepStrictEqual(requests, testCases.map(() => ({
+  method: 'CONNECT',
+  url: serverHost,
+  headers: {
+    'host': serverHost,
+  },
+})));
+assert.deepStrictEqual(errors, []);
+
+proxy.close();
+server.close();