dgram: skip the custom lookup for literal IP addresses

A user-supplied lookup function is no longer called when the
destination is a literal IP of the socket's family; the address is
used directly, matching net.connect(), which skips the lookup for a
literal IP host before consulting options.lookup. This is a breaking
change for a lookup that expected to be invoked for IP addresses.

Refs: https://github.com/DataDog/dd-trace-js/issues/2984
Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: https://github.com/nodejs/node/pull/64131
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>

doc/api/dgram.md

@@ -1043,8 +1043,8 @@ changes:
   * `recvBufferSize` {number} Sets the `SO_RCVBUF` socket value.
   * `sendBufferSize` {number} Sets the `SO_SNDBUF` socket value.
   * `lookup` {Function} Custom lookup function. **Default:** [`dns.lookup()`][].
-    When the default is used, a literal IP address of the socket's family
-    resolves to itself without calling [`dns.lookup()`][].
+    A literal IP address of the socket's family resolves to itself; the lookup
+    function is not called for it.
   * `signal` {AbortSignal} An AbortSignal that may be used to close a socket.
   * `receiveBlockList` {net.BlockList} `receiveBlockList` can be used for discarding
     inbound datagram to specific IP addresses, IP ranges, or IP subnets. This does not

lib/internal/dgram.js

@@ -19,32 +19,35 @@ const { UV_EINVAL } = internalBinding('uv');
 const kStateSymbol = Symbol('state symbol');
 let dns;  // Lazy load for startup performance.
 
-
-function lookup4(lookup, address, callback) {
-  return lookup(address || '127.0.0.1', 4, callback);
-}
-
-
-function lookup6(lookup, address, callback) {
-  return lookup(address || '::1', 6, callback);
-}
-
-// A literal IP of the socket's family resolves to itself, so skip dns.lookup().
-// Defer with nextTick to keep the callback async (e.g. bind()'s 'listening').
-function defaultLookup(address, family, callback) {
+function lookupOrSkip(lookup, address, family, callback) {
   if (isIP(address) === family) {
     process.nextTick(callback, null, address, family);
     return;
   }
-  if (dns === undefined) {
-    dns = require('dns');
+  return lookup(address, family, callback);
+}
+
+function lookup4(lookup, address, callback) {
+  if (address) {
+    return lookupOrSkip(lookup, address, 4, callback);
   }
-  return dns.lookup(address, family, callback);
+  process.nextTick(callback, null, '127.0.0.1', 4);
+}
+
+function lookup6(lookup, address, callback) {
+  if (address) {
+    return lookupOrSkip(lookup, address, 6, callback);
+  }
+  process.nextTick(callback, null, '::1', 6);
 }
 
 function newHandle(type, lookup) {
   if (lookup === undefined) {
-    lookup = defaultLookup;
+    if (dns === undefined) {
+      dns = require('dns');
+    }
+
+    lookup = dns.lookup;
   } else {
     validateFunction(lookup, 'lookup');
   }

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

@@ -9,18 +9,30 @@ const originalLookup = dns.lookup;
 {
   // Verify that the provided lookup function is called.
   const lookup = common.mustCall((host, family, callback) => {
-    originalLookup(host, family, callback);
+    assert.strictEqual(host, 'example.invalid');
+    callback(null, '127.0.0.1', 4);
   });
 
   const socket = dgram.createSocket({ type: 'udp4', lookup });
 
-  socket.bind(common.mustCall(() => {
+  socket.bind(0, 'example.invalid', common.mustCall(() => {
     socket.close();
   }));
 }
 
 {
-  // Verify that the default lookup forwards host names to dns.lookup().
+  // IPs resolve to themselves, so a custom lookup is not called.
+  const lookup = common.mustNotCall('lookup ran for a literal IP address');
+
+  const socket = dgram.createSocket({ type: 'udp4', lookup });
+
+  socket.bind(0, '127.0.0.1', common.mustCall(() => {
+    socket.close();
+  }));
+}
+
+{
+  // Verify that lookup defaults to dns.lookup().
   dns.lookup = common.mustCall((host, family, callback) => {
     dns.lookup = originalLookup;
     assert.strictEqual(host, 'example.invalid');