Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: dns docs for family = 0 and all = true is potentially misleading #51482

Closed
domdomegg opened this issue Jan 16, 2024 · 2 comments · Fixed by #51517 or #51653
Closed

docs: dns docs for family = 0 and all = true is potentially misleading #51482

domdomegg opened this issue Jan 16, 2024 · 2 comments · Fixed by #51517 or #51653
Labels
doc Issues and PRs related to the documentations.

Comments

@domdomegg
Copy link
Contributor

Affected URL(s)

https://nodejs.org/api/dns.html#dnslookuphostname-options-callback

Description of the problem

I think the docs are potentially misleading on the family = 0 dns lookup option.

Currently, the docs read: "The value 0 indicates that IPv4 and IPv6 addresses are both returned."

I initially interpreted this as "[NOT CORRECT] setting the family value to 0 and all to true will mean on a system that supports both IPv4 and IPv6, an IPv4 and IPv6 address will be returned"

However, what I think this actually means is "setting the family value to 0 and all to true will mean on a system that supports both IPv4 and IPv6, either an IPv4 or IPv6 address, or both will be returned - depending on the underlying DNS implementation"

As a short example demoing this behaviour (at least on Ubuntu 22.04.3, running Node v20.11.0):

require('node:dns').lookup('localhost', { all: true, family: 0 }, console.log)
// null [ { address: '127.0.0.1', family: 4 } ]

require('node:dns').lookup('localhost', { all: true, family: 4 }, console.log)
// null [ { address: '127.0.0.1', family: 4 } ]

require('node:dns').lookup('localhost', { all: true, family: 6 }, console.log)
// null [ { address: '::1', family: 6 } ]

Meaning if a user really does want both (rather than just accepts either), they should do two calls: one for IPv4 and one for IPv6 separately.

Some evidence for it being potentially confusing:

Also see #28159.

@domdomegg domdomegg added the doc Issues and PRs related to the documentations. label Jan 16, 2024
@domdomegg
Copy link
Contributor Author

The alternative angle is to view this as a potential feature for the dns module - where the behaviour of dns.lookup('localhost', { all: true, family: 0 }, console.log) is standardised across platforms to produce IPv4 and IPv6 results when available. Especially because even if documented, people still may depend on this behaviour because it works on some (most?) machines.

@duncanchiu409
Copy link
Contributor

duncanchiu409 commented Jan 16, 2024

The current implementation is "setting the family value to 0 and all to true will mean on a system that supports both IPv4 and IPv6, either an IPv4 or IPv6 address". If I am not wrong. :)

aduh95 pushed a commit that referenced this issue Jan 30, 2024
PR-URL: #51517
Fixes: #51482
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
domdomegg added a commit to domdomegg/node that referenced this issue Feb 3, 2024
rdw-msft pushed a commit to rdw-msft/node that referenced this issue Feb 9, 2024
PR-URL: nodejs#51517
Fixes: nodejs#51482
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
targos pushed a commit that referenced this issue Feb 15, 2024
PR-URL: #51517
Fixes: #51482
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this issue Feb 19, 2024
PR-URL: nodejs#51517
Fixes: nodejs#51482
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
richardlau pushed a commit that referenced this issue Mar 25, 2024
PR-URL: #51517
Fixes: #51482
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
richardlau pushed a commit that referenced this issue Mar 25, 2024
PR-URL: #51517
Fixes: #51482
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
aduh95 pushed a commit to domdomegg/node that referenced this issue May 11, 2024
aduh95 pushed a commit that referenced this issue May 11, 2024
PR-URL: #51653
Fixes: #51482
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this issue May 12, 2024
PR-URL: #51653
Fixes: #51482
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
marco-ippolito pushed a commit that referenced this issue Jun 17, 2024
PR-URL: #51653
Fixes: #51482
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
marco-ippolito pushed a commit that referenced this issue Jun 17, 2024
PR-URL: #51653
Fixes: #51482
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
marco-ippolito pushed a commit that referenced this issue Jun 17, 2024
PR-URL: #51653
Fixes: #51482
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
EliphazBouye pushed a commit to EliphazBouye/node that referenced this issue Jun 20, 2024
PR-URL: nodejs#51653
Fixes: nodejs#51482
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
bmeck pushed a commit to bmeck/node that referenced this issue Jun 22, 2024
PR-URL: nodejs#51653
Fixes: nodejs#51482
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
2 participants