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: #25517 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
nodejs · Jan 20, 2019 · aaa7547 · aaa7547
1 parent cc26957
commit aaa7547
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 0 deletions.
diff --git a/doc/api/tls.md b/doc/api/tls.md
@@ -1023,6 +1023,9 @@ being issued by trusted CA (`options.ca`).
 <!-- 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.
@@ -1088,6 +1091,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}
@@ -1549,6 +1555,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
@@ -1256,6 +1256,11 @@ exports.connect = function connect(...args) {
       localAddress: options.localAddress,
       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
@@ -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);