Skip to content

Commit

Permalink
tls: make tls.connect() accept a timeout option
Browse files Browse the repository at this point in the history
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 <vieuxtech@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
lpinca authored and MylesBorins committed May 16, 2019
1 parent b4b4c11 commit 2319bc5
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 0 deletions.
7 changes: 7 additions & 0 deletions doc/api/tls.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
19 changes: 19 additions & 0 deletions test/parallel/test-tls-connect-timeout-option.js
Original file line number Diff line number Diff line change
@@ -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);

0 comments on commit 2319bc5

Please sign in to comment.