Skip to content

Commit

Permalink
tls: accept lookup option for tls.connect()
Browse files Browse the repository at this point in the history
`net.connect()` and consequently `http.Agent` support custom DNS
`lookup` option. However, as we move to `https.Agent` - this option no
longer works because it is not proxied by `tls.connect`.

Fix this inconsistency by passing it down to `net.connect`.

PR-URL: nodejs#12839
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
indutny authored and Olivier Martin committed May 19, 2017
1 parent 5d393d5 commit 254dc91
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 1 deletion.
5 changes: 5 additions & 0 deletions doc/api/tls.md
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,9 @@ decrease overall server throughput.
<!-- YAML
added: v0.11.3
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/12839
description: The `lookup` option is supported now.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/11984
description: The `ALPNProtocols` and `NPNProtocols` options can
Expand Down Expand Up @@ -809,6 +812,7 @@ changes:
`tls.createSecureContext()`. *Note*: In effect, all
[`tls.createSecureContext()`][] options can be provided, but they will be
_completely ignored_ unless the `secureContext` option is missing.
* `lookup`: {Function} Custom lookup function. Defaults to [`dns.lookup()`][].
* ...: Optional [`tls.createSecureContext()`][] options can be provided, see
the `secureContext` option for more information.
* `callback` {Function}
Expand Down Expand Up @@ -1291,3 +1295,4 @@ where `secure_socket` has the same API as `pair.cleartext`.
[modifying the default cipher suite]: #tls_modifying_the_default_tls_cipher_suite
[specific attacks affecting larger AES key sizes]: https://www.schneier.com/blog/archives/2009/07/another_new_aes.html
[tls.Server]: #tls_class_tls_server
[`dns.lookup()`]: dns.html#dns_dns_lookup_hostname_options_callback
3 changes: 2 additions & 1 deletion lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -1063,7 +1063,8 @@ exports.connect = function(...args /* [port,] [host,] [options,] [cb] */) {
port: options.port,
host: options.host,
family: options.family,
localAddress: options.localAddress
localAddress: options.localAddress,
lookup: options.lookup
};
}
socket.connect(connect_opt, function() {
Expand Down
32 changes: 32 additions & 0 deletions test/parallel/test-tls-lookup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const tls = require('tls');

const expectedError = /^TypeError: "lookup" option should be a function$/;

['foobar', 1, {}, []].forEach(function connectThrows(input) {
const opts = {
host: 'localhost',
port: common.PORT,
lookup: input
};

assert.throws(function() {
tls.connect(opts);
}, expectedError);
});

connectDoesNotThrow(common.mustCall(() => {}));

function connectDoesNotThrow(input) {
const opts = {
host: 'localhost',
port: common.PORT,
lookup: input
};

assert.doesNotThrow(function() {
tls.connect(opts);
});
}

0 comments on commit 254dc91

Please sign in to comment.