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

[RFC] API design for IPv6 and more types of records #43

Closed
WyriHaximus opened this issue Dec 27, 2016 · 3 comments · Fixed by #110
Closed

[RFC] API design for IPv6 and more types of records #43

WyriHaximus opened this issue Dec 27, 2016 · 3 comments · Fixed by #110

Comments

@WyriHaximus
Copy link
Member

WyriHaximus commented Dec 27, 2016

Currently this package only supports A and CNAME records in it's implementation and after a quick chat with @clue earlier today we decided we need to come up with a good way support IPv6 and more record types. Here is a quick hack up to get the discussion started:

$resolver = new Resolver('8.8.8.8', Options::PREFER_IPV6);
// Tries to resolve AAAA (IPv6) record first and then tries the A (IPv4) record.
$resolver->resolve('wyrihaximus.net');
$resolver = new Resolver('8.8.8.8');
// Resolves to NS record
$resolver->resolve('wyrihaximus.net', RecordType::NS);
@clue
Copy link
Member

clue commented Sep 12, 2017

I think you're raising a very relevant issue here 👍

Currently, the Resolver only resolves hostnames to IPv4 addresses (A and CNAME), while the ExecutorInterface allows you to execute arbitrary queries. As such, I believe this issue to be twofold:

How can we configure the Resolver to resolve to IPv6 addresses? How can I explicitly set this and eventually how can this be determined automatically? Is IPv6 preferred over IPv4 and are even both supported? The most common usage of this is likely with the higher-level Connector, which is why I've justed created reactphp/socket#116.

Other than the default resolve() method, there's also the question on how to execute "special queries", such as MX or SRV. These do not really fit with the existing resolve() interface which only resolve with one random record as a result, while MX and SRV actually need access to all results to implement a fallback strategy (such as trying highest priority first etc.). As such, I believe it's probably best to either add a new API for these more advanced queries or maybe even keep this within the ExecutorInterface only.

I'm curious what others think about this, what other projects offer similar APIs? 👍

@kelunik
Copy link

kelunik commented Sep 12, 2017

I'd also be interested in how to determine IPv6 support / route-ability for amphp/socket#35.

For the second issue I'd recommend to have separate query and resolve methods as amphp/dns has and as the DNS RFC suggests. Multiple records might also be useful for A / AAAA with a retry mechanism in the Connector.

I'd probably remove either Resolver or Executor, but there's no definite need for it. Doesn't really matter whether they're two methods or two classes where one uses the other.

@clue clue added this to the v0.4.16 milestone Jun 28, 2018
@clue
Copy link
Member

clue commented Jun 28, 2018

@clue How can we configure the Resolver to resolve to IPv6 addresses? How can I explicitly set this and eventually how can this be determined automatically? Is IPv6 preferred over IPv4 and are even both supported? The most common usage of this is likely with the higher-level Connector, which is why I've justed created reactphp/socket#116.

Other than the default resolve() method, there's also the question on how to execute "special queries", such as MX or SRV. These do not really fit with the existing resolve() interface which only resolve with one random record as a result, while MX and SRV actually need access to all results to implement a fallback strategy (such as trying highest priority first etc.). As such, I believe it's probably best to either add a new API for these more advanced queries or maybe even keep this within the ExecutorInterface only.

We now have support for the most relevant resource record types (RRs) such as A, AAAA, MX, TXT, SRV etc. (see #31).

The current $resolver->resolve($host) is limited to IPv4, only looks up A records (and obeys CNAME records accordingly) and only returns one random record value. As such, this is pretty much the async equivalent of the (mostly deprecated) gethostbyname() function.

It is my understanding that we should add a new method which supports passing an explicit record type (such as AAAA) and return the full array of all record values. I would suggest adding a new $resolver->resolveAll($host, $type) method (very much similar to what @WyriHaximus suggested in #67). As such, this would be pretty much the async equivalent of the more modern getaddrinfo() function (see also https://www.akkadia.org/drepper/userapi-ipv6.html) or PHP's dns_get_record().

For the reference: Node.js uses dedicated method names (such as resolveA(), resolve6(), etc.) instead of accepting a type parameter. Given our current APIs, I would suggest using a single new method which requires a type parameter instead of adding new methods for each type (new record record types are still added occasionally). Similarly, this method should only return the record values, for example the IP for A/AAAA queries and a structure with priority and target for MX queries. Its query type and meta data such as TTL should not be included. This also implies that it is not suited for ANY (also known as * wildcard) queries as results would be rather meaningless without knowing its resource type. As such, I would suggest rejecting ANY queries and instead looking into a dedicated resolveAny() method eventually which should include at least the answer type and value (arguably out of scope in this discussion).

Once the resolveAll() method is in, we can explicitly query IPv6 (AAAA) or IPv4 (A) record values. It is my understanding that this library SHOULD NOT provide a way to automatically decide which type of query it should send. Instead, it should be left up to higher-level consumers of this API to query the appropriate type. For our Socket component, this means that it will likely start querying both types simultaneously and then try both address types following the "happy eyeballs algorithm" (see reactphp/socket#116 for more details).

Once this is supported in our Socket component, we may want to add some configuration options to disable either protocol type (there's no reliable way to detect IPv6 support) and also deprecate the then unused $resolver->resolve($host) method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants