dns: add getServers and setServers

[tjfontaine]

getServers returns an array of ips that are currently being used for
resolution

setServers takes an array of ips that are to be used for resolution,
this will throw if there's invalid input but preserve the original
configuration

[bnoordhuis]

s/ips/IP addresses/ ?

[tjfontaine]

I have pushed the changes for the lint fixes. I also further extended the test to include setting IPv6 resolvers.

However, ares_set_servers_csv currently has broken IPv6 parsing, if the address includes '::' it thinks everything to the right of the first ':' will be the port.

So this PR is currently blocked, ares_set_servers_csv should be fixed, and I could also parse on my own and use ares_set_servers instead

[keithws]

I believe I fixed the bug in ares_set_servers_csv with this pull request.

[tjfontaine]

Thanks for looking into this, I've commented on that pullreq with another issue I found. I updated my current branch but haven't force pushed, I opted to do the parsing in js and the slightly safer ares_set_servers function

[bnoordhuis]

c-ares doesn't take pull requests, I believe. You have to git send-email it to the mailing list.

[tjfontaine]

@bnoordhuis I did push my new change, if you could please re-review I'd appreciate it.

[bnoordhuis]

Style: const void*

[tjfontaine]

@bnoordhuis updated, I also got rid of the superfluous memcpy since uv_inet_pton does that.

[bnoordhuis]

Check the return value. If return_value.code is not UV_OK, ip probably contains garbage.

[bnoordhuis]

A few more nits, otherwise LGTM.

[tjfontaine]

landed in 8886c6b

[jcspencer]

Is there a need to make these functions invoke callbacks, as getting the data synchronously isn't the best thing to do.

[tjfontaine]

I struggled with that decision, in fact in my original ticket #3132 that's what I had done, with an eye towards how I do it in my JS implementation of the dns module.

Ultimately c-ares initializes synchronously when node itself starts, so most of your file operations to determine the resolvers happen at startup, the call to ares_get_servers is only pulling from the state of our c-ares channel and not hitting disk or performing any other blocking call.

In any JS reimplementation that might happen in the future, it would also make sense to initialize synchronously (on the first require) as otherwise you end up having to queue any incoming requests while you wait for the platform to initialize.

[jcspencer]

Would it be better to add optional callbacks, that are called once the servers have been loaded/set? So you're not waiting for a variable to be initialised synchronously.

[tjfontaine]

Well it's not really an issue for c-ares as everything has already been initialized by the time you are executing javascript. And replacing c-ares with a pure javascript version is still a hypothetical situation for now.

Perhaps @bnoordhuis or @isaacs have an opinion on this

[jcspencer]

One possible option would be to compile c-ares into JavaScript with Emscripten. I'm not sure wether this would better or worsen performance, but either way, it's an option

[tjfontaine]

While I find emscripten interesting, it is certainly the wrong way to handle this particular situation. Especially as there are multiple DNS in javascript parsers, including one I've written. The question is if there's enough to gain/solve by using a javascript implementation to offset the overhead of owning the maintenance cost. The vast majority of requests are handled through dns.lookup which doesn't go through c-ares anyway.

[jcspencer]

Yeah, a pure javascript dns module would be great.