diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..20c930e --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,12 @@ + +### 2026-05-26 + +- client/udp.js refactors + 1. Mismatched ids are dropped and the listener keeps waiting — stray packets no longer crash the process. + 2. Sender filtering. Reject any packet whose rinfo.port isn't the configured resolver port; additionally enforce rinfo.address when dns is an IP literal (using net.isIP). + 3. Defensive Packet.parse. Wrapped in try/catch so a malformed stray packet doesn't reject the promise — it's dropped with a debug log. + 4. Timeout. New timeout option (default 10s, set 0 to disable). On expiry the promise rejects with code: 'ETIMEDOUT'. Timer is .unref()-ed so it never holds the event loop open. + 5. Full 16-bit transaction IDs. query.header.id = crypto.randomInt(0x10000), 6.5× the keyspace and uses a CSPRNG. + 6. Proper cleanup. Single cleanup() clears the timer, removes both listeners, and closes the socket; settled guard prevents double-resolve/reject from racing message + timeout. + 7. Error event handled. Socket errors now reject the promise instead of going unhandled. + diff --git a/client/udp.js b/client/udp.js index 1fa69bc..6b8cb32 100644 --- a/client/udp.js +++ b/client/udp.js @@ -1,15 +1,21 @@ const udp = require('dgram'); +const net = require('net'); +const crypto = require('crypto'); const Packet = require('../packet'); -const { equal } = require('assert'); const { debuglog } = require('util'); const debug = debuglog('dns2'); -module.exports = ({ dns = '8.8.8.8', port = 53, socketType = 'udp4' } = {}) => { +module.exports = ({ + dns = '8.8.8.8', + port = 53, + socketType = 'udp4', + timeout = 10000, +} = {}) => { return (name, type = 'A', cls = Packet.CLASS.IN, options = {}) => { const { clientIp, recursive = true } = options; const query = new Packet(); - query.header.id = (Math.random() * 1e4) | 0; + query.header.id = crypto.randomInt(0x10000); // see https://github.com/song940/node-dns/issues/29 if (recursive) { query.header.rd = 1; @@ -25,15 +31,66 @@ module.exports = ({ dns = '8.8.8.8', port = 53, socketType = 'udp4' } = {}) => { type : Packet.TYPE[type], }); const client = new udp.Socket(socketType); + // Only enforce a strict source-address check when `dns` is an IP literal; + // hostnames would require an extra resolve to compare against. + const expectedAddress = net.isIP(dns) ? dns : null; return new Promise((resolve, reject) => { - client.once('message', function onMessage(message) { - client.close(); - const response = Packet.parse(message); - equal(response.header.id, query.header.id); + let settled = false; + let timer; + const cleanup = () => { + if (settled) return; + settled = true; + if (timer) clearTimeout(timer); + client.removeListener('message', onMessage); + client.removeListener('error', onError); + try { client.close(); } catch (_) { /* already closed */ } + }; + function onMessage(message, rinfo) { + // Drop packets that didn't come from the configured resolver. + if (rinfo.port !== port || (expectedAddress && rinfo.address !== expectedAddress)) { + debug('udp: dropping packet from unexpected sender %s:%d', rinfo.address, rinfo.port); + return; + } + let response; + try { + response = Packet.parse(message); + } catch (e) { + debug('udp: dropping unparseable packet: %s', e.message); + return; + } + // Stray / late reply from a reused ephemeral port — keep listening. + if (response.header.id !== query.header.id) { + debug('udp: dropping response with mismatched id %d (expected %d)', + response.header.id, query.header.id); + return; + } + cleanup(); resolve(response); - }); + } + function onError(err) { + cleanup(); + reject(err); + } + client.on('message', onMessage); + client.on('error', onError); + + if (timeout > 0) { + timer = setTimeout(() => { + cleanup(); + const err = new Error(`DNS query timed out after ${timeout}ms`); + err.code = 'ETIMEDOUT'; + reject(err); + }, timeout); + timer.unref(); + } + debug('send', dns, query.toBuffer()); - client.send(query.toBuffer(), port, dns, err => err && reject(err)); + client.send(query.toBuffer(), port, dns, err => { + if (err) { + cleanup(); + reject(err); + } + }); }); }; }; diff --git a/test/index.js b/test/index.js index d3eb113..e65e30f 100644 --- a/test/index.js +++ b/test/index.js @@ -401,6 +401,61 @@ test('server/udp-tcp#simple-request-async-response', async() => { await server.close(); }); +test('client/udp ignores stray response and resolves on matching id', async() => { + // Simulate the scenario from upstream issue #100: a stray UDP packet (e.g. + // late reply on a reused ephemeral port) arrives before the real response. + // The client must drop it and keep listening rather than asserting/crashing. + const server = udp.createSocket('udp4'); + await new Promise(resolve => server.bind(0, '127.0.0.1', resolve)); + const { port: serverPort } = server.address(); + + server.on('message', (msg, rinfo) => { + const request = Packet.parse(msg); + + // Stray packet: same socket, but a different (wrong) transaction id. + const stray = new Packet(); + stray.header.id = (request.header.id + 1) & 0xffff; + stray.header.qr = 1; + server.send(stray.toBuffer(), rinfo.port, rinfo.address); + + // Real reply, slightly delayed so the stray definitely lands first. + const response = Packet.createResponseFromRequest(request); + response.answers.push({ + name : request.questions[0].name, + type : Packet.TYPE.A, + class : Packet.CLASS.IN, + ttl : 300, + address : '1.2.3.4', + }); + setTimeout(() => server.send(response.toBuffer(), rinfo.port, rinfo.address), 5); + }); + + const query = UDPClient({ dns: '127.0.0.1', port: serverPort, timeout: 2000 }); + const reply = await query('stray.test'); + assert.equal(reply.answers.length, 1); + assert.equal(reply.answers[0].address, '1.2.3.4'); + await new Promise(resolve => server.close(resolve)); +}); + +test('client/udp times out when no matching response arrives', async() => { + // Server replies with only stray packets; client must time out, not hang. + const server = udp.createSocket('udp4'); + await new Promise(resolve => server.bind(0, '127.0.0.1', resolve)); + const { port: serverPort } = server.address(); + + server.on('message', (msg, rinfo) => { + const request = Packet.parse(msg); + const stray = new Packet(); + stray.header.id = (request.header.id + 1) & 0xffff; + stray.header.qr = 1; + server.send(stray.toBuffer(), rinfo.port, rinfo.address); + }); + + const query = UDPClient({ dns: '127.0.0.1', port: serverPort, timeout: 100 }); + await assert.rejects(query('timeout.test'), err => err.code === 'ETIMEDOUT'); + await new Promise(resolve => server.close(resolve)); +}); + test('server/all#invalid-request', async() => { const server = createServer({ doh : true,