Node: Add dns.resolve that returns all results

[skeggse]

Similar to #736, it'd be nice if we had a dns.resolve that queries for all results.

Would the Node core maintainers be amenable to a pr that adds this functionality via c-ares, or should such an addition be delayed until after #1013 has been resolved?

[silverwind]

resolve already returns an array of results, what's exactly the issue?

[skeggse]

Unless I'm mistaken, resolve returns an array of results for one record type (like A, AAAA, CNAME, MX, etc). I'd like to be able to do a wildcard query, supported by c-ares with the ns_t_any type.

[silverwind]

Oh, you're talking about the ANY query. Well, that'd be indeed something we should support imho.

[skeggse]

Thoughts on whether we should wait for #1013? Or should I put together a pr?

Edit: er... not sure how #1843 fits into this

[silverwind]

Personally, I'd rather see #1843 reviewed, fixed and shipped. @mscdex's call if we should further extend c-ares now.

[skeggse]

Sounds good. This isn't super critical for me atm.

[mscdex]

Work is still ongoing with the js dns resolver. If it's a simple change and someone wants to add it for c-ares in the meantime, feel free to do so.

[silverwind]

This shouldn't be too hard to add. dns.resolve should take a new rrtype ANY, and we likely want a resolveAny method as well.

[silverwind]

On the other hand, there is the issue that we need to return the rrtype of the various records to the user, but we can't really change the addresses array on dns.resolve to something else. Maybe this special query should live only as resolveAny where we could return an array of objects like

[
  {rrtype: 'A', record: 'something.com'},
  {rrtype: 'MX', record: 'mx.something.com'},
]

[kulkarniankita]

Is anyone working on adding this? If not, I can work on this. (cc: @jasnell, @silverwind )

[kulkarniankita]

Working on this one now. @jasnell @mhdawson

[bnoordhuis]

On the other hand, there is the issue that we need to return the rrtype of the various records to the user, but we can't really change the addresses array on dns.resolve to something else.

Correct me if I'm wrong but dns.resolve() already returns differently shaped arrays for different query types, doesn't it? Having ANY records have a different shape doesn't look like an issue to me.

$ node -e 'require("dns").resolve("google.com", "A", console.log)'
null [ '74.125.136.138',
  '74.125.136.113',
  '74.125.136.139',
  '74.125.136.101',
  '74.125.136.102',
  '74.125.136.100' ]

$ node -e 'require("dns").resolve("google.com", "MX", console.log)'
null [ { exchange: 'alt1.aspmx.l.google.com', priority: 20 },
  { exchange: 'aspmx.l.google.com', priority: 10 },
  { exchange: 'alt3.aspmx.l.google.com', priority: 40 },
  { exchange: 'alt2.aspmx.l.google.com', priority: 30 },
  { exchange: 'alt4.aspmx.l.google.com', priority: 50 } ]

[silverwind]

@bnoordhuis you're correct. Won't be an issue for this addition. I was wrongly assuming we always return arrays for resolve.

[kulkarniankita]

So basically this 'resolveAny' query should return this right
$ node -e 'require("dns").resolve("google.com", "ANY", console.log)'
Would it look like this:

[ '74.125.136.138',
  '74.125.136.113'], 
[ { exchange: 'alt1.aspmx.l.google.com', priority: 20 },
  { exchange: 'aspmx.l.google.com', priority: 10 }] ,[], ...

Does above example look right? Can you please provide a example if it needs correction? @silverwind @bnoordhuis

[bnoordhuis]

I'd make every entry an object with at least a record type field.

[silverwind]

ANY (rrtype 255) is kind of a special. It can return pretty much any record, as seen by this dig:

$ dig google.com any

; <<>> DiG 9.8.3-P1 <<>> google.com any
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 22716
;; flags: qr rd ra; QUERY: 1, ANSWER: 20, AUTHORITY: 0, ADDITIONAL: 0

;; QUESTION SECTION:
;google.com.            IN  ANY

;; ANSWER SECTION:
google.com.     299 IN  A   188.21.9.55
google.com.     299 IN  A   188.21.9.58
google.com.     299 IN  A   188.21.9.54
google.com.     299 IN  A   188.21.9.53
google.com.     299 IN  A   188.21.9.57
google.com.     299 IN  A   188.21.9.52
google.com.     299 IN  A   188.21.9.59
google.com.     299 IN  A   188.21.9.56
google.com.     3599    IN  TXT "v=spf1 include:_spf.google.com ~all"
google.com.     59  IN  SOA ns2.google.com. dns-admin.google.com. 104759467 900 900 1800 60
google.com.     599 IN  MX  50 alt4.aspmx.l.google.com.
google.com.     21599   IN  NS  ns2.google.com.
google.com.     21599   IN  NS  ns4.google.com.
google.com.     21599   IN  NS  ns1.google.com.
google.com.     599 IN  MX  30 alt2.aspmx.l.google.com.
google.com.     21599   IN  NS  ns3.google.com.
google.com.     599 IN  MX  20 alt1.aspmx.l.google.com.
google.com.     599 IN  MX  10 aspmx.l.google.com.
google.com.     21599   IN  TYPE257 \# 19 0005697373756573796D616E7465632E636F6D
google.com.     599 IN  MX  40 alt3.aspmx.l.google.com.

I'd do something like this:

[
 {type: 'A', value: '188.21.9.55'},
 {type: 'A', value: '188.21.9.58'},
 {type: 'TXT', value: 'v=spf1 include:_spf.google.com ~all'},
 {type: 'MX', value: 'aspmx.l.google.com', priority: '10'}
]

Maybe you can lean on existing property names, but generally I'd follow a scheme like @bnoordhuis mentioned, with type being a mandatory field, maybe value too.

[kulkarniankita]

@silverwind that example helps. I will follow this pattern as it makes sense:

[
 {type: 'A', value: '188.21.9.55'},
 {type: 'A', value: '188.21.9.58'}, ...
]

[silverwind]

@kulkarniankita: for things like SOA,SRV, MX and other multi-value records, I suppose we could return them like resolveSoa and relatives do, but with the type field included:

{ type: 'SOA',
  nsname: 'ns3.google.com',
  hostmaster: 'dns-admin.google.com',
  serial: 104759467,
  refresh: 900,
  retry: 900,
  expire: 1800,
  minttl: 60 }

[silverwind]

@kulkarniankita are you working on this? If not, I'd be interested in implementing it myself.

[kulkarniankita]

@silverwind yes I am working on this

[skeggse]

How's your progress on this, @kulkarniankita?

[kulkarniankita]

Hi, I was stuck for a while on this one. @jasnell is helping answer some questions. I should be able to get it done sooner now. Thanks.

[silverwind]

I feel guity for labeling this a good first contribution, sorry for the trouble. :sweat_smile:

[kulkarniankita]

@silverwind Nevertheless, good learning experience! Can I request some mentoring from you @silverwind

[silverwind]

Let's see, the addition should be similar to when the last two query types were added which according to git blame are:

• NAPTR
• SOA

So the main challenge here will be the C++ function queryAll and I must admit C++ is not my strong point. ares_query takes a class and a type argument, which according the RFC 1035 are 1 and 255 respectively for the * aka ALL query:

3.2.4. CLASS values

IN              1 the Internet

3.2.3. QTYPE values

*               255 A request for all records

QTYPE in this regard is just a special TYPE. I hope this helps, maybe someone more adept on C++ can chime in on what should be heeded there.

[silverwind]

Also I just found out there's an * class too (in addition to the * type), you can try it like this:

$ dig google.com -c any -t any

For 99.99% of purposes, we can assume IN will be the requested class, thought :)

[Chris911]

As far as I can tell the c-ares wrappers are pretty simple;

1. A Send function which calls ares_query with the proper type (in this case it would be ns_t_any).
2. A Parse function which parses the DNS response. Currently protected.

However, looking at the documentation for c-ares there's no parser for for the ANY request. After working the code a little bit it looks like from the response from the ANY request works with all (or most) of the available parsers. So technically this feature could be implemented by duplicating the parsing logic of each record type wrapper in the newly created ANY query wrapper.

Is this the way to do or should the the wrappers be refactored to expose their Parse method so it can be re-used in the ANY wrapper.

cc: @silverwind

[Chris911]

By the way @silverwind if you end up starting the implementation let me know I'd like to help/discuss. I'll probably be on IRC.

[silverwind]

@Chris911 I'd say re-using code where possible sounds like a good idea. The returned records should be as close as possible in formatting to the current methods for each record type. I'd say if @kulkarniankita is okay with it, feel free to take a shot at an implementation. I'm very rarely on IRC, so would prefer to discuss things here on GitHub.

Oh, and I have to mention that we have #1843 queued up which will likely replace c-ares in the long run, but last I've heard there are still some performance issues with the JS DNS. The motivations for it were because c-ares is plagued by an unresolved a crash issue and generally looks to be on the unmaintained side.

[Chris911]

Alright. I started working on the implementation and have something working but there's a lot of duplicated code. This would require a big refactor to share the parse functions. I saw #1843 the other and might skip the C++ refactor as it might be replaced shortly.

Thanks for your time. Will keep looking for good first contributions (don't hesitate to ping me if you see something :stuck_out_tongue:)

[skeggse]

Hm this issue has been bounced around a lot. Are any of you still working on this, and is any of that work public?

[silverwind]

Doesn't look like anyone is actively working on this. If anyone wants to take it, let us know.

[bj7]

Hello @silverwind, is this still a good contribution? I'm interested in getting involved with Node in my spare time. Thanks

[silverwind]

Sure, go ahead!

[bj7]

Just FYI, I am still working on this; I just don't always have a lot of time due to my day job so it may take me a minute to come up to speed.

[bj7]

I'm going through and reading the nodejs docs on dns, previous commits for soa and naptr, the c-ares docs, etc. Are there any other docs I should also take a look at as I work this (or helpful blogs, etc)? Thanks!

[semaja2]

I agree the ANY type record is a must, as you may not know if a record is a CNAME or A

[XadillaX]

@silverwind I'm working on it now at https://github.com/nodejs/node/issues/13137.