From 2319bc55ca350c1f2ebdc715dca64da6ec5c84a9 Mon Sep 17 00:00:00 2001
From: Luigi Pinca <luigipinca@gmail.com>
Date: Tue, 15 Jan 2019 16:12:12 +0100
Subject: [PATCH] tls: make tls.connect() accept a timeout option

If specified, and only when a socket is created internally, the option
will make `socket.setTimeout()` to be called on the created socket with
the given timeout.

This is consistent with the `timeout` option of `net.connect()` and
prevents the `timeout` option of the `https.Agent` from being ignored
when a socket is created.

PR-URL: https://github.com/nodejs/node/pull/25517
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
---
 doc/api/tls.md                                |  7 +++++++
 lib/_tls_wrap.js                              |  5 +++++
 .../test-tls-connect-timeout-option.js        | 19 +++++++++++++++++++
 3 files changed, 31 insertions(+)
 create mode 100644 test/parallel/test-tls-connect-timeout-option.js

diff --git a/doc/api/tls.md b/doc/api/tls.md
index e75dbf43a5c663..fce4f48eb7b46a 100644
--- a/doc/api/tls.md
+++ b/doc/api/tls.md
@@ -967,6 +967,9 @@ similar to:
 <!-- YAML
 added: v0.11.3
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/25517
+    description: The `timeout` option is supported now.
   - version: v8.0.0
     pr-url: https://github.com/nodejs/node/pull/12839
     description: The `lookup` option is supported now.
@@ -1029,6 +1032,9 @@ changes:
     `tls.createSecureContext()`.
   * `lookup`: {Function} Custom lookup function. **Default:**
     [`dns.lookup()`][].
+  * `timeout`: {number} If set and if a socket is created internally, will call
+    [`socket.setTimeout(timeout)`][] after the socket is created, but before it
+    starts the connection.
   * ...: [`tls.createSecureContext()`][] options that are used if the
     `secureContext` option is missing, otherwise they are ignored.
 * `callback` {Function}
@@ -1481,6 +1487,7 @@ where `secureSocket` has the same API as `pair.cleartext`.
 [`server.getTicketKeys()`]: #tls_server_getticketkeys
 [`server.listen()`]: net.html#net_server_listen
 [`server.setTicketKeys()`]: #tls_server_setticketkeys_keys
+[`socket.setTimeout(timeout)`]: #net_socket_settimeout_timeout_callback
 [`tls.DEFAULT_ECDH_CURVE`]: #tls_tls_default_ecdh_curve
 [`tls.Server`]: #tls_class_tls_server
 [`tls.TLSSocket.getPeerCertificate()`]: #tls_tlssocket_getpeercertificate_detailed
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
index 4cee805e967b13..7455928dbeba27 100644
--- a/lib/_tls_wrap.js
+++ b/lib/_tls_wrap.js
@@ -1154,6 +1154,11 @@ exports.connect = function connect(...args) {
       localPort: options.localPort,
       lookup: options.lookup
     };
+
+    if (options.timeout) {
+      tlssock.setTimeout(options.timeout);
+    }
+
     tlssock.connect(connectOpt, tlssock._start);
   }
 
diff --git a/test/parallel/test-tls-connect-timeout-option.js b/test/parallel/test-tls-connect-timeout-option.js
new file mode 100644
index 00000000000000..4e4d4325906a97
--- /dev/null
+++ b/test/parallel/test-tls-connect-timeout-option.js
@@ -0,0 +1,19 @@
+'use strict';
+
+const common = require('../common');
+
+// This test verifies that `tls.connect()` honors the `timeout` option when the
+// socket is internally created.
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const tls = require('tls');
+
+const socket = tls.connect({
+  lookup: () => {},
+  timeout: 1000
+});
+
+assert.strictEqual(socket.timeout, 1000);