Skip to content

Conversation

Garionion
Copy link
Contributor

@Garionion Garionion commented Jul 14, 2023

This PR adds the ability to restrict the default Resolver to IPv4 or IPv6 only. Its useful when the targets are only reachable by e.g. IPv4 anyway and one wants to reduce the load on the DNS Server

dnscache.go Outdated

func (d *defaultResolverWithTrace) LookupHost(ctx context.Context, host string) (addrs []string, err error) {
// `net.Resolver#LookupHost` does not cause invocation of `net.Resolver#lookupIPAddr`, therefore the `DNSStart` and `DNSDone` tracing hooks
// built into the stdlib are never called. `LookupIP`, despite it's name, can also be used to lookup a hostname but does cause these hooks to be
// triggered. The format of the reponse is different, therefore it needs this thin wrapper converting it.
rawIPs, err := net.DefaultResolver.LookupIP(ctx, "ip", host)
rawIPs, err := net.DefaultResolver.LookupIP(ctx, d.ipVersion, host)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to handle the case where d.ipVersion is "".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would argue that this is not necessary as the only way to use this defaultResolverWithTrace (from outside this lib) is by either using the defaultResolver or by retreiving a defaultResolverWithTrace from the functions NewResolverOnlyV4 and NewResolverOnlyV6. In all 3 cases there is alreade the ipVersion set.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure but code evolve over time and this situation could change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok i've added that if ipVersion is not ip, ip4 or ip6 then ip will be used

@rs rs merged commit fc85eb6 into rs:master Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants