-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
dns: fix Windows SRV ECONNREFUSED regression by correcting c-ares fallback detection #61453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
NotVivek12
wants to merge
7
commits into
nodejs:main
Choose a base branch
from
NotVivek12:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+284
−6
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
7914dcd
fix fix Windows SRV ECONNREFUSED regression
NotVivek12 0683f48
Merge branch 'main' of https://github.com/NotVivek12/node
NotVivek12 40ea867
Merge branch 'nodejs:main' into main
NotVivek12 2a528e2
add regression tests
NotVivek12 5160161
remove unintentional file
NotVivek12 2fb8a09
Merge branch 'nodejs:main' into main
NotVivek12 a72c8ea
Merge branch 'main' of https://github.com/NotVivek12/node
NotVivek12 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| 'use strict'; | ||
| // Regression test for SRV record resolution returning ECONNREFUSED. | ||
| // | ||
| // This test verifies that dns.resolveSrv() properly handles SRV queries | ||
| // and doesn't incorrectly return ECONNREFUSED errors when DNS servers | ||
| // are reachable but the query format or handling has issues. | ||
| // | ||
| // Background: In certain Node.js versions, SRV queries could fail with | ||
| // ECONNREFUSED even when the DNS server was accessible, affecting | ||
| // applications using MongoDB Atlas (mongodb+srv://) and other services | ||
| // that rely on SRV record discovery. | ||
|
|
||
| const common = require('../common'); | ||
| const dnstools = require('../common/dns'); | ||
| const dns = require('dns'); | ||
| const dnsPromises = dns.promises; | ||
| const assert = require('assert'); | ||
| const dgram = require('dgram'); | ||
|
|
||
| // Test 1: Basic SRV resolution should succeed, not return ECONNREFUSED | ||
| { | ||
| const server = dgram.createSocket('udp4'); | ||
| const srvRecord = { | ||
| type: 'SRV', | ||
| name: 'mongodb-server.cluster0.example.net', | ||
| port: 27017, | ||
| priority: 0, | ||
| weight: 1, | ||
| ttl: 60, | ||
| }; | ||
|
|
||
| server.on('message', common.mustCall((msg, { address, port }) => { | ||
| const parsed = dnstools.parseDNSPacket(msg); | ||
| const domain = parsed.questions[0].domain; | ||
|
|
||
| server.send(dnstools.writeDNSPacket({ | ||
| id: parsed.id, | ||
| questions: parsed.questions, | ||
| answers: [Object.assign({ domain }, srvRecord)], | ||
| }), port, address); | ||
| })); | ||
|
|
||
| server.bind(0, common.mustCall(async () => { | ||
| const { port } = server.address(); | ||
| const resolver = new dnsPromises.Resolver(); | ||
| resolver.setServers([`127.0.0.1:${port}`]); | ||
|
|
||
| try { | ||
| const result = await resolver.resolveSrv( | ||
| '_mongodb._tcp.cluster0.example.net' | ||
| ); | ||
|
|
||
| // Should NOT throw ECONNREFUSED | ||
| assert.ok(Array.isArray(result)); | ||
| assert.strictEqual(result.length, 1); | ||
| assert.strictEqual(result[0].name, 'mongodb-server.cluster0.example.net'); | ||
| assert.strictEqual(result[0].port, 27017); | ||
| assert.strictEqual(result[0].priority, 0); | ||
| assert.strictEqual(result[0].weight, 1); | ||
| } catch (err) { | ||
| // This is the regression: should NOT get ECONNREFUSED | ||
| assert.notStrictEqual( | ||
| err.code, | ||
| 'ECONNREFUSED', | ||
| 'SRV query should not fail with ECONNREFUSED when server is reachable' | ||
| ); | ||
| throw err; | ||
| } finally { | ||
| server.close(); | ||
| } | ||
| })); | ||
| } | ||
|
|
||
| // Test 2: Multiple SRV records (common for MongoDB Atlas clusters) | ||
| { | ||
| const server = dgram.createSocket('udp4'); | ||
| const srvRecords = [ | ||
| { type: 'SRV', name: 'shard-00-00.cluster.mongodb.net', port: 27017, priority: 0, weight: 1, ttl: 60 }, | ||
| { type: 'SRV', name: 'shard-00-01.cluster.mongodb.net', port: 27017, priority: 0, weight: 1, ttl: 60 }, | ||
| { type: 'SRV', name: 'shard-00-02.cluster.mongodb.net', port: 27017, priority: 0, weight: 1, ttl: 60 }, | ||
| ]; | ||
|
|
||
| server.on('message', common.mustCall((msg, { address, port }) => { | ||
| const parsed = dnstools.parseDNSPacket(msg); | ||
| const domain = parsed.questions[0].domain; | ||
|
|
||
| server.send(dnstools.writeDNSPacket({ | ||
| id: parsed.id, | ||
| questions: parsed.questions, | ||
| answers: srvRecords.map((r) => Object.assign({ domain }, r)), | ||
| }), port, address); | ||
| })); | ||
|
|
||
| server.bind(0, common.mustCall(async () => { | ||
| const { port } = server.address(); | ||
| const resolver = new dnsPromises.Resolver(); | ||
| resolver.setServers([`127.0.0.1:${port}`]); | ||
|
|
||
| const result = await resolver.resolveSrv('_mongodb._tcp.cluster.mongodb.net'); | ||
|
|
||
| assert.strictEqual(result.length, 3); | ||
|
|
||
| const names = result.map((r) => r.name).sort(); | ||
| assert.deepStrictEqual(names, [ | ||
| 'shard-00-00.cluster.mongodb.net', | ||
| 'shard-00-01.cluster.mongodb.net', | ||
| 'shard-00-02.cluster.mongodb.net', | ||
| ]); | ||
|
|
||
| server.close(); | ||
| })); | ||
| } | ||
|
|
||
| // Test 3: Callback-based API should also work | ||
| { | ||
| const server = dgram.createSocket('udp4'); | ||
|
|
||
| server.on('message', common.mustCall((msg, { address, port }) => { | ||
| const parsed = dnstools.parseDNSPacket(msg); | ||
| const domain = parsed.questions[0].domain; | ||
|
|
||
| server.send(dnstools.writeDNSPacket({ | ||
| id: parsed.id, | ||
| questions: parsed.questions, | ||
| answers: [{ | ||
| domain, | ||
| type: 'SRV', | ||
| name: 'service.example.com', | ||
| port: 443, | ||
| priority: 10, | ||
| weight: 5, | ||
| ttl: 120, | ||
| }], | ||
| }), port, address); | ||
| })); | ||
|
|
||
| server.bind(0, common.mustCall(() => { | ||
| const { port } = server.address(); | ||
| const resolver = new dns.Resolver(); | ||
| resolver.setServers([`127.0.0.1:${port}`]); | ||
|
|
||
| resolver.resolveSrv('_https._tcp.example.com', common.mustSucceed((result) => { | ||
| assert.strictEqual(result.length, 1); | ||
| assert.strictEqual(result[0].name, 'service.example.com'); | ||
| assert.strictEqual(result[0].port, 443); | ||
| assert.strictEqual(result[0].priority, 10); | ||
| assert.strictEqual(result[0].weight, 5); | ||
| server.close(); | ||
| })); | ||
| })); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| 'use strict'; | ||
| // Regression test for dns.resolveSrv() functionality. | ||
| // This test ensures SRV record resolution works correctly, which is | ||
| // critical for services like MongoDB Atlas that use SRV records for | ||
| // connection discovery (mongodb+srv:// URIs). | ||
| // | ||
| // Related issue: dns.resolveSrv() returning ECONNREFUSED instead of | ||
| // properly resolving SRV records. | ||
|
|
||
| const common = require('../common'); | ||
| const dnstools = require('../common/dns'); | ||
| const dns = require('dns'); | ||
| const dnsPromises = dns.promises; | ||
| const assert = require('assert'); | ||
| const dgram = require('dgram'); | ||
|
|
||
| const srvRecords = [ | ||
| { | ||
| type: 'SRV', | ||
| name: 'server1.example.org', | ||
| port: 27017, | ||
| priority: 0, | ||
| weight: 5, | ||
| ttl: 300, | ||
| }, | ||
| { | ||
| type: 'SRV', | ||
| name: 'server2.example.org', | ||
| port: 27017, | ||
| priority: 0, | ||
| weight: 5, | ||
| ttl: 300, | ||
| }, | ||
| { | ||
| type: 'SRV', | ||
| name: 'server3.example.org', | ||
| port: 27017, | ||
| priority: 1, | ||
| weight: 10, | ||
| ttl: 300, | ||
| }, | ||
| ]; | ||
|
|
||
| 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, '_mongodb._tcp.cluster0.example.org'); | ||
|
|
||
| server.send(dnstools.writeDNSPacket({ | ||
| id: parsed.id, | ||
| questions: parsed.questions, | ||
| answers: srvRecords.map((record) => Object.assign({ domain }, record)), | ||
| }), port, address); | ||
| }, 2)); // Called twice: once for callback, once for promises | ||
|
|
||
| server.bind(0, common.mustCall(async () => { | ||
| const address = server.address(); | ||
| const resolver = new dns.Resolver(); | ||
| const resolverPromises = new dnsPromises.Resolver(); | ||
|
|
||
| resolver.setServers([`127.0.0.1:${address.port}`]); | ||
| resolverPromises.setServers([`127.0.0.1:${address.port}`]); | ||
|
|
||
| function validateResult(result) { | ||
| assert.ok(Array.isArray(result), 'Result should be an array'); | ||
| assert.strictEqual(result.length, 3, 'Should have 3 SRV records'); | ||
|
|
||
| for (const record of result) { | ||
| assert.strictEqual(typeof record, 'object'); | ||
| assert.strictEqual(typeof record.name, 'string'); | ||
| assert.strictEqual(typeof record.port, 'number'); | ||
| assert.strictEqual(typeof record.priority, 'number'); | ||
| assert.strictEqual(typeof record.weight, 'number'); | ||
| assert.strictEqual(record.port, 27017); | ||
| } | ||
|
|
||
| // Verify we got all expected server names | ||
| const names = result.map((r) => r.name).sort(); | ||
| assert.deepStrictEqual(names, [ | ||
| 'server1.example.org', | ||
| 'server2.example.org', | ||
| 'server3.example.org', | ||
| ]); | ||
| } | ||
|
|
||
| // Test promises API | ||
| const promiseResult = await resolverPromises.resolveSrv( | ||
| '_mongodb._tcp.cluster0.example.org' | ||
| ); | ||
| validateResult(promiseResult); | ||
|
|
||
| // Test callback API | ||
| resolver.resolveSrv( | ||
| '_mongodb._tcp.cluster0.example.org', | ||
| common.mustSucceed((result) => { | ||
| validateResult(result); | ||
| server.close(); | ||
| }) | ||
| ); | ||
| })); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to check for host/network byte order conversion here like we do for IPv4 (via
htonl)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, byte order conversion is not needed for IPv6 in this case because IPv6 addresses are stored and compared as a byte array (16 individual unsigned char values), not as a multi-byte integer. memcmp compares byte-by-byte, so endianness doesn't matter.