Add dns.resolve that returns all results #2848

skeggse · 2015-09-13T21:27:01Z

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?

The text was updated successfully, but these errors were encountered:

silverwind · 2015-09-14T05:15:07Z

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

skeggse · 2015-09-14T13:36:14Z

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 · 2015-09-14T15:18:27Z

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

skeggse · 2015-09-14T15:19:56Z

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 · 2015-09-14T15:29:19Z

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

skeggse · 2015-09-14T15:31:33Z

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

mscdex · 2015-09-14T16:06:18Z

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 · 2015-09-24T19:25:01Z

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 · 2015-09-24T19:31:12Z

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 · 2015-09-25T02:26:23Z

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

kulkarniankita · 2015-10-03T00:40:47Z

Working on this one now. @jasnell @mhdawson

bnoordhuis · 2015-10-03T10:51:30Z

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 · 2015-10-03T11:23:17Z

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

kulkarniankita · 2015-10-06T01:29:07Z

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 · 2015-10-06T15:18:40Z

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

silverwind · 2015-10-06T15:19:36Z

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 · 2015-10-06T15:25:06Z

@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 · 2015-10-06T15:42:52Z

@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 · 2015-10-20T02:53:57Z

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

kulkarniankita · 2015-10-21T18:32:17Z

@silverwind yes I am working on this

skeggse · 2016-01-06T00:03:22Z

How's your progress on this, @kulkarniankita?

kulkarniankita · 2016-01-06T21:44:10Z

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 · 2016-01-09T06:32:26Z

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

kulkarniankita · 2016-01-13T15:41:37Z

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

silverwind · 2016-01-13T16:46:39Z

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 · 2016-01-13T17:09:19Z

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 · 2016-01-24T21:42:01Z

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

A Send function which calls ares_query with the proper type (in this case it would be ns_t_any).
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 · 2016-01-25T16:01:55Z

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 · 2016-01-25T16:44:43Z

@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 · 2016-01-25T16:56:35Z

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 😛)

skeggse · 2016-04-06T22:47:44Z

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 · 2016-04-08T16:36:02Z

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

bj7 · 2016-06-08T00:11:21Z

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

silverwind · 2016-06-08T07:15:53Z

Sure, go ahead!

bj7 · 2016-06-14T00:41:21Z

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 · 2016-06-18T19:25:25Z

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 · 2016-08-06T12:42:16Z

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

XadillaX · 2017-05-21T10:35:21Z

@silverwind I'm working on it now at #13137.

`dns.resolveAny` and `dns.resolve` with `"ANY"` has the similar behavior like `$ dig <domain> any` and returns an array with several types of records. `dns.resolveAny` parses the result packet by several rules in turn. Supported types: * A * AAAA * CNAME * MX * NAPTR * NS * PTR * SOA * SRV * TXT Refs: nodejs#2848

`dns.resolveAny` and `dns.resolve` with `"ANY"` has the similar behavior like `$ dig <domain> any` and returns an array with several types of records. `dns.resolveAny` parses the result packet by several rules in turn. Supported types: * A * AAAA * CNAME * MX * NAPTR * NS * PTR * SOA * SRV * TXT Fixes: #2848 PR-URL: #13137 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Roman Reiss <[email protected]>

brendanashworth added dns Issues and PRs related to the dns subsystem. feature request Issues that request new features to be added to Node.js. labels Sep 13, 2015

skeggse mentioned this issue Sep 14, 2015

Explore ANY DNS Query skeggse/isemail#5

Closed

silverwind added the good first issue Issues that are suitable for first-time contributors. label Sep 24, 2015

XadillaX mentioned this issue May 21, 2017

dns: add resolveAny support #13137

Closed

14 tasks

addaleax closed this as completed in 27de369 Jun 9, 2017

Add dns.resolve that returns all results #2848

Add dns.resolve that returns all results #2848

Comments

skeggse commented Sep 13, 2015

silverwind commented Sep 14, 2015

skeggse commented Sep 14, 2015

silverwind commented Sep 14, 2015

skeggse commented Sep 14, 2015

silverwind commented Sep 14, 2015

skeggse commented Sep 14, 2015

mscdex commented Sep 14, 2015

silverwind commented Sep 24, 2015

silverwind commented Sep 24, 2015

kulkarniankita commented Sep 25, 2015

kulkarniankita commented Oct 3, 2015

bnoordhuis commented Oct 3, 2015

silverwind commented Oct 3, 2015

kulkarniankita commented Oct 6, 2015

bnoordhuis commented Oct 6, 2015

silverwind commented Oct 6, 2015

kulkarniankita commented Oct 6, 2015

silverwind commented Oct 6, 2015

silverwind commented Oct 20, 2015

kulkarniankita commented Oct 21, 2015

skeggse commented Jan 6, 2016

kulkarniankita commented Jan 6, 2016

silverwind commented Jan 9, 2016

kulkarniankita commented Jan 13, 2016

silverwind commented Jan 13, 2016

silverwind commented Jan 13, 2016

Chris911 commented Jan 24, 2016

Chris911 commented Jan 25, 2016

silverwind commented Jan 25, 2016

Chris911 commented Jan 25, 2016

skeggse commented Apr 6, 2016

silverwind commented Apr 8, 2016

bj7 commented Jun 8, 2016

silverwind commented Jun 8, 2016

bj7 commented Jun 14, 2016

bj7 commented Jun 18, 2016 • edited Loading

semaja2 commented Aug 6, 2016

XadillaX commented May 21, 2017

bj7 commented Jun 18, 2016 •

edited

Loading