dns: fix unsigned record values

Fixes: #28790

PR-URL: #28792
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>

Author: mscdex

src/cares_wrap.cc

@@ -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();
   }
 
@@ -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();
@@ -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();
@@ -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();
@@ -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();
@@ -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(),
@@ -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);
 

test/parallel/test-dns.js

@@ -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);
@@ -326,3 +328,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();
+      }));
+    })();
+
+  }));
+}