feat(unstable): add Deno.resolveDns API

[magurotuna]

This PR adds new ops, op_resolve_addr_sync and op_resolve_addr_async which do DNS resolution, and exposes them as Deno.resolveAddrSync and Deno.resolveAddr respectively.
It comes from @ry's comment: #8743 (review), and also resolves #6110.

For manual testing, I prepared a script like:

const a = await Deno.resolveAddr("example.com", 80);
const b = Deno.resolveAddrSync("example.com", 53);
console.log(a, b);

then ran it via cargo run -- run --allow-net foo.ts, I got:

[ "[2606:2800:220:1:248:1893:25c8:1946]:80", "93.184.216.34:80" ] [ "[2606:2800:220:1:248:1893:25c8:1946]:53", "93.184.216.34:53" ]

Seeing this result, I wonder if we need really to pass a port number to Deno.resolveAddr. As far as I understand, domain name resolution itself has nothing to do with a port number, but std::net::ToSocketAddr and tokio::net::lookup_host require a port number as well as hostname.
If Deno.resolveAddr and Deno.resolveAddrSync should be specialized in resolving domain name, it would be nice for the functions not to receive port numbers, and to return resolved addresses with port numbers truncated.

[bartlomieju]

CC @mrkurt thoughts?

[magurotuna]

There's a widely used dns resolver crate: https://docs.rs/trust-dns-resolver/0.19.6/trust_dns_resolver/
It might be better to use it.

[mrkurt]

I think maybe the resolve_addr Deno already has is wrong for this. That's meant to turn a host:port address into something useful for listen/dial. The resolution happens under the covers, but the result isn't really a DNS response (ipv6 addresses wouldn't normally be in [] for example).

What I'd like is the equivalent of this dig command:

dig aaaa nrt.curl.internal @10.0.0.1 +short

That queries AAAA records on the 10.0.0.1 DNS server records for nrt.curl.internal. If the underlying op can accept record type, name, and DNS server addresses, then return the list of results, I think the TypeScript API can do most of the work with just one op.

[magurotuna]

@mrkurt
I see, then it seems like we should utilize trust_dns_resolver crate instead. I'll give it a shot.

[bartlomieju]

@magurotuna looks great! Could we get some integration tests for this feature?

@mrkurt thanks for the review, if you have any suggestions regarding missing functionality that would be helpful (it seems you'll be the first user of this API 😄 )

[magurotuna]

I added an integration test named _079_resolve_dns , and now Deno.resolveDns returns always an array of string (string[]) regardless of the record types. The problem I've found debatable is how to stringify respective record types. For now, the returned values look like:

deno/cli/tests/079_resolve_dns.ts.out

Lines 2 to 9 in 4b8f870

	A: [ "1.2.3.4" ]
	AAAA: [ "1:2:3:4:5:6:7:8" ]
	ANAME: [ "aname.com." ]
	CNAME: [ "cname.com." ]
	MX: [ "0 mx.com." ]
	PTR: [ "ptr.com." ]
	SRV: [ "0 100 1234 srv.com." ]
	TXT: [ "foo bar" ]

In particular, MX and SRV records are stringified unkindly; 0 mx.com. means "priority is 0, exchange is mx.com", and 0 100 1234 srv.com. means "priority is 0, weight is 100, port is 1234, name is srv.com".
Just because I didn't come up with any suitable form for them, the data of MX and SRV records are stringified like this.

In addition, as to the TXT records, ideally they are represented as string[][] (actually Node.js does) but in order to make it string[], they get flattened by placing whitespace between each.

Lastly, should trailing periods in ANAME, CNAME, MX, PTR, and SRV be removed? 🤔

[ry]

@magurotuna Thank you for doing this work to return strings. Now that I see this, it seems less preferable to the original version with interface for each DNS record, because of the difficult to_string implementations in SRV and PTR . I apologize for the extra work and suggesting the wrong direction.

[ry]

This is very awesome, but I think it should be done in cli/tests/integration_tests.rs since it's only used in a single test.

[magurotuna]

but I think it should be done in cli/tests/integration_tests.rs since it's only used in a single test.

Makes sense, but when I tried to move run_dns_server to integration_tests.rs, I ran into a pretty weird error, which indicated that DNS resolution was timed out.
I'm struggling to figure out why this error happened. For now, it has turned out that this snippet does work (note that it's in the main function), however, if I put exactly same stuff into a test (annotated with #[tokio::test]), it does NOT work somehow.
(in this snippet I use dig command instead of TypeScript code. But if I switch to TypeScript code, timeout error still happens)

I think Deno's next release is coming nearly, and it looks like this error will take me some time to solve. So I'd like to suggest putting off this refacroting after the release - if this is fine, I'll fix the return type of Deno.resolveDns and adjust the test case to it as soon as possible.

[magurotuna]

Actually I've figured out why, so I'm doing this refactoring right now.

[magurotuna]

I think it's ready to land. Please take a look :-)

[lucacasonato]

Tested this locally, and works as expected. Great work @magurotuna. I'll leave the final review to @ry.

[ry]

LGTM - very nice work @magurotuna !

addressed