Support resolving plain DNS PTR records #4921

dturing · 2016-01-27T23:42:28Z

Node's DNS module currently supports reverse IP lookup with dns.reverse(), which internally uses c-ares' gethostbyaddr(). It works great to look up the registered hostname for IP addresses ("Reverse DNS").

In DNS-SD (RFC6763), PTR records are used for listing service instance names for a domain (see Section 4.1), and there might be other uses of PTR records outside of Reverse DNS or DNS-SD.

c-ares supports querying and parsing PTR records, and ares_gethostbyaddr() uses ares_parse_ptr_reply internally. It's just not wrapped in the current version of node.

I took on the straightforward work of copying the glue code that wraps ares_query/ares_parse_srv_reply into dns.resolveSrv (SRV record lookup), and use ares_parse_ptr_reply to implement dns.resolvePtr, as seen in this pull request. It returns a simple array of strings containing the returned PTR records.

for example,

> dns.resolvePtr("8.8.8.8.in-addr.arpa",console.log);
null [ 'google-public-dns-a.google.com' ]

if i have registered a number of DNS-SD service instances, it can look like:

> dns.resolvePtr("_http._tcp.lan",console.log);
null [ 'Public_Message_Board._http._tcp.lan',
  'New_Employee_Information_Page._http._tcp.lan',
  'Company_Website._http._tcp.lan' ]

One caveat: this clears up the use of rrtype in dns.resolve() with rrtype="PTR", but changes the behaviour of resolve(hostname,"PTR",cb). Before, this was using dns.reverse(), so to lookup the hostname for 8.8.4.4, you would do:
dns.resolve("8.8.4.4","PTR",console.log);
Now, you would have to:
dns.resolve("4.4.8.8.in-addr.arpa","PTR",console.log);
Or, better, you can still do:
dns.reverse("8.8.4.4",console.log)

silverwind · 2016-01-28T18:29:05Z

Thanks! I'm in support of breaking dns.resolve("8.8.4.4","PTR",console.log), it's nonsense to have magic query transformations like this.

silverwind · 2016-01-28T19:03:20Z

test/internet/test-dns.js

+
+  checkWrap(req);
+});
+


Can you add a IPv6 resolvePtr test too?

Oh, and possibly also one for dns.resolvePtr('8.8.8.8') which should result in an error.

Thanks for your comments. I will add the requested tests next week.

@silverwind - i dont think a IPv6 resolvePtr test makes sense, PTRs actually don't deal with addresses.

I could resolve PTR for "8.8.8.8.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.6.8.4.0.6.8.4.1.0.0.2.ip6.arpa", Google's v6 reverse, but really the test_relovePtr uses "8.8.8.8.in-addr.arpa" only because it's a readily available PTR record - that it's a in-addr.arpa entry is just a coincidence.

dns.reverse(), which deals with addresses, is tested in test-dns-ipv6.js already.

silverwind · 2016-01-29T23:31:45Z

Changes are LGTM so far, waiting on the tests.

jasnell · 2016-02-01T17:32:25Z

/cc @mscdex

silverwind · 2016-02-01T17:36:38Z

CI: https://ci.nodejs.org/job/node-test-pull-request/1485/

silverwind · 2016-02-01T21:10:54Z

CI is green, LGTM

mscdex · 2016-02-01T21:20:35Z

LGTM as long as the unrelated DNS test additions remain in a separate commit when this PR is landed. The others can be squashed together.

dturing · 2016-02-02T13:39:57Z

cool, thanks. Is there anything i can or should do about keeping those commits separate?

jasnell · 2016-02-02T17:48:11Z

@dturing ... yes, the easiest thing would be to rebase and squash just those commits together. If things are out of order, one easy thing to do would be a git reset --soft HEAD~4 on the branch and simply redo the commits so that they are in the right order, then force push back to your dev branch. That would make the commit history a bit cleaner. Then, when that's done, drop a comment here and make sure to mention that the commits should not be squashed when landed :-)

silverwind · 2016-02-04T17:18:52Z

@dturing would be great if you can combine commits 1 and 4, and 2 and 3. One more CI run after that and it should be good to land.

test wether the various resolve functions cause ENOTFOUND when trying to resolve a known invalid domain/hostname.

Resolving plain PTR records is used beyond reverse DNS, most prominently with DNS-SD (RFC6763). This adds dns.resolvePtr(), and uses it (instead of dns.reverse()) in dns.resolve().

dturing · 2016-02-08T10:02:48Z

i've put the DNS test additions and the actual resolvePtr work in separate commits, which should stay separate as pointed out by @jasnell

silverwind · 2016-02-08T14:43:01Z

CI: https://ci.nodejs.org/job/node-test-pull-request/1586/

