order of getaddrinfo results not respected

Version: v5.10.1
Platform: Linux archer 4.4.5-1-ARCH deps: update openssl to 1.0.1j #1 SMP PREEMPT Thu Mar 10 07:38:19 CET 2016 x86_64 GNU/Linux
Subsystem: dns/cares_wrap

The results returned by getaddrinfo are reordered, giving preference to ipv4 addresses. The explanation given in #4693 seems unsatisfactory. Not implementing happy eyeballs is a reasonable design decision, of course, but why does that mean ipv4? It should mean taking the first entry. At the moment node is deliberately defying the system configuration.

@bnoordhuis

As I understand it, without the reordering, ipv6 addresses could be the first entry, which would break anywhere that doesn't have an ipv6 stack. The current implementation is intentional in order to give preference to ipv4. /cc @bnoordhuis

Is this still a problem when using the latest v6 release candidate (aka is this fixed by #6021)?

The DNS hints aren't related to the reordering.

@bnoordhuis

@bnoordhuis would the dns.ADDRCONFIG flag mitigate the problem?

As I understand it, the proper place to configure preferences is via gai.conf.

Snippet from my gai.conf:

# precedence  <mask>   <value>
#
#    Defaults:
#
#precedence  ::1/128       50
#precedence  ::/0          40
#precedence  2002::/16     30
#precedence ::/96          20
#precedence ::ffff:0:0/96  10
#
#    For sites which prefer IPv4 connections change the last line to
#
#precedence ::ffff:0:0/96  100

would the dns.ADDRCONFIG flag mitigate the problem?

Only partially. AI_ADDRCONFIG means 'return AAAA entries only when there is a configured IPv6 interface' but the presence of such an interface doesn't say anything about IPv6 traffic actually being routable. (That's true for IPv4 as well, of course; my point is that AI_ADDRCONFIG isn't a fix per se.)

We made the decision to prefer IPv4 over IPv6 three or four years ago when IPv6 support was much more spotty than it is today. It's something we can reconsider if the majority thinks it's a good idea.

There are likely two things that can happen here:

We can improve the documentation to indicate why the results are ordered the way they are, and
We can add an option that would tell the impl not to re-order the output. The current default behavior would remain.

This is clearly a violation of the principle of least astonishment and I don't see how adding documentation is going to help. Hardly anyone affected by this will be using the dns module directly. It took me half a day before realizing my issue was with the dns module. Since then I have put a workaround in place and am no longer affected. Still, I think you should slowly phase out this odd logic. IMHO, over time, keeping it will hurt more than it helps.

from what i gather, this is possibly the source of my issue with npm install failing (a possible regression of npm/npm#6857)

This time i'm running alpine (hence, muslc) under docker, again, IPv6-only network.

~ # node -e "dns.lookup('registry.npmjs.org', (...args) => console.log(args))"
[ null, '151.101.112.162', 4 ]
~ #

note that musl implements rfc 3484/6724.

this works fine in ruby, btw:

# pry -r 'open-uri'
[1] pry(main)> open("https://registry.npmjs.org/")
=> #<StringIO:0x0055ecc25d4fd0
 @base_uri=#<URI::HTTPS https://registry.npmjs.org/>,
 @meta=
  {"server"=>"CouchDB/1.5.0 (Erlang OTP/R16B03)",
   "content-type"=>"application/json",
   "cache-control"=>"max-age=300",
   "content-length"=>"194",
   "accept-ranges"=>"bytes",
…etc…

from what i gather, this can be fixed with:

diff --git a/lib/dns.js b/lib/dns.js
index cbb994b8f2..c161a89a66 100644
--- a/lib/dns.js
+++ b/lib/dns.js
@@ -140,7 +140,7 @@ exports.lookup = function lookup(hostname, options, callback) {
     if (all) {
       callback(null, []);
     } else {
-      callback(null, null, family === 6 ? 6 : 4);
+      callback(null, null, family === 4 ? 4 : 6);
     }
     return {};
   }

That fix would default the family to 6 iff the caller isn't asking for all families. However, most of the time the caller should be asking for all families. Even if that was not the case, changing defaults is a mean thing to do to clients.

Generally any caller should ask for all families and then try the results in-order. If the caller were to implement happy eyeballs they would ask for all and concurrently try the per-familiy subsequences in-order.

IMHO the proper way to fix this is to apply #4693. Whether to happy eyeball or not is orthogonal.

Happy Eyeballs - for those who haven't heard the term

@jasnell

every month or two, i try a fresh major release of nodejs, only to find this issue is still there.

(@jasnell commented on 22 Apr 2016) There are likely two things that can happen here:

We can improve the documentation to indicate why the results are ordered the way they are, and

We can add an option that would tell the impl not to re-order the output. The current default behavior would remain.

i highly agree with @benschulz here:

(@benschulz commented on 25 Apr 2016 • edited) This is clearly a violation of the principle of least astonishment and I don't see how adding documentation is going to help. Hardly anyone affected by this will be using the dns module directly. It took me half a day before realizing my issue was with the dns module. Since then I have put a workaround in place and am no longer affected. Still, I think you should slowly phase out this odd logic. IMHO, over time, keeping it will hurt more than it helps.

and believe that #4693 is the correct way forward — in 7.x+

Should this remain open? (I'm guessing the answer is "yes" but would be happy to find out I'm wrong.)

This is your project and you are free to take it in any direction you please. If you think the status quo is good enough or even right, you should close the ticket. All I can tell you is that I had a really rough time with this. But that's just one data point, I imagine there are several more people affected.
Fermi estimate: Two people have raised the issue; if we conservatively posit that .1‒1% of those affected manage to figure out the root cause and find this report, it'd be around 200‒2000 people affected in total.

Hope that helps. :P

Edit: Added missing "affected" qualifier. =)

For all interested parties, I have implemented happy eyeballs for my company, Balena, here:

https://github.com/balena-io-modules/fetch

The code is open source, so if anyone wants to adapt it to their needs, please feel free.

If anyone would like me to make a PR to node with the code adapted for nodejs core, please let me know and how exactly it should be integrated.

As IPv4 addresses are exhausted, this will be more and more of a problem, especially on mobile networks, and especially in developing countries with large populations.

It's probably best to go ahead and prepare for it now if we can!

Ok, I have created another repo which implements "happy eyeballs" and will patch the http(s).Agents to default to the happy eyeballs algorithm:

https://www.npmjs.com/package/happy-eyeballs

With this package you can just add

import 'happy-eyeballs/eye-patch'

to the top of your file to default all createConnection calls to use happy eyeballs.

I will be making a PR to NodeJS core as well.

@zwhitchcox

That is amazing to see, thanks a lot, @zwhitchcox!

Navigation Menu

Repository navigation

order of getaddrinfo results not respected #6307

77 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

order of getaddrinfo results not respected #6307

Description

Activity

jasnell commented on Apr 20, 2016

addaleax commented on Apr 20, 2016

cjihrig commented on Apr 20, 2016

cjihrig commented on Apr 20, 2016

benschulz commented on Apr 20, 2016

bnoordhuis commented on Apr 20, 2016

jasnell commented on Apr 22, 2016

benschulz commented on Apr 25, 2016

igalic commented on Dec 23, 2016

benschulz commented on Dec 27, 2016

sam-github commented on Dec 29, 2016

igalic commented on Feb 3, 2017

Trott commented on Jul 16, 2017

benschulz commented on Jul 16, 2017

77 remaining items

zwhitchcox commented on Sep 26, 2021

zwhitchcox commented on Oct 1, 2021

telmich commented on Oct 1, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions