Skip to content

Commit

Permalink
dns: fix unsigned record values
Browse files Browse the repository at this point in the history
Fixes: nodejs#28790

PR-URL: nodejs#28792
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
mscdex authored and gntem committed Jul 27, 2019
1 parent bfd5d37 commit 00bd55d
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 8 deletions.
19 changes: 11 additions & 8 deletions src/cares_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ Local<Array> AddrTTLToArray(Environment* env,

Local<Array> ttls = Array::New(isolate, naddrttls);
for (size_t i = 0; i < naddrttls; i++) {
auto value = Integer::New(isolate, addrttls[i].ttl);
auto value = Integer::NewFromUnsigned(isolate, addrttls[i].ttl);
ttls->Set(context, i, value).Check();
}

Expand Down Expand Up @@ -1139,7 +1139,7 @@ int ParseSoaReply(Environment* env,
hostmaster.get())).Check();
soa_record->Set(context,
env->serial_string(),
Integer::New(env->isolate(), serial)).Check();
Integer::NewFromUnsigned(env->isolate(), serial)).Check();
soa_record->Set(context,
env->refresh_string(),
Integer::New(env->isolate(), refresh)).Check();
Expand All @@ -1151,7 +1151,7 @@ int ParseSoaReply(Environment* env,
Integer::New(env->isolate(), expire)).Check();
soa_record->Set(context,
env->minttl_string(),
Integer::New(env->isolate(), minttl)).Check();
Integer::NewFromUnsigned(env->isolate(), minttl)).Check();
soa_record->Set(context,
env->type_string(),
env->dns_soa_string()).Check();
Expand Down Expand Up @@ -1219,7 +1219,8 @@ class QueryAnyWrap: public QueryWrap {
ret->Get(context, i).ToLocalChecked()).Check();
obj->Set(context,
env()->ttl_string(),
Integer::New(env()->isolate(), addrttls[i].ttl)).Check();
Integer::NewFromUnsigned(
env()->isolate(), addrttls[i].ttl)).Check();
obj->Set(context,
env()->type_string(),
env()->dns_a_string()).Check();
Expand Down Expand Up @@ -1265,8 +1266,8 @@ class QueryAnyWrap: public QueryWrap {
ret->Get(context, i).ToLocalChecked()).Check();
obj->Set(context,
env()->ttl_string(),
Integer::New(env()->isolate(), addr6ttls[i - a_count].ttl))
.Check();
Integer::NewFromUnsigned(
env()->isolate(), addr6ttls[i - a_count].ttl)).Check();
obj->Set(context,
env()->type_string(),
env()->dns_aaaa_string()).Check();
Expand Down Expand Up @@ -1709,7 +1710,8 @@ class QuerySoaWrap: public QueryWrap {
soa_out->hostmaster)).Check();
soa_record->Set(context,
env()->serial_string(),
Integer::New(env()->isolate(), soa_out->serial)).Check();
Integer::NewFromUnsigned(
env()->isolate(), soa_out->serial)).Check();
soa_record->Set(context,
env()->refresh_string(),
Integer::New(env()->isolate(),
Expand All @@ -1722,7 +1724,8 @@ class QuerySoaWrap: public QueryWrap {
Integer::New(env()->isolate(), soa_out->expire)).Check();
soa_record->Set(context,
env()->minttl_string(),
Integer::New(env()->isolate(), soa_out->minttl)).Check();
Integer::NewFromUnsigned(
env()->isolate(), soa_out->minttl)).Check();

ares_free_data(soa_out);

Expand Down
112 changes: 112 additions & 0 deletions test/parallel/test-dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@

'use strict';
const common = require('../common');
const dnstools = require('../common/dns');
const assert = require('assert');

const dns = require('dns');
const dnsPromises = dns.promises;
const dgram = require('dgram');

const existing = dns.getServers();
assert(existing.length > 0);
Expand Down Expand Up @@ -325,3 +327,113 @@ common.expectsError(() => {
assert.deepStrictEqual(err.message, 'queryMx ENOTFOUND foo.onion');
});
}

{
const cases = [
{ method: 'resolveAny',
answers: [
{ type: 'A', address: '1.2.3.4', ttl: 3333333333 },
{ type: 'AAAA', address: '::42', ttl: 3333333333 },
{ type: 'MX', priority: 42, exchange: 'foobar.com', ttl: 3333333333 },
{ type: 'NS', value: 'foobar.org', ttl: 3333333333 },
{ type: 'PTR', value: 'baz.org', ttl: 3333333333 },
{
type: 'SOA',
nsname: 'ns1.example.com',
hostmaster: 'admin.example.com',
serial: 3210987654,
refresh: 900,
retry: 900,
expire: 1800,
minttl: 3333333333
},
]
},

{ method: 'resolve4',
options: { ttl: true },
answers: [ { type: 'A', address: '1.2.3.4', ttl: 3333333333 } ]
},

{ method: 'resolve6',
options: { ttl: true },
answers: [ { type: 'AAAA', address: '::42', ttl: 3333333333 } ]
},

{ method: 'resolveSoa',
answers: [
{
type: 'SOA',
nsname: 'ns1.example.com',
hostmaster: 'admin.example.com',
serial: 3210987654,
refresh: 900,
retry: 900,
expire: 1800,
minttl: 3333333333
}
]
},
];

const server = dgram.createSocket('udp4');

server.on('message', common.mustCall((msg, { address, port }) => {
const parsed = dnstools.parseDNSPacket(msg);
const domain = parsed.questions[0].domain;
assert.strictEqual(domain, 'example.org');

server.send(dnstools.writeDNSPacket({
id: parsed.id,
questions: parsed.questions,
answers: cases[0].answers.map(
(answer) => Object.assign({ domain }, answer)
),
}), port, address);
}, cases.length * 2));

server.bind(0, common.mustCall(() => {
const address = server.address();
dns.setServers([`127.0.0.1:${address.port}`]);

function validateResults(res) {
if (!Array.isArray(res))
res = [res];

assert.deepStrictEqual(res.map(tweakEntry),
cases[0].answers.map(tweakEntry));
}

function tweakEntry(r) {
const ret = { ...r };

const { method } = cases[0];

// TTL values are only provided for A and AAAA entries.
if (!['A', 'AAAA'].includes(ret.type) && !/^resolve(4|6)?$/.test(method))
delete ret.ttl;

if (method !== 'resolveAny')
delete ret.type;

return ret;
}

(async function nextCase() {
if (cases.length === 0)
return server.close();

const { method, options } = cases[0];

validateResults(await dnsPromises[method]('example.org', options));

dns[method]('example.org', options, common.mustCall((err, res) => {
assert.ifError(err);
validateResults(res);
cases.shift();
nextCase();
}));
})();

}));
}

0 comments on commit 00bd55d

Please sign in to comment.