mscdex · 2016-02-08T15:14:09Z

LGTM

test whether the various resolve functions cause ENOTFOUND when trying to resolve a known invalid domain/hostname. PR-URL: #4921 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Brian White <mscdex@mscdex.net>

Resolving plain PTR records is used beyond reverse DNS, most prominently with DNS-SD (RFC6763). This adds dns.resolvePtr(), and uses it (instead of dns.reverse()) in dns.resolve(). PR-URL: #4921 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Brian White <mscdex@mscdex.net>

silverwind · 2016-02-08T16:32:17Z

Thanks! Landed in c4ab861 and dbdbdd4.

dturing · 2016-02-08T20:59:44Z

Great, thank yous!

test whether the various resolve functions cause ENOTFOUND when trying to resolve a known invalid domain/hostname. PR-URL: nodejs#4921 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Brian White <mscdex@mscdex.net>

Resolving plain PTR records is used beyond reverse DNS, most prominently with DNS-SD (RFC6763). This adds dns.resolvePtr(), and uses it (instead of dns.reverse()) in dns.resolve(). PR-URL: nodejs#4921 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Brian White <mscdex@mscdex.net>

The following significant (semver-major) changes have been made since the previous Node v5.0.0 release. * Buffer * New Buffer constructors have been added [#4682](#4682) * Previously deprecated Buffer APIs are removed [#5048](#5048), [#4594](#4594) * Improved error handling [#4514](#4514) * Cluster * Worker emitted as first argument in 'message' event [#5361](#5361). * Crypto * Improved error handling [#3100](#3100), [#5611](#5611) * Simplified Certificate class bindings [#5382](#5382) * Improved control over FIPS mode [#5181](#5181) * pbkdf2 digest overloading is deprecated [#4047](#4047) * Dependencies * Reintroduce shared c-ares build support [#5775](#5775). * V8 updated to 5.0.71.31 [#6111](#6111). * DNS * Add resolvePtr API to query plain DNS PTR records [#4921](#4921). * Domains * Clear stack when no error handler [#4659](#4659). * File System * The `fs.realpath()` and `fs.realpathSync()` methods have been updated to use a more efficient libuv implementation. This change includes the removal of the `cache` argument and the method can throw new errors [#3594](#3594) * FS apis can now accept and return paths as Buffers [#5616](#5616). * Error handling and type checking improvements [#5616](#5616), [#5590](#5590), [#4518](#4518), [#3917](#3917). * fs.read's string interface is deprecated [#4525](#4525) * HTTP * 'clientError' can now be used to return custom errors from an HTTP server [#4557](#4557). * Modules * Current directory is now prioritized for local lookups [#5689](#5689) * Symbolic links are preserved when requiring modules [#5950](#5950) * Net * DNS hints no longer implicitly set [#6021](#6021). * Improved error handling and type checking [#5981](#5981), [#5733](#5733), [#2904](#2904) * Path * Improved type checking [#5348](#5348). * Process * Introduce process warnings API [#4782](#4782). * Throw exception when non-function passed to nextTick [#3860](#3860). * Readline * Emit key info unconditionally [#6024](#6024) * REPL * Assignment to `_` will emit a warning. [#5535](#5535) * Timers * Fail early when callback is not a function [#4362](#4362) * TLS * Rename 'clientError' to 'tlsClientError' [#4557](#4557) * SHA1 used for sessionIdContext [#3866](#3866) * TTY * Previously deprecated setRawMode wrapper is removed [#2528](#2528). * Util * Changes to Error object formatting [#4582](#4582). * Windows * Windows XP and Vista are no longer supported [#5167](#5167), [#5167](#5167).

The following significant (semver-major) changes have been made since the previous Node v5.0.0 release. * Buffer * New Buffer constructors have been added [#4682](#4682) * Previously deprecated Buffer APIs are removed [#5048](#5048), [#4594](#4594) * Improved error handling [#4514](#4514) * Cluster * Worker emitted as first argument in 'message' event [#5361](#5361). * Crypto * Improved error handling [#3100](#3100), [#5611](#5611) * Simplified Certificate class bindings [#5382](#5382) * Improved control over FIPS mode [#5181](#5181) * pbkdf2 digest overloading is deprecated [#4047](#4047) * Dependencies * Reintroduce shared c-ares build support [#5775](#5775). * V8 updated to 5.0.71.31 [#6111](#6111). * DNS * Add resolvePtr API to query plain DNS PTR records [#4921](#4921). * Domains * Clear stack when no error handler [#4659](#4659). * File System * The `fs.realpath()` and `fs.realpathSync()` methods have been updated to use a more efficient libuv implementation. This change includes the removal of the `cache` argument and the method can throw new errors [#3594](#3594) * FS apis can now accept and return paths as Buffers [#5616](#5616). * Error handling and type checking improvements [#5616](#5616), [#5590](#5590), [#4518](#4518), [#3917](#3917). * fs.read's string interface is deprecated [#4525](#4525) * HTTP * 'clientError' can now be used to return custom errors from an HTTP server [#4557](#4557). * Modules * Current directory is now prioritized for local lookups [#5689](#5689) * Symbolic links are preserved when requiring modules [#5950](#5950) * Net * DNS hints no longer implicitly set [#6021](#6021). * Improved error handling and type checking [#5981](#5981), [#5733](#5733), [#2904](#2904) * OS X * MACOSX_DEPLOYMENT_TARGET has been bumped up to 10.7 [#6402](#6402). * Path * Improved type checking [#5348](#5348). * Process * Introduce process warnings API [#4782](#4782). * Throw exception when non-function passed to nextTick [#3860](#3860). * Readline * Emit key info unconditionally [#6024](#6024) * REPL * Assignment to `_` will emit a warning. [#5535](#5535) * Timers * Fail early when callback is not a function [#4362](#4362) * TLS * Rename 'clientError' to 'tlsClientError' [#4557](#4557) * SHA1 used for sessionIdContext [#3866](#3866) * TTY * Previously deprecated setRawMode wrapper is removed [#2528](#2528). * Util * Changes to Error object formatting [#4582](#4582). * Windows * Windows XP and Vista are no longer supported [#5167](#5167), [#5167](#5167).

mscdex added dns Issues and PRs related to the dns subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. labels Jan 28, 2016

silverwind reviewed Jan 28, 2016
View reviewed changes

dturing added 2 commits February 8, 2016 10:54

dns: add failure test for dns.resolveXXX

4b204d5

test wether the various resolve functions cause ENOTFOUND when trying to resolve a known invalid domain/hostname.

dns: add resolvePtr to query for plain DNS PTR records

a2a280c

Resolving plain PTR records is used beyond reverse DNS, most prominently with DNS-SD (RFC6763). This adds dns.resolvePtr(), and uses it (instead of dns.reverse()) in dns.resolve().

dturing force-pushed the dns-ptr-queries branch from 40e1e9e to a2a280c Compare February 8, 2016 09:59

silverwind closed this Feb 8, 2016

silverwind added the relnotes label Feb 8, 2016

jasnell mentioned this pull request Mar 17, 2016

Planning for v6 #5766

Closed

jasnell mentioned this pull request Apr 19, 2016

What is new in v6? #6264

Closed

jasnell mentioned this pull request Apr 26, 2016

2016-04-26, Version 6.0.0 (Current) Release #6383

Closed

outsideris mentioned this pull request May 24, 2016

Translate Node.js v6.0.0 nodejs/nodejs-ko#345

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support resolving plain DNS PTR records #4921

Support resolving plain DNS PTR records #4921

dturing commented Jan 27, 2016

silverwind commented Jan 28, 2016

silverwind Jan 28, 2016

silverwind Jan 28, 2016

dturing Jan 29, 2016

dturing Feb 1, 2016

silverwind commented Jan 29, 2016

jasnell commented Feb 1, 2016

silverwind commented Feb 1, 2016

silverwind commented Feb 1, 2016

mscdex commented Feb 1, 2016

dturing commented Feb 2, 2016

jasnell commented Feb 2, 2016

silverwind commented Feb 4, 2016

dturing commented Feb 8, 2016

silverwind commented Feb 8, 2016

mscdex commented Feb 8, 2016

silverwind commented Feb 8, 2016

dturing commented Feb 8, 2016


		checkWrap(req);
		});

Support resolving plain DNS PTR records #4921

Support resolving plain DNS PTR records #4921

Conversation

dturing commented Jan 27, 2016

silverwind commented Jan 28, 2016

silverwind Jan 28, 2016

Choose a reason for hiding this comment

silverwind Jan 28, 2016

Choose a reason for hiding this comment

dturing Jan 29, 2016

Choose a reason for hiding this comment

dturing Feb 1, 2016

Choose a reason for hiding this comment

silverwind commented Jan 29, 2016

jasnell commented Feb 1, 2016

silverwind commented Feb 1, 2016

silverwind commented Feb 1, 2016

mscdex commented Feb 1, 2016

dturing commented Feb 2, 2016

jasnell commented Feb 2, 2016

silverwind commented Feb 4, 2016

dturing commented Feb 8, 2016

silverwind commented Feb 8, 2016

mscdex commented Feb 8, 2016

silverwind commented Feb 8, 2016

dturing commented Feb 8, 2016