dgram: skip dns.lookup() for literal IP addresses · nodejs/node@9e50bb0 · GitHub
Skip to content

Commit 9e50bb0

Browse files
BridgeARrichardlau
authored andcommitted
dgram: skip dns.lookup() for literal IP addresses
Every unconnected send(), and the implicit bind on first send, resolved the destination through dns.lookup() even when it was already a literal IP. The address resolves to itself, so the call is redundant, and tools that instrument dns.lookup() record a lookup for every datagram sent to an IP. Skip the resolver for a literal IP of the socket's family and report it on the next tick, keeping dns.lookup()'s asynchronous contract. A custom lookup function is still consulted for every address. Refs: DataDog/dd-trace-js#2984 Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de> PR-URL: #64133 Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent b7bf6e3 commit 9e50bb0

6 files changed

Lines changed: 155 additions & 17 deletions

File tree

benchmark/dgram/send-to-ip.js

Lines changed: 43 additions & 0 deletions

doc/api/dgram.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,6 +1039,8 @@ changes:
10391039
* `recvBufferSize` {number} Sets the `SO_RCVBUF` socket value.
10401040
* `sendBufferSize` {number} Sets the `SO_SNDBUF` socket value.
10411041
* `lookup` {Function} Custom lookup function. **Default:** [`dns.lookup()`][].
1042+
When the default is used, a literal IP address of the socket's family
1043+
resolves to itself without calling [`dns.lookup()`][].
10421044
* `signal` {AbortSignal} An AbortSignal that may be used to close a socket.
10431045
* `receiveBlockList` {net.BlockList} `receiveBlockList` can be used for discarding
10441046
inbound datagram to specific IP addresses, IP ranges, or IP subnets. This does not

lib/internal/dgram.js

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const {
88
const { codes: {
99
ERR_SOCKET_BAD_TYPE,
1010
} } = require('internal/errors');
11+
const { isIP } = require('internal/net');
1112
const { UDP } = internalBinding('udp_wrap');
1213
const { guessHandleType } = require('internal/util');
1314
const {
@@ -28,13 +29,22 @@ function lookup6(lookup, address, callback) {
2829
return lookup(address || '::1', 6, callback);
2930
}
3031

32+
// A literal IP of the socket's family resolves to itself, so skip dns.lookup().
33+
// Defer with nextTick to keep the callback async (e.g. bind()'s 'listening').
34+
function defaultLookup(address, family, callback) {
35+
if (isIP(address) === family) {
36+
process.nextTick(callback, null, address, family);
37+
return;
38+
}
39+
if (dns === undefined) {
40+
dns = require('dns');
41+
}
42+
return dns.lookup(address, family, callback);
43+
}
44+
3145
function newHandle(type, lookup) {
3246
if (lookup === undefined) {
33-
if (dns === undefined) {
34-
dns = require('dns');
35-
}
36-
37-
lookup = dns.lookup;
47+
lookup = defaultLookup;
3848
} else {
3949
validateFunction(lookup, 'lookup');
4050
}

test/parallel/test-dgram-custom-lookup.js

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@ const assert = require('assert');
44
const dgram = require('dgram');
55
const dns = require('dns');
66

7+
const originalLookup = dns.lookup;
8+
79
{
810
// Verify that the provided lookup function is called.
911
const lookup = common.mustCall((host, family, callback) => {
10-
dns.lookup(host, family, callback);
12+
originalLookup(host, family, callback);
1113
});
1214

1315
const socket = dgram.createSocket({ type: 'udp4', lookup });
@@ -18,17 +20,17 @@ const dns = require('dns');
1820
}
1921

2022
{
21-
// Verify that lookup defaults to dns.lookup().
22-
const originalLookup = dns.lookup;
23-
23+
// Verify that the default lookup forwards host names to dns.lookup().
2424
dns.lookup = common.mustCall((host, family, callback) => {
2525
dns.lookup = originalLookup;
26-
originalLookup(host, family, callback);
26+
assert.strictEqual(host, 'example.invalid');
27+
assert.strictEqual(family, 4);
28+
callback(null, '127.0.0.1', 4);
2729
});
2830

2931
const socket = dgram.createSocket({ type: 'udp4' });
3032

31-
socket.bind(common.mustCall(() => {
33+
socket.bind(0, 'example.invalid', common.mustCall(() => {
3234
socket.close();
3335
}));
3436
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
'use strict';
2+
3+
// The default dgram lookup resolves a literal IP address of the socket's own
4+
// family to itself, without calling dns.lookup(). Each case below stubs the
5+
// process-global dns.lookup(), so they run sequentially to keep one case from
6+
// observing another's stub.
7+
8+
const common = require('../common');
9+
const assert = require('assert');
10+
const dgram = require('dgram');
11+
const dns = require('dns');
12+
13+
const originalLookup = dns.lookup;
14+
15+
function ipv4SendSkipsLookup(next) {
16+
dns.lookup = common.mustNotCall('dns.lookup() ran for an IPv4 literal');
17+
18+
const receiver = dgram.createSocket('udp4');
19+
const sender = dgram.createSocket('udp4');
20+
21+
receiver.on('message', common.mustCall((msg) => {
22+
assert.strictEqual(msg.toString(), 'payload');
23+
dns.lookup = originalLookup;
24+
receiver.close();
25+
sender.close();
26+
next();
27+
}));
28+
29+
receiver.bind(0, '127.0.0.1', common.mustCall(() => {
30+
sender.send('payload', receiver.address().port, '127.0.0.1', common.mustCall());
31+
}));
32+
}
33+
34+
function ipv6BindSkipsLookup(next) {
35+
if (!common.hasIPv6) {
36+
next();
37+
return;
38+
}
39+
40+
dns.lookup = common.mustNotCall('dns.lookup() ran for an IPv6 literal');
41+
42+
const socket = dgram.createSocket('udp6');
43+
44+
socket.bind(0, '::1', common.mustCall(() => {
45+
dns.lookup = originalLookup;
46+
socket.close();
47+
next();
48+
}));
49+
}
50+
51+
function mismatchedFamilyFallsThrough(next) {
52+
// '::1' is not an IPv4 literal, so a udp4 socket still resolves it via
53+
// dns.lookup() rather than short-circuiting.
54+
dns.lookup = common.mustCall((host, family, callback) => {
55+
dns.lookup = originalLookup;
56+
assert.strictEqual(host, '::1');
57+
assert.strictEqual(family, 4);
58+
callback(null, '127.0.0.1', 4);
59+
});
60+
61+
const socket = dgram.createSocket('udp4');
62+
63+
socket.bind(0, '::1', common.mustCall(() => {
64+
socket.close();
65+
next();
66+
}));
67+
}
68+
69+
const cases = [
70+
ipv4SendSkipsLookup,
71+
ipv6BindSkipsLookup,
72+
mismatchedFamilyFallsThrough,
73+
];
74+
75+
(function runNext() {
76+
const testCase = cases.shift();
77+
if (testCase !== undefined) {
78+
testCase(runNext);
79+
}
80+
})();

test/sequential/test-dgram-implicit-bind-failure.js

Lines changed: 7 additions & 6 deletions

0 commit comments

Comments
 (0)