Node: dns.lookup allows a "falsy" hostname, but behaviour is undocumented and appears useless

[sam-github]

I assumed this was a bug, but on looking into it, it seems deliberate: https://github.com/nodejs/node/blob/6f216710eb49436f0c372fbaf6da3e65ba9578c2/lib/dns.js#L132

@cjihrig you added this, I think, in 5086d6ef94d1 (missing PR metadata), but it looks like you were cleaning up something from an earlier commit. Any ideas why falsey is allowed?

Its even tested: https://github.com/nodejs/node/blob/6f216710eb49436f0c372fbaf6da3e65ba9578c2/test/parallel/test-dns-lookup.js#L38-L48, but I'm not sure if the test assertions were just to add coverage, or were because that was the expected output, @abouthiroppy added them in https://github.com/nodejs/node/pull/10844, do you have any comments?

My guess was that the intention is that if hostname is falsy, there may be enough information in options to return a useful value, in which case options should be mandatory if hostname is missing.

Except... I don't see how that can be, it looks to me you will always get no results no matter what flags you put into options, and the tests even assert that, so... why is a falsey hostname allowed?

• Version: *
• Platform: *
• Subsystem: dns

% ./node
> dns.lookup(false, console.log)
{}
> null null 4

[cjihrig]

I have no clue. It looks like this behavior goes back at least 6 years. My changes look to have just been building on top of the existing quirks.

[sam-github]

@bnoordhuis do you remember the history of any of this?

[bnoordhuis]

There is no real backstory. It was introduced in commit fd3cd755d15e37fd20aaadf22a30fc84a5cb737f from 2010 as an optimization of sorts.

I think the idea was that an empty hostname string (which is falsy) never returns any results so you might as well take a shortcut.

[sam-github]

@bnoordhuis Is there any reason not to call lookup of an empty string an error? I'm tempted to make everything falsy throw, like

% dig ""
dig: '' is not a legal name (unexpected end of input)
% nslookup ""
nslookup: '' is not in legal name syntax (unexpected end of input)

[bnoordhuis]

Is there any reason not to call lookup of an empty string an error?

No reason except backwards compatibility. :-)

[targos]

So what should we do:

• Deprecate falsy argument?
• Document that?
• Nothing and close?

[sam-github]

node is getting stricter about non-sensical arguments over time, I think that's a good trend, and it we should start to reject arguments that are invalid: false, null, undefined, and the empty string, too, IMO.

[sohailrajdev97]

Is there someone working to change this behavior ? I would like to contribute to this issue and any leads would be appreciated.

[oyyd]

Try to resolve this issue in #23173. Please comment if there are any inappropriate changes.