search

szmarczak / cacheable-lookup

A cacheable dns.lookup(…) that respects TTL :tada:
MIT License
193 stars 29 forks source link

Querying for family=6 when family=4 is explicitly requested? #52

Open brainsiq opened 2 years ago

brainsiq commented 2 years ago

Hi,

I'm trying to understand the design of the module and how it compares to the Node.js core DNS resolution. The Node.js docs state that family can be specified on a request:

family IP address family to use when resolving host or hostname. Valid values are 4 or 6. When unspecified, both IP v4 and v6 will be used.

If I dig into Node.js source code for HTTP requests it looks like this is indeed the behaviour (setting family on the request results in family being passed down into dns.lookup). If we are requesting private DNS records and know only v4 is available it seems to make sense to pass 4 here to avoid the ip v6 lookup being made.

When we use cacheable-lookup we see it requesting both ipv4 and ipv6 regardless of the requested family. Both results are cached and then the cached results are filtered to only return the requested family. This seems different to how a similar module does it (https://github.com/LCMApps/dns-lookup-cache/).

I'm by no means an expert on DNS resolution and best practise, but is this intentional? Is it a bad idea to specify the family on a request?

I can appreciate that this is likely a micro-optimisation and has a maintenance cost if the DNS server is later changed to update the family specified in code from 4 to 6. Saying that, the default configuration of the module is to use a pretty short cache TTL for ENODATA errors so it looks like we do get quite a lot of ipv6 lookups despite them being unsupported, and I'm unsure if increasing the error ttl is a good idea.

szmarczak commented 2 years ago

I'm by no means an expert on DNS resolution and best practise, but is this intentional? Is it a bad idea to specify the family on a request?

In most cases you want both 4 & 6. Indeed it makes an unnecessary load on the server, if using only one.

I'm unsure if increasing the error ttl is a good idea

It's for both 4 & 6, so definitely not a good idea.

I'll prepare a fix next week.

rasika-jay commented 2 years ago

Thanks, I too have this problem with private route53 addresses.

ThatOneCalculator commented 1 year ago

Any update? Having this problem as well.