Skip to content

Commit

Permalink
dns: make dns.Resolver timeout configurable
Browse files Browse the repository at this point in the history
PR-URL: #33472
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
  • Loading branch information
bnoordhuis authored and codebytere committed Jun 18, 2020
1 parent 15eb5a3 commit d4442b1
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 13 deletions.
16 changes: 16 additions & 0 deletions doc/api/dns.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,22 @@ The following methods from the `dns` module are available:
* [`resolver.reverse()`][`dns.reverse()`]
* [`resolver.setServers()`][`dns.setServers()`]

### `Resolver([options])`
<!-- YAML
added: v8.3.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/33472
description: The constructor now accepts an `options` object.
The single supported option is `timeout`.
-->

Create a new resolver.

* `options` {Object}
* `timeout` {integer} Query timeout in milliseconds, or `-1` to use the
default timeout.

### `resolver.cancel()`
<!-- YAML
added: v8.3.0
Expand Down
6 changes: 4 additions & 2 deletions lib/internal/dns/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const {
bindDefaultResolver,
Resolver: CallbackResolver,
validateHints,
validateTimeout,
emitInvalidHostnameWarning,
} = require('internal/dns/utils');
const { codes, dnsException } = require('internal/errors');
Expand Down Expand Up @@ -208,8 +209,9 @@ const resolveMap = ObjectCreate(null);

// Resolver instances correspond 1:1 to c-ares channels.
class Resolver {
constructor() {
this._handle = new ChannelWrap();
constructor(options = undefined) {
const timeout = validateTimeout(options);
this._handle = new ChannelWrap(timeout);
}
}

Expand Down
13 changes: 11 additions & 2 deletions lib/internal/dns/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const {

const errors = require('internal/errors');
const { isIP } = require('internal/net');
const { validateInt32 } = require('internal/validators');
const {
ChannelWrap,
strerror,
Expand All @@ -23,10 +24,17 @@ const {
ERR_INVALID_OPT_VALUE
} = errors.codes;

function validateTimeout(options) {
const { timeout = -1 } = { ...options };
validateInt32(timeout, 'options.timeout', -1, 2 ** 31 - 1);
return timeout;
}

// Resolver instances correspond 1:1 to c-ares channels.
class Resolver {
constructor() {
this._handle = new ChannelWrap();
constructor(options = undefined) {
const timeout = validateTimeout(options);
this._handle = new ChannelWrap(timeout);
}

cancel() {
Expand Down Expand Up @@ -162,6 +170,7 @@ module.exports = {
getDefaultResolver,
setDefaultResolver,
validateHints,
validateTimeout,
Resolver,
emitInvalidHostnameWarning,
};
26 changes: 17 additions & 9 deletions src/cares_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ using node_ares_task_list =

class ChannelWrap : public AsyncWrap {
public:
ChannelWrap(Environment* env, Local<Object> object);
ChannelWrap(Environment* env, Local<Object> object, int timeout);
~ChannelWrap() override;

static void New(const FunctionCallbackInfo<Value>& args);
Expand Down Expand Up @@ -190,18 +190,21 @@ class ChannelWrap : public AsyncWrap {
bool query_last_ok_;
bool is_servers_default_;
bool library_inited_;
int timeout_;
int active_query_count_;
node_ares_task_list task_list_;
};

ChannelWrap::ChannelWrap(Environment* env,
Local<Object> object)
Local<Object> object,
int timeout)
: AsyncWrap(env, object, PROVIDER_DNSCHANNEL),
timer_handle_(nullptr),
channel_(nullptr),
query_last_ok_(true),
is_servers_default_(true),
library_inited_(false),
timeout_(timeout),
active_query_count_(0) {
MakeWeak();

Expand All @@ -210,10 +213,11 @@ ChannelWrap::ChannelWrap(Environment* env,

void ChannelWrap::New(const FunctionCallbackInfo<Value>& args) {
CHECK(args.IsConstructCall());
CHECK_EQ(args.Length(), 0);

CHECK_EQ(args.Length(), 1);
CHECK(args[0]->IsInt32());
const int timeout = args[0].As<Int32>()->Value();
Environment* env = Environment::GetCurrent(args);
new ChannelWrap(env, args.This());
new ChannelWrap(env, args.This(), timeout);
}

class GetAddrInfoReqWrap : public ReqWrap<uv_getaddrinfo_t> {
Expand Down Expand Up @@ -462,6 +466,7 @@ void ChannelWrap::Setup() {
options.flags = ARES_FLAG_NOCHECKRESP;
options.sock_state_cb = ares_sockstate_cb;
options.sock_state_cb_data = this;
options.timeout = timeout_;

int r;
if (!library_inited_) {
Expand All @@ -474,9 +479,9 @@ void ChannelWrap::Setup() {
}

/* We do the call to ares_init_option for caller. */
r = ares_init_options(&channel_,
&options,
ARES_OPT_FLAGS | ARES_OPT_SOCK_STATE_CB);
const int optmask =
ARES_OPT_FLAGS | ARES_OPT_TIMEOUTMS | ARES_OPT_SOCK_STATE_CB;
r = ares_init_options(&channel_, &options, optmask);

if (r != ARES_SUCCESS) {
Mutex::ScopedLock lock(ares_library_mutex);
Expand All @@ -495,7 +500,10 @@ void ChannelWrap::StartTimer() {
} else if (uv_is_active(reinterpret_cast<uv_handle_t*>(timer_handle_))) {
return;
}
uv_timer_start(timer_handle_, AresTimeout, 1000, 1000);
int timeout = timeout_;
if (timeout == 0) timeout = 1;
if (timeout < 0 || timeout > 1000) timeout = 1000;
uv_timer_start(timer_handle_, AresTimeout, timeout, timeout);
}

void ChannelWrap::CloseTimer() {
Expand Down
53 changes: 53 additions & 0 deletions test/parallel/test-dns-channel-timeout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const dgram = require('dgram');
const dns = require('dns');

for (const ctor of [dns.Resolver, dns.promises.Resolver]) {
for (const timeout of [null, true, false, '', '2']) {
assert.throws(() => new ctor({ timeout }), {
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
});
}

for (const timeout of [-2, 4.2, 2 ** 31]) {
assert.throws(() => new ctor({ timeout }), {
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError',
});
}

for (const timeout of [-1, 0, 1]) new ctor({ timeout }); // OK
}

for (const timeout of [0, 1, 2]) {
const server = dgram.createSocket('udp4');
server.bind(0, '127.0.0.1', common.mustCall(() => {
const resolver = new dns.Resolver({ timeout });
resolver.setServers([`127.0.0.1:${server.address().port}`]);
resolver.resolve4('nodejs.org', common.mustCall((err) => {
assert.throws(() => { throw err; }, {
code: 'ETIMEOUT',
name: 'Error',
});
server.close();
}));
}));
}

for (const timeout of [0, 1, 2]) {
const server = dgram.createSocket('udp4');
server.bind(0, '127.0.0.1', common.mustCall(() => {
const resolver = new dns.promises.Resolver({ timeout });
resolver.setServers([`127.0.0.1:${server.address().port}`]);
resolver.resolve4('nodejs.org').catch(common.mustCall((err) => {
assert.throws(() => { throw err; }, {
code: 'ETIMEOUT',
name: 'Error',
});
server.close();
}));
}));
}

0 comments on commit d4442b1

Please sign in to comment.