DNS: Support NAPTR queries #3177

Closed
wants to merge 1 commit into
from

Projects

None yet

4 participants

@ssuda
ssuda commented Apr 25, 2012

Adding support for NAPTR records
fixes #3170

@bnoordhuis bnoordhuis commented on an outdated diff Apr 29, 2012
src/cares_wrap.cc
@@ -485,6 +485,63 @@ class QuerySrvWrap: public QueryWrap {
}
};
+#if (ARES_VERSION >= 0x010706)
@bnoordhuis
bnoordhuis Apr 29, 2012 Member

Drop the conditional.

@bnoordhuis bnoordhuis commented on an outdated diff Apr 29, 2012
src/cares_wrap.cc
+ public:
+ int Send(const char* name) {
+ ares_query(ares_channel,
+ name,
+ ns_c_in,
+ ns_t_naptr,
+ Callback,
+ GetQueryArg());
+ return 0;
+ }
+
+ protected:
+ void Parse(unsigned char* buf, int len) {
+ HandleScope scope;
+
+ struct ares_naptr_reply* naptr_start;
@bnoordhuis
bnoordhuis Apr 29, 2012 Member

Superfluous struct keyword. (Happens down below too).

@bnoordhuis bnoordhuis commented on an outdated diff Apr 29, 2012
src/node.cc
@@ -2082,6 +2082,9 @@ static void ProcessTitleSetter(Local<String> property,
obj->Set(String::NewSymbol("ipv6"), True()); // TODO ping libuv
obj->Set(String::NewSymbol("tls_npn"), Boolean::New(use_npn));
obj->Set(String::NewSymbol("tls_sni"), Boolean::New(use_sni));
+#if (ARES_VERSION >= 0x010706)
+ obj->Set(String::NewSymbol("dns_naptr"), True());
+#endif
@bnoordhuis
bnoordhuis Apr 29, 2012 Member

This can be removed. NAPTR support will be in v0.8.0.

@bnoordhuis
Member

LGTM in general. This PR depends on a c-ares upgrade in libuv but that will happen soon.

ssuda DNS: Support NAPTR queries
Adding support for NAPTR records
fixes #3170
9dc7341
@ssuda
ssuda commented Apr 30, 2012

Updated to include @bnoordhuis comments, Thanks.

@bnoordhuis bnoordhuis was assigned Apr 30, 2012
@isaacs
isaacs commented Jun 16, 2012

Landed on 91bf18f.

Thanks!

@isaacs isaacs closed this Jun 16, 2012
@isaacs
isaacs commented Jun 16, 2012

Whoops, that was premature, sorry. NAPTR support is not landed yet.

Pushing this off until post-0.8. Sorry for the confusion.

Reverted on a90bc78, tagged for post-v0.8.

@isaacs isaacs reopened this Jun 16, 2012
@johntdyer

Any idea when this is going to happen ? Would love to use Node for an intelligent proxy, but I require NAPTR support in order to do it.

@isaacs
isaacs commented Mar 6, 2013

It looks like the upgrade to cares is done, but this PR has to be rebased onto master in order to be considered.

I fixed test-internet in #4933, but with this patch on top of that branch, it won't build without this additional patch:

diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc
index 1734a62..5940ec3 100644
--- a/src/cares_wrap.cc
+++ b/src/cares_wrap.cc
@@ -651,9 +651,10 @@ class QueryNaptrWrap: public QueryWrap {

       Local<Object> naptr_record = Object::New();

-      naptr_record->Set(flags_symbol, String::New(naptr_current->flags));
-      naptr_record->Set(service_symbol, String::New(naptr_current->service));
-      naptr_record->Set(regexp_symbol, String::New(naptr_current->regexp));
+      naptr_record->Set(flags_symbol, String::New((const char *)naptr_current->flags));
+      naptr_record->Set(service_symbol, String::New((const char *)naptr_current->service));
+      naptr_record->Set(regexp_symbol, String::New((const char *)naptr_current->regexp));
+
       naptr_record->Set(replacement_symbol, String::New(naptr_current->replacement));
       naptr_record->Set(order_symbol, Integer::New(naptr_current->order));
       naptr_record->Set(preference_symbol, Integer::New(naptr_current->preference));

and the tests don't pass.

I'm closing this PR for now. Please feel free to reincarnate it when those changes are made.

Thanks!

@isaacs isaacs closed this Mar 6, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment