From 73ca011156c0e3fcf81ba88400f2f8c028fc9273 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Wed, 14 Aug 2024 17:32:41 -0400 Subject: [PATCH 01/16] drivers-2922 downstream changes first pass --- src/connection_string.ts | 5 -- ...itial_dns_seedlist_discovery.prose.test.ts | 47 +++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts diff --git a/src/connection_string.ts b/src/connection_string.ts index bf9acf556c2..a08f2b17eff 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -64,11 +64,6 @@ export async function resolveSRVRecord(options: MongoOptions): Promise { + function makeSrvStub() { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'localhost', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); + } + + afterEach(async () => { + sinon.restore(); + }); + + it('1.1 Driver should not throw error on SRV URI with two parts', async () => { + // 1. stub dns resolution to always pass + makeSrvStub(); + // 2. assert that creating a MongoClient with the uri 'mongodb+srv://mongodb.localhost' does not cause an error + //const client = new MongoClient('mongodb+srv://mongodb.localhost', {}); + const client = new MongoClient('mongodb+srv://mongodb.localhost'); + // 3. assert that connecting the client from 2. to the server does not cause an error + await client.connect(); + }); + + it('1.2 Driver should not throw error on SRV URI with one part', async () => { + // 1. stub dns resolution to always pass + makeSrvStub(); + // 2. assert that creating a MongoClient with the uri 'mongodb+srv//localhost' does not cause an error + const client = new MongoClient('mongodb+srv://localhost', {}); + // 3. assert that connecting the client from 2. to the server does not cause an error + await client.connect(); + }); +}); From ad151f7e7c82882bd1787bbfa5ea1c070a0a760d Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Mon, 19 Aug 2024 16:25:54 -0400 Subject: [PATCH 02/16] temp commit --- .../change-streams/change_stream.test.ts | 1 + .../connection_pool.test.ts | 48 +++++++++++ test/integration/crud/bulk.test.ts | 2 +- ...itial_dns_seedlist_discovery.prose.test.ts | 85 ++++++++++--------- 4 files changed, 95 insertions(+), 41 deletions(-) diff --git a/test/integration/change-streams/change_stream.test.ts b/test/integration/change-streams/change_stream.test.ts index 3fa1e792ed0..aae2a33bfa8 100644 --- a/test/integration/change-streams/change_stream.test.ts +++ b/test/integration/change-streams/change_stream.test.ts @@ -1,5 +1,6 @@ import { strict as assert } from 'assert'; import { expect } from 'chai'; +import * as dns from 'dns'; import { on, once } from 'events'; import { gte, lt } from 'semver'; import * as sinon from 'sinon'; diff --git a/test/integration/connection-monitoring-and-pooling/connection_pool.test.ts b/test/integration/connection-monitoring-and-pooling/connection_pool.test.ts index 437961b1201..e31fe2aeed1 100644 --- a/test/integration/connection-monitoring-and-pooling/connection_pool.test.ts +++ b/test/integration/connection-monitoring-and-pooling/connection_pool.test.ts @@ -3,6 +3,8 @@ import { once } from 'node:events'; import { expect } from 'chai'; import { type ConnectionPoolCreatedEvent, type Db, type MongoClient } from '../../mongodb'; +import sinon = require('sinon'); +import dns = require('dns'); describe('Connection Pool', function () { let client: MongoClient; @@ -19,6 +21,52 @@ describe('Connection Pool', function () { describe('Events', function () { describe('ConnectionPoolCreatedEvent', function () { + describe.only( + 'Initial DNS Seedlist Discovery (Prose Tests)', + { requires: { topology: 'single' } }, + () => { + function makeSrvStub() { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'localhost', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); + } + + afterEach(async function () { + sinon.restore(); + }); + + it('1.1 Driver should not throw error on valid SRV URI with one part', async function () { + // 1. make dns resolution always pass + makeSrvStub(); + // 2. assert that creating a MongoClient with the uri 'mongodb+srv:/localhost' does not cause an error + client = this.configuration.newClient('mongodb+srv://localhost', {}); + console.log('stubbed srv client', client); + // 3. assert that connecting the client from 2. to the server does not cause an error + //await client.connect(); + }); + + it('1.1 Driver should not throw error on valid SRV URI with two parts', async function () { + // 1. make dns resolution always pass + makeSrvStub(); + // 2. assert that creating a MongoClient with the uri 'mongodb+srv://mongodb.localhost' does not cause an error + const client = this.configuration.newClient('mongodb://localhost', {}); + console.log('stubbed normal client', client); + // 3. assert that connecting the client to the server does not cause an error + //await client.connect(); + }); + } + ); context('when no connection pool options are passed in', function () { let pConnectionPoolCreated: Promise; let connectionPoolCreated: ConnectionPoolCreatedEvent; diff --git a/test/integration/crud/bulk.test.ts b/test/integration/crud/bulk.test.ts index c479b842962..6d260dd3fa8 100644 --- a/test/integration/crud/bulk.test.ts +++ b/test/integration/crud/bulk.test.ts @@ -723,7 +723,7 @@ describe('Bulk', function () { } }); - it( + it.only( 'should correctly execute ordered batch using w:0', // TODO(NODE-6060): set `moreToCome` op_msg bit when `w: 0` is specified { requires: { mongodb: '<8.0.0' } }, diff --git a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts index c629ec4da2d..2a7f8daeee8 100644 --- a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts +++ b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts @@ -1,47 +1,52 @@ -import { expect } from 'chai'; import * as dns from 'dns'; - -import { MongoClient } from '../../mongodb'; import sinon = require('sinon'); +// import { expect } from 'chai'; -describe.only('Initial DNS Seedlist Discovery (Prose Tests)', () => { - function makeSrvStub() { - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'localhost', - port: 27017, - weight: 0, - priority: 0 - } - ]; - }); +import { type MongoClient } from '../../mongodb'; - sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { - throw { code: 'ENODATA' }; - }); - } +describe( + 'Initial DNS Seedlist Discovery (Prose Tests)', + { requires: { topology: 'single' } }, + () => { + let client: MongoClient; + + function makeSrvStub() { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'localhost', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); - afterEach(async () => { - sinon.restore(); - }); + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); + } - it('1.1 Driver should not throw error on SRV URI with two parts', async () => { - // 1. stub dns resolution to always pass - makeSrvStub(); - // 2. assert that creating a MongoClient with the uri 'mongodb+srv://mongodb.localhost' does not cause an error - //const client = new MongoClient('mongodb+srv://mongodb.localhost', {}); - const client = new MongoClient('mongodb+srv://mongodb.localhost'); - // 3. assert that connecting the client from 2. to the server does not cause an error - await client.connect(); - }); + afterEach(async function () { + sinon.restore(); + }); + + it('1.1 Driver should not throw error on valid SRV URI with one part', async function () { + // 1. make dns resolution always pass + //makeSrvStub(); + // 2. assert that creating a MongoClient with the uri 'mongodb+srv:/localhost' does not cause an error + client = this.configuration.newClient('mongodb://localhost', {}); + // 3. assert that connecting the client from 2. to the server does not cause an error + await client.connect(); + }); - it('1.2 Driver should not throw error on SRV URI with one part', async () => { - // 1. stub dns resolution to always pass - makeSrvStub(); - // 2. assert that creating a MongoClient with the uri 'mongodb+srv//localhost' does not cause an error - const client = new MongoClient('mongodb+srv://localhost', {}); - // 3. assert that connecting the client from 2. to the server does not cause an error - await client.connect(); - }); -}); + it('1.1 Driver should not throw error on valid SRV URI with two parts', async function () { + // 1. make dns resolution always pass + makeSrvStub(); + // 2. assert that creating a MongoClient with the uri 'mongodb+srv://mongodb.localhost' does not cause an error + //const client = new MongoClient('mongodb+srv://mongodb.localhost', {}); + // 3. assert that connecting the client to the server does not cause an error + //await client.connect(); + }); + } +); From 15dc8ee0a94efc6aaca93b17cd9255cb82f647b5 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 5 Sep 2024 00:40:09 -0400 Subject: [PATCH 03/16] added more prose tests --- src/utils.ts | 11 +- .../change-streams/change_stream.test.ts | 1 - .../connection_pool.test.ts | 48 ----- test/integration/crud/bulk.test.ts | 2 +- ...itial_dns_seedlist_discovery.prose.test.ts | 201 +++++++++++++++--- 5 files changed, 176 insertions(+), 87 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index cebb81e0a0d..9c90ce570e5 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1141,14 +1141,21 @@ export function matchesParentDomain(address: string, srvHost: string): boolean { const normalizedSrvHost = srvHost.endsWith('.') ? srvHost.slice(0, srvHost.length - 1) : srvHost; const allCharacterBeforeFirstDot = /^.*?\./; + const srvIsLessThanThreeParts = srvHost.split('.').length < 3; // Remove all characters before first dot // Add leading dot back to string so // an srvHostDomain = '.trusted.site' // will not satisfy an addressDomain that endsWith '.fake-trusted.site' const addressDomain = `.${normalizedAddress.replace(allCharacterBeforeFirstDot, '')}`; - const srvHostDomain = `.${normalizedSrvHost.replace(allCharacterBeforeFirstDot, '')}`; + const srvHostDomain = srvIsLessThanThreeParts + ? normalizedSrvHost + : `.${normalizedSrvHost.replace(allCharacterBeforeFirstDot, '')}`; - return addressDomain.endsWith(srvHostDomain); + return ( + addressDomain.endsWith(srvHostDomain) && + (!srvIsLessThanThreeParts || + normalizedAddress.split('.').length > normalizedSrvHost.split('.').length) + ); } interface RequestOptions { diff --git a/test/integration/change-streams/change_stream.test.ts b/test/integration/change-streams/change_stream.test.ts index aae2a33bfa8..3fa1e792ed0 100644 --- a/test/integration/change-streams/change_stream.test.ts +++ b/test/integration/change-streams/change_stream.test.ts @@ -1,6 +1,5 @@ import { strict as assert } from 'assert'; import { expect } from 'chai'; -import * as dns from 'dns'; import { on, once } from 'events'; import { gte, lt } from 'semver'; import * as sinon from 'sinon'; diff --git a/test/integration/connection-monitoring-and-pooling/connection_pool.test.ts b/test/integration/connection-monitoring-and-pooling/connection_pool.test.ts index e31fe2aeed1..437961b1201 100644 --- a/test/integration/connection-monitoring-and-pooling/connection_pool.test.ts +++ b/test/integration/connection-monitoring-and-pooling/connection_pool.test.ts @@ -3,8 +3,6 @@ import { once } from 'node:events'; import { expect } from 'chai'; import { type ConnectionPoolCreatedEvent, type Db, type MongoClient } from '../../mongodb'; -import sinon = require('sinon'); -import dns = require('dns'); describe('Connection Pool', function () { let client: MongoClient; @@ -21,52 +19,6 @@ describe('Connection Pool', function () { describe('Events', function () { describe('ConnectionPoolCreatedEvent', function () { - describe.only( - 'Initial DNS Seedlist Discovery (Prose Tests)', - { requires: { topology: 'single' } }, - () => { - function makeSrvStub() { - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'localhost', - port: 27017, - weight: 0, - priority: 0 - } - ]; - }); - - sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { - throw { code: 'ENODATA' }; - }); - } - - afterEach(async function () { - sinon.restore(); - }); - - it('1.1 Driver should not throw error on valid SRV URI with one part', async function () { - // 1. make dns resolution always pass - makeSrvStub(); - // 2. assert that creating a MongoClient with the uri 'mongodb+srv:/localhost' does not cause an error - client = this.configuration.newClient('mongodb+srv://localhost', {}); - console.log('stubbed srv client', client); - // 3. assert that connecting the client from 2. to the server does not cause an error - //await client.connect(); - }); - - it('1.1 Driver should not throw error on valid SRV URI with two parts', async function () { - // 1. make dns resolution always pass - makeSrvStub(); - // 2. assert that creating a MongoClient with the uri 'mongodb+srv://mongodb.localhost' does not cause an error - const client = this.configuration.newClient('mongodb://localhost', {}); - console.log('stubbed normal client', client); - // 3. assert that connecting the client to the server does not cause an error - //await client.connect(); - }); - } - ); context('when no connection pool options are passed in', function () { let pConnectionPoolCreated: Promise; let connectionPoolCreated: ConnectionPoolCreatedEvent; diff --git a/test/integration/crud/bulk.test.ts b/test/integration/crud/bulk.test.ts index 6d260dd3fa8..c479b842962 100644 --- a/test/integration/crud/bulk.test.ts +++ b/test/integration/crud/bulk.test.ts @@ -723,7 +723,7 @@ describe('Bulk', function () { } }); - it.only( + it( 'should correctly execute ordered batch using w:0', // TODO(NODE-6060): set `moreToCome` op_msg bit when `w: 0` is specified { requires: { mongodb: '<8.0.0' } }, diff --git a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts index 2a7f8daeee8..290f8cbc0a3 100644 --- a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts +++ b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts @@ -1,52 +1,183 @@ import * as dns from 'dns'; import sinon = require('sinon'); -// import { expect } from 'chai'; -import { type MongoClient } from '../../mongodb'; +import { expect } from 'chai'; + +import { MongoAPIError, Server, ServerDescription, Topology } from '../../mongodb'; +import { topologyWithPlaceholderClient } from '../../tools/utils'; describe( 'Initial DNS Seedlist Discovery (Prose Tests)', { requires: { topology: 'single' } }, () => { - let client: MongoClient; - - function makeSrvStub() { - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'localhost', - port: 27017, - weight: 0, - priority: 0 - } - ]; + context('When running validation on an SRV string before DNS resolution', function () { + beforeEach(async function () { + // this fn stubs DNS resolution to always pass - so we are only checking pre-DNS validation + + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'resolved.mongodb.localhost', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); + + sinon.stub(Topology.prototype, 'selectServer').callsFake(async () => { + return new Server( + topologyWithPlaceholderClient([], {} as any), + new ServerDescription('a:1'), + {} as any + ); + }); }); - sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { - throw { code: 'ENODATA' }; + afterEach(async function () { + sinon.restore(); }); - } - afterEach(async function () { - sinon.restore(); - }); + it('do not error on an SRV because it has one domain level', async function () { + const client = await this.configuration.newClient('mongodb+srv://localhost', {}); + client.connect(); + client.close(); + }); - it('1.1 Driver should not throw error on valid SRV URI with one part', async function () { - // 1. make dns resolution always pass - //makeSrvStub(); - // 2. assert that creating a MongoClient with the uri 'mongodb+srv:/localhost' does not cause an error - client = this.configuration.newClient('mongodb://localhost', {}); - // 3. assert that connecting the client from 2. to the server does not cause an error - await client.connect(); + it('do not error on an SRV because it has two domain levels', async function () { + const client = await this.configuration.newClient('mongodb+srv://mongodb.localhost', {}); + client.connect(); + client.close(); + }); }); - it('1.1 Driver should not throw error on valid SRV URI with two parts', async function () { - // 1. make dns resolution always pass - makeSrvStub(); - // 2. assert that creating a MongoClient with the uri 'mongodb+srv://mongodb.localhost' does not cause an error - //const client = new MongoClient('mongodb+srv://mongodb.localhost', {}); - // 3. assert that connecting the client to the server does not cause an error - //await client.connect(); - }); + context( + 'When given a DNS resolution that does NOT end with the original SRVs domain name', + function () { + beforeEach(async function () { + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); + }); + + afterEach(async function () { + sinon.restore(); + }); + + it('an SRV with one domain level causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'localhost.mongodb', // this string contains the SRV but does not end with it + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://localhost', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + + it('an SRV with two domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'evil.localhost', // this string only ends with part of the domain, not all of it! + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://mongodb.localhost', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + + it('an SRV with three or more domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'blogs.evil.co.uk', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://blogs.mongodb.com', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + } + ); + + context( + 'When given a DNS resolution that is identical to the original SRVs hostname', + function () { + beforeEach(async function () { + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); + }); + + afterEach(async function () { + sinon.restore(); + }); + + it('an SRV with one domain level causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'localhost', // this string contains the SRV but does not end with it + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://localhost', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + + it('an SRV with two domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'mongodb.localhost', // this string only ends with part of the domain, not all of it! + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://mongodb.localhost', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + } + ); } ); From 586f7c024c3212b381cdcff81d55c6e4ca5b6af0 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 5 Sep 2024 01:02:40 -0400 Subject: [PATCH 04/16] wording fix --- .../initial_dns_seedlist_discovery.prose.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts index 290f8cbc0a3..1e7189040f5 100644 --- a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts +++ b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts @@ -56,7 +56,7 @@ describe( }); context( - 'When given a DNS resolution that does NOT end with the original SRVs domain name', + 'When given a host from DNS resolution that does NOT end with the original SRVs domain name', function () { beforeEach(async function () { sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { @@ -128,7 +128,7 @@ describe( ); context( - 'When given a DNS resolution that is identical to the original SRVs hostname', + 'When given a host from DNS resolution that is identical to the original SRVs hostname', function () { beforeEach(async function () { sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { From bea037e22bff14a6c768972f54c43e3d00bc7f74 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 5 Sep 2024 01:04:23 -0400 Subject: [PATCH 05/16] remove stray comment --- .../initial_dns_seedlist_discovery.prose.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts index 1e7189040f5..c3e09011bdb 100644 --- a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts +++ b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts @@ -144,7 +144,7 @@ describe( sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { return [ { - name: 'localhost', // this string contains the SRV but does not end with it + name: 'localhost', port: 27017, weight: 0, priority: 0 @@ -163,7 +163,7 @@ describe( sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { return [ { - name: 'mongodb.localhost', // this string only ends with part of the domain, not all of it! + name: 'mongodb.localhost', port: 27017, weight: 0, priority: 0 From ae79ac59c1c0d9c978a9a47975b399ff214870c8 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 5 Sep 2024 01:31:28 -0400 Subject: [PATCH 06/16] fix up --- src/connection_string.ts | 6 ++--- src/sdam/srv_polling.ts | 7 +++-- src/utils.ts | 27 ++++++++++++------- ...itial_dns_seedlist_discovery.prose.test.ts | 13 +++++---- 4 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index 6ed62cfc605..b857a8d767f 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -34,11 +34,11 @@ import { ReadPreference, type ReadPreferenceMode } from './read_preference'; import { ServerMonitoringMode } from './sdam/monitor'; import type { TagSet } from './sdam/server_description'; import { + checkParentDomainMatch, DEFAULT_PK_FACTORY, emitWarning, HostAddress, isRecord, - matchesParentDomain, parseInteger, setDifference, squashError @@ -81,9 +81,7 @@ export async function resolveSRVRecord(options: MongoOptions): Promise HostAddress.fromString(`${r.name}:${r.port ?? 27017}`)); diff --git a/src/sdam/srv_polling.ts b/src/sdam/srv_polling.ts index 39d393b7830..c95c386cfa7 100644 --- a/src/sdam/srv_polling.ts +++ b/src/sdam/srv_polling.ts @@ -3,7 +3,7 @@ import { clearTimeout, setTimeout } from 'timers'; import { MongoRuntimeError } from '../error'; import { TypedEventEmitter } from '../mongo_types'; -import { HostAddress, matchesParentDomain, squashError } from '../utils'; +import { checkParentDomainMatch, HostAddress, squashError } from '../utils'; /** * @internal @@ -127,8 +127,11 @@ export class SrvPoller extends TypedEventEmitter { const finalAddresses: dns.SrvRecord[] = []; for (const record of srvRecords) { - if (matchesParentDomain(record.name, this.srvHost)) { + try { + checkParentDomainMatch(record.name, this.srvHost); finalAddresses.push(record); + } catch (error) { + squashError(error); } } diff --git a/src/utils.ts b/src/utils.ts index 2b1bb563463..9be7f838640 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -18,6 +18,7 @@ import type { FindCursor } from './cursor/find_cursor'; import type { Db } from './db'; import { type AnyError, + MongoAPIError, MongoCompatibilityError, MongoInvalidArgumentError, MongoNetworkTimeoutError, @@ -1131,16 +1132,18 @@ export function parseUnsignedInteger(value: unknown): number | null { } /** - * Determines whether a provided address matches the provided parent domain. + * This function throws a MongoAPIError in the event that either of the following is true: + * * If the provided address domain does not match the provided parent domain + * * If the parent domain contains less than three `.` separated parts and the provided address does not contain at least one more domain level than its parent * * If a DNS server were to become compromised SRV records would still need to * advertise addresses that are under the same domain as the srvHost. * * @param address - The address to check against a domain * @param srvHost - The domain to check the provided address against - * @returns Whether the provided address matches the parent domain + * @returns void */ -export function matchesParentDomain(address: string, srvHost: string): boolean { +export function checkParentDomainMatch(address: string, srvHost: string): void { // Remove trailing dot if exists on either the resolved address or the srv hostname const normalizedAddress = address.endsWith('.') ? address.slice(0, address.length - 1) : address; const normalizedSrvHost = srvHost.endsWith('.') ? srvHost.slice(0, srvHost.length - 1) : srvHost; @@ -1151,16 +1154,22 @@ export function matchesParentDomain(address: string, srvHost: string): boolean { // Add leading dot back to string so // an srvHostDomain = '.trusted.site' // will not satisfy an addressDomain that endsWith '.fake-trusted.site' - const addressDomain = `.${normalizedAddress.replace(allCharacterBeforeFirstDot, '')}`; + const addressDomain = srvIsLessThanThreeParts + ? normalizedAddress + : `.${normalizedAddress.replace(allCharacterBeforeFirstDot, '')}`; const srvHostDomain = srvIsLessThanThreeParts ? normalizedSrvHost : `.${normalizedSrvHost.replace(allCharacterBeforeFirstDot, '')}`; - return ( - addressDomain.endsWith(srvHostDomain) && - (!srvIsLessThanThreeParts || - normalizedAddress.split('.').length > normalizedSrvHost.split('.').length) - ); + if (!addressDomain.endsWith(srvHostDomain)) { + throw new MongoAPIError('Server record does not share hostname with parent URI'); + } + if ( + srvIsLessThanThreeParts && + normalizedAddress.split('.').length <= normalizedSrvHost.split('.').length + ) { + throw new MongoAPIError('Server record does not have least one more domain than parent URI'); + } } interface RequestOptions { diff --git a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts index c3e09011bdb..a6e0cbe4f79 100644 --- a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts +++ b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts @@ -1,7 +1,6 @@ -import * as dns from 'dns'; -import sinon = require('sinon'); - import { expect } from 'chai'; +import * as dns from 'dns'; +import * as sinon from 'sinon'; import { MongoAPIError, Server, ServerDescription, Topology } from '../../mongodb'; import { topologyWithPlaceholderClient } from '../../tools/utils'; @@ -156,7 +155,9 @@ describe( .connect() .catch(e => e); expect(err).to.be.instanceOf(MongoAPIError); - expect(err.message).to.equal('Server record does not share hostname with parent URI'); + expect(err.message).to.equal( + 'Server record does not have least one more domain than parent URI' + ); }); it('an SRV with two domain levels causes a runtime error', async function () { @@ -175,7 +176,9 @@ describe( .connect() .catch(e => e); expect(err).to.be.instanceOf(MongoAPIError); - expect(err.message).to.equal('Server record does not share hostname with parent URI'); + expect(err.message).to.equal( + 'Server record does not have least one more domain than parent URI' + ); }); } ); From b2f843e42d8151b260dc72d023fa53d36d4197f7 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 5 Sep 2024 01:34:30 -0400 Subject: [PATCH 07/16] add back in comment --- src/utils.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/utils.ts b/src/utils.ts index 9be7f838640..ba79f67b947 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1162,12 +1162,14 @@ export function checkParentDomainMatch(address: string, srvHost: string): void { : `.${normalizedSrvHost.replace(allCharacterBeforeFirstDot, '')}`; if (!addressDomain.endsWith(srvHostDomain)) { + // TODO(NODE-3484): Replace with MongoConnectionStringError throw new MongoAPIError('Server record does not share hostname with parent URI'); } if ( srvIsLessThanThreeParts && normalizedAddress.split('.').length <= normalizedSrvHost.split('.').length ) { + // TODO(NODE-3484): Replace with MongoConnectionStringError throw new MongoAPIError('Server record does not have least one more domain than parent URI'); } } From 154e2adf31032d7de8b36fb756118f2bea75c103 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 5 Sep 2024 14:09:58 -0400 Subject: [PATCH 08/16] fix failing unit tests --- test/unit/utils.test.ts | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts index f796a652807..b0721e522eb 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.ts @@ -3,6 +3,7 @@ import { expect } from 'chai'; import { BufferPool, ByteUtils, + checkParentDomainMatch, compareObjectId, HostAddress, hostMatchesWildcards, @@ -10,7 +11,6 @@ import { isUint8Array, LEGACY_HELLO_COMMAND, List, - matchesParentDomain, MongoDBCollectionNamespace, MongoDBNamespace, MongoRuntimeError, @@ -937,7 +937,7 @@ describe('driver utils', function () { }); }); - describe('matchesParentDomain()', () => { + describe('checkParentDomainMatch()', () => { const exampleSrvName = 'i-love-javascript.mongodb.io'; const exampleSrvNameWithDot = 'i-love-javascript.mongodb.io.'; const exampleHostNameWithoutDot = 'i-love-javascript-00.mongodb.io'; @@ -946,35 +946,40 @@ describe('driver utils', function () { const exampleHostNamThatDoNotMatchParentWithDot = 'i-love-javascript-00.evil-mongodb.io.'; context('when address does not match parent domain', () => { - it('without a trailing dot returns false', () => { - expect(matchesParentDomain(exampleHostNamThatDoNotMatchParent, exampleSrvName)).to.be.false; + it('without a trailing dot throws', () => { + expect(() => + checkParentDomainMatch(exampleHostNamThatDoNotMatchParent, exampleSrvName) + ).to.throw('Server record does not share hostname with parent URI'); }); - it('with a trailing dot returns false', () => { - expect(matchesParentDomain(exampleHostNamThatDoNotMatchParentWithDot, exampleSrvName)).to.be - .false; + it('with a trailing dot throws', () => { + expect(() => + checkParentDomainMatch(exampleHostNamThatDoNotMatchParentWithDot, exampleSrvName) + ).to.throw('Server record does not share hostname with parent URI'); }); }); context('when addresses in SRV record end with a dot', () => { it('accepts address since it is considered to still match the parent domain', () => { - expect(matchesParentDomain(exampleHostNamesWithDot, exampleSrvName)).to.be.true; + expect(() => checkParentDomainMatch(exampleHostNamesWithDot, exampleSrvName)).to.not.throw; }); }); context('when SRV host ends with a dot', () => { it('accepts address if it ends with a dot', () => { - expect(matchesParentDomain(exampleHostNamesWithDot, exampleSrvNameWithDot)).to.be.true; + expect(() => checkParentDomainMatch(exampleHostNamesWithDot, exampleSrvNameWithDot)).to.not + .throw; }); it('accepts address if it does not end with a dot', () => { - expect(matchesParentDomain(exampleHostNameWithoutDot, exampleSrvName)).to.be.true; + expect(() => checkParentDomainMatch(exampleHostNameWithoutDot, exampleSrvName)).to.not + .throw; }); }); context('when addresses in SRV record end without dots', () => { it('accepts address since it matches the parent domain', () => { - expect(matchesParentDomain(exampleHostNamesWithDot, exampleSrvName)).to.be.true; + expect(() => checkParentDomainMatch(exampleHostNamesWithDot, exampleSrvName)).to.not.throw; }); }); }); From 9dd438ee4e2350b278f0cd261bff89d2bb4b7f9b Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 19 Sep 2024 17:48:39 -0400 Subject: [PATCH 09/16] requested changes 1 --- ...itial_dns_seedlist_discovery.prose.test.ts | 306 +++++++++--------- test/unit/utils.test.ts | 18 +- 2 files changed, 163 insertions(+), 161 deletions(-) diff --git a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts index a6e0cbe4f79..7f4a10e2e6b 100644 --- a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts +++ b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts @@ -5,182 +5,178 @@ import * as sinon from 'sinon'; import { MongoAPIError, Server, ServerDescription, Topology } from '../../mongodb'; import { topologyWithPlaceholderClient } from '../../tools/utils'; -describe( - 'Initial DNS Seedlist Discovery (Prose Tests)', - { requires: { topology: 'single' } }, - () => { - context('When running validation on an SRV string before DNS resolution', function () { +describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { + context('1) When running validation on an SRV string before DNS resolution', function () { + beforeEach(async function () { + // this fn stubs DNS resolution to always pass - so we are only checking pre-DNS validation + + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'resolved.mongodb.localhost', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); + + sinon.stub(Topology.prototype, 'selectServer').callsFake(async () => { + return new Server( + topologyWithPlaceholderClient([], {} as any), + new ServerDescription('a:1'), + {} as any + ); + }); + }); + + afterEach(async function () { + sinon.restore(); + }); + + it('does not error on an SRV because it has one domain level', async function () { + const client = await this.configuration.newClient('mongodb+srv://localhost', {}); + client.connect(); + client.close(); + }); + + it('does not error on an SRV because it has two domain levels', async function () { + const client = await this.configuration.newClient('mongodb+srv://mongodb.localhost', {}); + client.connect(); + client.close(); + }); + }); + + context( + '2) When given a host from DNS resolution that does NOT end with the original SRVs domain name', + function () { beforeEach(async function () { - // this fn stubs DNS resolution to always pass - so we are only checking pre-DNS validation + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); + }); + + afterEach(async function () { + sinon.restore(); + }); + it('an SRV with one domain level causes a runtime error', async function () { sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { return [ { - name: 'resolved.mongodb.localhost', + name: 'localhost.mongodb', // this string contains the SRV but does not end with it port: 27017, weight: 0, priority: 0 } ]; }); + const err = await this.configuration + .newClient('mongodb+srv://localhost', {}) + .connect() + .catch((e: any) => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); - sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { - throw { code: 'ENODATA' }; - }); - - sinon.stub(Topology.prototype, 'selectServer').callsFake(async () => { - return new Server( - topologyWithPlaceholderClient([], {} as any), - new ServerDescription('a:1'), - {} as any - ); + it('an SRV with two domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'evil.localhost', // this string only ends with part of the domain, not all of it! + port: 27017, + weight: 0, + priority: 0 + } + ]; }); + const err = await this.configuration + .newClient('mongodb+srv://mongodb.localhost', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); }); - afterEach(async function () { - sinon.restore(); + it('an SRV with three or more domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'blogs.evil.co.uk', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://blogs.mongodb.com', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); }); + } + ); - it('do not error on an SRV because it has one domain level', async function () { - const client = await this.configuration.newClient('mongodb+srv://localhost', {}); - client.connect(); - client.close(); + context( + '3) When given a host from DNS resolution that is identical to the original SRVs hostname', + function () { + beforeEach(async function () { + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); }); - it('do not error on an SRV because it has two domain levels', async function () { - const client = await this.configuration.newClient('mongodb+srv://mongodb.localhost', {}); - client.connect(); - client.close(); + afterEach(async function () { + sinon.restore(); }); - }); - - context( - 'When given a host from DNS resolution that does NOT end with the original SRVs domain name', - function () { - beforeEach(async function () { - sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { - throw { code: 'ENODATA' }; - }); - }); - - afterEach(async function () { - sinon.restore(); - }); - - it('an SRV with one domain level causes a runtime error', async function () { - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'localhost.mongodb', // this string contains the SRV but does not end with it - port: 27017, - weight: 0, - priority: 0 - } - ]; - }); - const err = await this.configuration - .newClient('mongodb+srv://localhost', {}) - .connect() - .catch(e => e); - expect(err).to.be.instanceOf(MongoAPIError); - expect(err.message).to.equal('Server record does not share hostname with parent URI'); - }); - - it('an SRV with two domain levels causes a runtime error', async function () { - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'evil.localhost', // this string only ends with part of the domain, not all of it! - port: 27017, - weight: 0, - priority: 0 - } - ]; - }); - const err = await this.configuration - .newClient('mongodb+srv://mongodb.localhost', {}) - .connect() - .catch(e => e); - expect(err).to.be.instanceOf(MongoAPIError); - expect(err.message).to.equal('Server record does not share hostname with parent URI'); - }); - - it('an SRV with three or more domain levels causes a runtime error', async function () { - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'blogs.evil.co.uk', - port: 27017, - weight: 0, - priority: 0 - } - ]; - }); - const err = await this.configuration - .newClient('mongodb+srv://blogs.mongodb.com', {}) - .connect() - .catch(e => e); - expect(err).to.be.instanceOf(MongoAPIError); - expect(err.message).to.equal('Server record does not share hostname with parent URI'); - }); - } - ); - - context( - 'When given a host from DNS resolution that is identical to the original SRVs hostname', - function () { - beforeEach(async function () { - sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { - throw { code: 'ENODATA' }; - }); - }); - afterEach(async function () { - sinon.restore(); - }); - - it('an SRV with one domain level causes a runtime error', async function () { - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'localhost', - port: 27017, - weight: 0, - priority: 0 - } - ]; - }); - const err = await this.configuration - .newClient('mongodb+srv://localhost', {}) - .connect() - .catch(e => e); - expect(err).to.be.instanceOf(MongoAPIError); - expect(err.message).to.equal( - 'Server record does not have least one more domain than parent URI' - ); + it('an SRV with one domain level causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'localhost', + port: 27017, + weight: 0, + priority: 0 + } + ]; }); + const err = await this.configuration + .newClient('mongodb+srv://localhost', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal( + 'Server record does not have least one more domain than parent URI' + ); + }); - it('an SRV with two domain levels causes a runtime error', async function () { - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'mongodb.localhost', - port: 27017, - weight: 0, - priority: 0 - } - ]; - }); - const err = await this.configuration - .newClient('mongodb+srv://mongodb.localhost', {}) - .connect() - .catch(e => e); - expect(err).to.be.instanceOf(MongoAPIError); - expect(err.message).to.equal( - 'Server record does not have least one more domain than parent URI' - ); + it('an SRV with two domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'mongodb.localhost', + port: 27017, + weight: 0, + priority: 0 + } + ]; }); - } - ); - } -); + const err = await this.configuration + .newClient('mongodb+srv://mongodb.localhost', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal( + 'Server record does not have least one more domain than parent URI' + ); + }); + } + ); +}); diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts index 7937d57a4f4..0b820eea38b 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.ts @@ -963,25 +963,31 @@ describe('driver utils', function () { context('when addresses in SRV record end with a dot', () => { it('accepts address since it is considered to still match the parent domain', () => { - expect(() => checkParentDomainMatch(exampleHostNamesWithDot, exampleSrvName)).to.not.throw; + expect(() => + checkParentDomainMatch(exampleHostNamesWithDot, exampleSrvName) + ).to.not.throw(); }); }); context('when SRV host ends with a dot', () => { it('accepts address if it ends with a dot', () => { - expect(() => checkParentDomainMatch(exampleHostNamesWithDot, exampleSrvNameWithDot)).to.not - .throw; + expect(() => + checkParentDomainMatch(exampleHostNamesWithDot, exampleSrvNameWithDot) + ).to.not.throw(); }); it('accepts address if it does not end with a dot', () => { - expect(() => checkParentDomainMatch(exampleHostNameWithoutDot, exampleSrvName)).to.not - .throw; + expect(() => + checkParentDomainMatch(exampleHostNameWithoutDot, exampleSrvName) + ).to.not.throw(); }); }); context('when addresses in SRV record end without dots', () => { it('accepts address since it matches the parent domain', () => { - expect(() => checkParentDomainMatch(exampleHostNamesWithDot, exampleSrvName)).to.not.throw; + expect(() => + checkParentDomainMatch(exampleHostNamesWithDot, exampleSrvName) + ).to.not.throw(); }); }); }); From f680380304ef2be051e64811119abeeb39b3b1a6 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Tue, 24 Sep 2024 16:59:11 -0400 Subject: [PATCH 10/16] requested changes + review from drivers ticket updates --- src/utils.ts | 13 +-- ...itial_dns_seedlist_discovery.prose.test.ts | 105 +++++++++++++++--- 2 files changed, 95 insertions(+), 23 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index 4ad9d36d8b0..df0fc3704fd 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1165,17 +1165,11 @@ export function checkParentDomainMatch(address: string, srvHost: string): void { // Add leading dot back to string so // an srvHostDomain = '.trusted.site' // will not satisfy an addressDomain that endsWith '.fake-trusted.site' - const addressDomain = srvIsLessThanThreeParts - ? normalizedAddress - : `.${normalizedAddress.replace(allCharacterBeforeFirstDot, '')}`; + const addressDomain = `.${normalizedAddress.replace(allCharacterBeforeFirstDot, '')}`; const srvHostDomain = srvIsLessThanThreeParts ? normalizedSrvHost : `.${normalizedSrvHost.replace(allCharacterBeforeFirstDot, '')}`; - if (!addressDomain.endsWith(srvHostDomain)) { - // TODO(NODE-3484): Replace with MongoConnectionStringError - throw new MongoAPIError('Server record does not share hostname with parent URI'); - } if ( srvIsLessThanThreeParts && normalizedAddress.split('.').length <= normalizedSrvHost.split('.').length @@ -1183,6 +1177,11 @@ export function checkParentDomainMatch(address: string, srvHost: string): void { // TODO(NODE-3484): Replace with MongoConnectionStringError throw new MongoAPIError('Server record does not have least one more domain than parent URI'); } + + if (!('.' + addressDomain).endsWith('.' + srvHostDomain)) { + // TODO(NODE-3484): Replace with MongoConnectionStringError + throw new MongoAPIError('Server record does not share hostname with parent URI'); + } } interface RequestOptions { diff --git a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts index 7f4a10e2e6b..efd0d5c60e5 100644 --- a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts +++ b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts @@ -6,14 +6,16 @@ import { MongoAPIError, Server, ServerDescription, Topology } from '../../mongod import { topologyWithPlaceholderClient } from '../../tools/utils'; describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { - context('1) When running validation on an SRV string before DNS resolution', function () { + context('1. When running validation on an SRV string before DNS resolution', function () { + let client; + beforeEach(async function () { // this fn stubs DNS resolution to always pass - so we are only checking pre-DNS validation sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { return [ { - name: 'resolved.mongodb.localhost', + name: 'resolved.mongo.localhost', port: 27017, weight: 0, priority: 0 @@ -36,23 +38,22 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { afterEach(async function () { sinon.restore(); + client.close(); }); it('does not error on an SRV because it has one domain level', async function () { - const client = await this.configuration.newClient('mongodb+srv://localhost', {}); - client.connect(); - client.close(); + client = await this.configuration.newClient('mongodb+srv://localhost', {}); + await client.connect(); }); it('does not error on an SRV because it has two domain levels', async function () { - const client = await this.configuration.newClient('mongodb+srv://mongodb.localhost', {}); - client.connect(); - client.close(); + client = await this.configuration.newClient('mongodb+srv://mongo.localhost', {}); + await client.connect(); }); }); context( - '2) When given a host from DNS resolution that does NOT end with the original SRVs domain name', + '2. When given a host from DNS resolution that does NOT end with the original SRVs domain name', function () { beforeEach(async function () { sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { @@ -68,7 +69,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { return [ { - name: 'localhost.mongodb', // this string contains the SRV but does not end with it + name: 'localhost.mongodb', port: 27017, weight: 0, priority: 0 @@ -87,7 +88,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { return [ { - name: 'evil.localhost', // this string only ends with part of the domain, not all of it! + name: 'test_1.evil.local', // this string only ends with part of the domain, not all of it! port: 27017, weight: 0, priority: 0 @@ -95,7 +96,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { ]; }); const err = await this.configuration - .newClient('mongodb+srv://mongodb.localhost', {}) + .newClient('mongodb+srv://mongo.local', {}) .connect() .catch(e => e); expect(err).to.be.instanceOf(MongoAPIError); @@ -106,7 +107,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { return [ { - name: 'blogs.evil.co.uk', + name: 'blogs.evil.com', port: 27017, weight: 0, priority: 0 @@ -124,7 +125,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { ); context( - '3) When given a host from DNS resolution that is identical to the original SRVs hostname', + '3. When given a host from DNS resolution that is identical to the original SRVs hostname', function () { beforeEach(async function () { sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { @@ -161,7 +162,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { return [ { - name: 'mongodb.localhost', + name: 'mongo.local', port: 27017, weight: 0, priority: 0 @@ -169,7 +170,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { ]; }); const err = await this.configuration - .newClient('mongodb+srv://mongodb.localhost', {}) + .newClient('mongodb+srv://mongo.local', {}) .connect() .catch(e => e); expect(err).to.be.instanceOf(MongoAPIError); @@ -179,4 +180,76 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { }); } ); + + context( + '4. When given a returned address that does NOT share the domain name of the SRV record because its missing a `.`', + function () { + beforeEach(async function () { + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); + }); + + afterEach(async function () { + sinon.restore(); + }); + + it('an SRV with one domain level causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'test_1.cluster_1localhost', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://localhost', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + + it('an SRV with two domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'test_1.my_hostmongo.local', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://mongo.local', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + + it('an SRV with three domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'cluster.testmongodb.com', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://blogs.mongodb.com', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + } + ); }); From 4d3efd8fcd0b56cec4b3a76e6dbcca44c488e65f Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Wed, 25 Sep 2024 14:00:49 -0400 Subject: [PATCH 11/16] fix failing tests --- src/utils.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index df0fc3704fd..967f27a86dc 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1166,10 +1166,13 @@ export function checkParentDomainMatch(address: string, srvHost: string): void { // an srvHostDomain = '.trusted.site' // will not satisfy an addressDomain that endsWith '.fake-trusted.site' const addressDomain = `.${normalizedAddress.replace(allCharacterBeforeFirstDot, '')}`; - const srvHostDomain = srvIsLessThanThreeParts + let srvHostDomain = srvIsLessThanThreeParts ? normalizedSrvHost : `.${normalizedSrvHost.replace(allCharacterBeforeFirstDot, '')}`; + if (!srvIsLessThanThreeParts && !srvHostDomain.startsWith('.')) { + srvHostDomain = '.' + srvHostDomain; + } if ( srvIsLessThanThreeParts && normalizedAddress.split('.').length <= normalizedSrvHost.split('.').length @@ -1177,8 +1180,7 @@ export function checkParentDomainMatch(address: string, srvHost: string): void { // TODO(NODE-3484): Replace with MongoConnectionStringError throw new MongoAPIError('Server record does not have least one more domain than parent URI'); } - - if (!('.' + addressDomain).endsWith('.' + srvHostDomain)) { + if (!addressDomain.endsWith(srvHostDomain)) { // TODO(NODE-3484): Replace with MongoConnectionStringError throw new MongoAPIError('Server record does not share hostname with parent URI'); } From 062c139c158f654b0aafaf390a523570d19dade1 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Wed, 25 Sep 2024 15:57:10 -0400 Subject: [PATCH 12/16] fix failing tests 2 --- src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.ts b/src/utils.ts index 967f27a86dc..b7fb7ca8920 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1170,7 +1170,7 @@ export function checkParentDomainMatch(address: string, srvHost: string): void { ? normalizedSrvHost : `.${normalizedSrvHost.replace(allCharacterBeforeFirstDot, '')}`; - if (!srvIsLessThanThreeParts && !srvHostDomain.startsWith('.')) { + if (!srvHostDomain.startsWith('.')) { srvHostDomain = '.' + srvHostDomain; } if ( From 78958350fa7ce7a2e4f6e66aa47b027b95752e52 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 26 Sep 2024 14:01:24 -0400 Subject: [PATCH 13/16] await close --- .../initial_dns_seedlist_discovery.prose.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts index efd0d5c60e5..117bff19d6b 100644 --- a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts +++ b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts @@ -38,7 +38,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { afterEach(async function () { sinon.restore(); - client.close(); + await client.close(); }); it('does not error on an SRV because it has one domain level', async function () { From 79f41e44eebf09778d769f3c619ae646ce046d03 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Tue, 8 Oct 2024 16:44:23 -0400 Subject: [PATCH 14/16] ready for rereivew --- src/utils.ts | 6 +- ...itial_dns_seedlist_discovery.prose.test.ts | 4 +- test/unit/utils.test.ts | 99 +++++++++++-------- 3 files changed, 60 insertions(+), 49 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index b7fb7ca8920..917147d1103 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1160,7 +1160,7 @@ export function checkParentDomainMatch(address: string, srvHost: string): void { const normalizedSrvHost = srvHost.endsWith('.') ? srvHost.slice(0, srvHost.length - 1) : srvHost; const allCharacterBeforeFirstDot = /^.*?\./; - const srvIsLessThanThreeParts = srvHost.split('.').length < 3; + const srvIsLessThanThreeParts = normalizedSrvHost.split('.').length < 3; // Remove all characters before first dot // Add leading dot back to string so // an srvHostDomain = '.trusted.site' @@ -1177,11 +1177,9 @@ export function checkParentDomainMatch(address: string, srvHost: string): void { srvIsLessThanThreeParts && normalizedAddress.split('.').length <= normalizedSrvHost.split('.').length ) { - // TODO(NODE-3484): Replace with MongoConnectionStringError - throw new MongoAPIError('Server record does not have least one more domain than parent URI'); + throw new MongoAPIError('Server record does not have at least one more domain level than parent URI'); } if (!addressDomain.endsWith(srvHostDomain)) { - // TODO(NODE-3484): Replace with MongoConnectionStringError throw new MongoAPIError('Server record does not share hostname with parent URI'); } } diff --git a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts index 117bff19d6b..f5e5e7de80f 100644 --- a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts +++ b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts @@ -154,7 +154,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { .catch(e => e); expect(err).to.be.instanceOf(MongoAPIError); expect(err.message).to.equal( - 'Server record does not have least one more domain than parent URI' + 'Server record does not have at least one more domain level than parent URI' ); }); @@ -175,7 +175,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { .catch(e => e); expect(err).to.be.instanceOf(MongoAPIError); expect(err.message).to.equal( - 'Server record does not have least one more domain than parent URI' + 'Server record does not have at least one more domain level than parent URI' ); }); } diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts index 0b820eea38b..af65ed646c7 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.ts @@ -940,56 +940,69 @@ describe('driver utils', function () { }); describe('checkParentDomainMatch()', () => { - const exampleSrvName = 'i-love-javascript.mongodb.io'; - const exampleSrvNameWithDot = 'i-love-javascript.mongodb.io.'; - const exampleHostNameWithoutDot = 'i-love-javascript-00.mongodb.io'; - const exampleHostNamesWithDot = exampleHostNameWithoutDot + '.'; - const exampleHostNamThatDoNotMatchParent = 'i-love-javascript-00.evil-mongodb.io'; - const exampleHostNamThatDoNotMatchParentWithDot = 'i-love-javascript-00.evil-mongodb.io.'; - context('when address does not match parent domain', () => { - it('without a trailing dot throws', () => { - expect(() => - checkParentDomainMatch(exampleHostNamThatDoNotMatchParent, exampleSrvName) - ).to.throw('Server record does not share hostname with parent URI'); - }); + const exampleSrvName = ['i-love-js', 'i-love-js.mongodb', 'i-love-javascript.mongodb.io']; + const exampleSrvNameWithDot = ['i-love-js.', 'i-love-js.mongodb.', 'i-love-javascript.mongodb.io.']; + const exampleHostNameWithoutDot = ['js-00.i-love-js', 'js-00.i-love-js.mongodb', 'i-love-javascript-00.mongodb.io']; + const exampleHostNamesWithDot = ['js-00.i-love-js.', 'js-00.i-love-js.mongodb.', 'i-love-javascript-00.mongodb.io.']; + const exampleHostNameThatDoNotMatchParent = ['js-00.i-love-js-a-little','js-00.i-love-js-a-little.mongodb', 'i-love-javascript-00.evil-mongodb.io']; + const exampleHostNameThatDoNotMatchParentWithDot = ['i-love-js','', 'i-love-javascript-00.evil-mongodb.io.']; + + for (let num = 0; num < 3; num += 1) { + context(`when srvName has ${num + 1} part${num !== 0 ? 's' : ''}`, () => { + context('when address does not match parent domain', () => { + it('without a trailing dot throws', () => { + expect(() => + checkParentDomainMatch(exampleHostNameThatDoNotMatchParent[num], exampleSrvName[num]) + ).to.throw('Server record does not share hostname with parent URI'); + }); - it('with a trailing dot throws', () => { - expect(() => - checkParentDomainMatch(exampleHostNamThatDoNotMatchParentWithDot, exampleSrvName) - ).to.throw('Server record does not share hostname with parent URI'); - }); - }); + it('with a trailing dot throws', () => { + expect(() => + checkParentDomainMatch(exampleHostNameThatDoNotMatchParentWithDot[num], exampleSrvName[num]) + ).to.throw(); + }); + }); - context('when addresses in SRV record end with a dot', () => { - it('accepts address since it is considered to still match the parent domain', () => { - expect(() => - checkParentDomainMatch(exampleHostNamesWithDot, exampleSrvName) - ).to.not.throw(); - }); - }); + context('when addresses in SRV record end with a dot', () => { + it('accepts address since it is considered to still match the parent domain', () => { + expect(() => + checkParentDomainMatch(exampleHostNamesWithDot[num], exampleSrvName[num]) + ).to.not.throw(); + }); + }); - context('when SRV host ends with a dot', () => { - it('accepts address if it ends with a dot', () => { - expect(() => - checkParentDomainMatch(exampleHostNamesWithDot, exampleSrvNameWithDot) - ).to.not.throw(); - }); + context('when SRV host ends with a dot', () => { + it('accepts address if it ends with a dot', () => { + expect(() => + checkParentDomainMatch(exampleHostNamesWithDot[num], exampleSrvNameWithDot[num]) + ).to.not.throw(); + }); - it('accepts address if it does not end with a dot', () => { - expect(() => - checkParentDomainMatch(exampleHostNameWithoutDot, exampleSrvName) - ).to.not.throw(); - }); - }); + it('accepts address if it does not end with a dot', () => { + expect(() => + checkParentDomainMatch(exampleHostNameWithoutDot[num], exampleSrvNameWithDot[num]) + ).to.not.throw(); + }); + + if (num < 2) { + it('does not accept address if it does not contain an extra domain level', () => { + expect(() => + checkParentDomainMatch(exampleSrvNameWithDot[num], exampleSrvNameWithDot[num]) + ).to.throw('Server record does not have at least one more domain level than parent URI'); + }); + } + }); - context('when addresses in SRV record end without dots', () => { - it('accepts address since it matches the parent domain', () => { - expect(() => - checkParentDomainMatch(exampleHostNamesWithDot, exampleSrvName) - ).to.not.throw(); + context('when addresses in SRV record end without dots', () => { + it('accepts address since it matches the parent domain', () => { + expect(() => + checkParentDomainMatch(exampleHostNamesWithDot[num], exampleSrvName[num]) + ).to.not.throw(); + }); + }); }); - }); + } }); describe('isUint8Array()', () => { From eafaf61b38ae3e6e2c330ae0d15121366854c954 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Tue, 8 Oct 2024 16:45:47 -0400 Subject: [PATCH 15/16] ready for rereivew --- src/utils.ts | 4 +++- test/unit/utils.test.ts | 40 ++++++++++++++++++++++++++++++++-------- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index 917147d1103..6d5a67d3ab9 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1177,7 +1177,9 @@ export function checkParentDomainMatch(address: string, srvHost: string): void { srvIsLessThanThreeParts && normalizedAddress.split('.').length <= normalizedSrvHost.split('.').length ) { - throw new MongoAPIError('Server record does not have at least one more domain level than parent URI'); + throw new MongoAPIError( + 'Server record does not have at least one more domain level than parent URI' + ); } if (!addressDomain.endsWith(srvHostDomain)) { throw new MongoAPIError('Server record does not share hostname with parent URI'); diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts index af65ed646c7..b15dbdf1f16 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.ts @@ -940,13 +940,32 @@ describe('driver utils', function () { }); describe('checkParentDomainMatch()', () => { - const exampleSrvName = ['i-love-js', 'i-love-js.mongodb', 'i-love-javascript.mongodb.io']; - const exampleSrvNameWithDot = ['i-love-js.', 'i-love-js.mongodb.', 'i-love-javascript.mongodb.io.']; - const exampleHostNameWithoutDot = ['js-00.i-love-js', 'js-00.i-love-js.mongodb', 'i-love-javascript-00.mongodb.io']; - const exampleHostNamesWithDot = ['js-00.i-love-js.', 'js-00.i-love-js.mongodb.', 'i-love-javascript-00.mongodb.io.']; - const exampleHostNameThatDoNotMatchParent = ['js-00.i-love-js-a-little','js-00.i-love-js-a-little.mongodb', 'i-love-javascript-00.evil-mongodb.io']; - const exampleHostNameThatDoNotMatchParentWithDot = ['i-love-js','', 'i-love-javascript-00.evil-mongodb.io.']; + const exampleSrvNameWithDot = [ + 'i-love-js.', + 'i-love-js.mongodb.', + 'i-love-javascript.mongodb.io.' + ]; + const exampleHostNameWithoutDot = [ + 'js-00.i-love-js', + 'js-00.i-love-js.mongodb', + 'i-love-javascript-00.mongodb.io' + ]; + const exampleHostNamesWithDot = [ + 'js-00.i-love-js.', + 'js-00.i-love-js.mongodb.', + 'i-love-javascript-00.mongodb.io.' + ]; + const exampleHostNameThatDoNotMatchParent = [ + 'js-00.i-love-js-a-little', + 'js-00.i-love-js-a-little.mongodb', + 'i-love-javascript-00.evil-mongodb.io' + ]; + const exampleHostNameThatDoNotMatchParentWithDot = [ + 'i-love-js', + '', + 'i-love-javascript-00.evil-mongodb.io.' + ]; for (let num = 0; num < 3; num += 1) { context(`when srvName has ${num + 1} part${num !== 0 ? 's' : ''}`, () => { @@ -959,7 +978,10 @@ describe('driver utils', function () { it('with a trailing dot throws', () => { expect(() => - checkParentDomainMatch(exampleHostNameThatDoNotMatchParentWithDot[num], exampleSrvName[num]) + checkParentDomainMatch( + exampleHostNameThatDoNotMatchParentWithDot[num], + exampleSrvName[num] + ) ).to.throw(); }); }); @@ -989,7 +1011,9 @@ describe('driver utils', function () { it('does not accept address if it does not contain an extra domain level', () => { expect(() => checkParentDomainMatch(exampleSrvNameWithDot[num], exampleSrvNameWithDot[num]) - ).to.throw('Server record does not have at least one more domain level than parent URI'); + ).to.throw( + 'Server record does not have at least one more domain level than parent URI' + ); }); } }); From ec63d530340f2d37c8f0dea6492fa301499e080c Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Mon, 14 Oct 2024 13:09:49 -0400 Subject: [PATCH 16/16] add in comments of prose tests --- ...itial_dns_seedlist_discovery.prose.test.ts | 460 ++++++++++-------- 1 file changed, 249 insertions(+), 211 deletions(-) diff --git a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts index f5e5e7de80f..a4fba77f068 100644 --- a/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts +++ b/test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts @@ -6,250 +6,288 @@ import { MongoAPIError, Server, ServerDescription, Topology } from '../../mongod import { topologyWithPlaceholderClient } from '../../tools/utils'; describe('Initial DNS Seedlist Discovery (Prose Tests)', () => { - context('1. When running validation on an SRV string before DNS resolution', function () { - let client; - - beforeEach(async function () { - // this fn stubs DNS resolution to always pass - so we are only checking pre-DNS validation - - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'resolved.mongo.localhost', - port: 27017, - weight: 0, - priority: 0 - } - ]; - }); - - sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { - throw { code: 'ENODATA' }; - }); - - sinon.stub(Topology.prototype, 'selectServer').callsFake(async () => { - return new Server( - topologyWithPlaceholderClient([], {} as any), - new ServerDescription('a:1'), - {} as any - ); - }); - }); - - afterEach(async function () { - sinon.restore(); - await client.close(); - }); + describe('1. Allow SRVs with fewer than 3 . separated parts', function () { + context('when running validation on an SRV string before DNS resolution', function () { + /** + * When running validation on an SRV string before DNS resolution, do not throw a error due to number of SRV parts. + * - mongodb+srv://localhost + * - mongodb+srv://mongo.localhost + */ - it('does not error on an SRV because it has one domain level', async function () { - client = await this.configuration.newClient('mongodb+srv://localhost', {}); - await client.connect(); - }); - - it('does not error on an SRV because it has two domain levels', async function () { - client = await this.configuration.newClient('mongodb+srv://mongo.localhost', {}); - await client.connect(); - }); - }); + let client; - context( - '2. When given a host from DNS resolution that does NOT end with the original SRVs domain name', - function () { beforeEach(async function () { - sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { - throw { code: 'ENODATA' }; - }); - }); + // this fn stubs DNS resolution to always pass - so we are only checking pre-DNS validation - afterEach(async function () { - sinon.restore(); - }); - - it('an SRV with one domain level causes a runtime error', async function () { - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'localhost.mongodb', - port: 27017, - weight: 0, - priority: 0 - } - ]; - }); - const err = await this.configuration - .newClient('mongodb+srv://localhost', {}) - .connect() - .catch((e: any) => e); - expect(err).to.be.instanceOf(MongoAPIError); - expect(err.message).to.equal('Server record does not share hostname with parent URI'); - }); - - it('an SRV with two domain levels causes a runtime error', async function () { sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { return [ { - name: 'test_1.evil.local', // this string only ends with part of the domain, not all of it! + name: 'resolved.mongo.localhost', port: 27017, weight: 0, priority: 0 } ]; }); - const err = await this.configuration - .newClient('mongodb+srv://mongo.local', {}) - .connect() - .catch(e => e); - expect(err).to.be.instanceOf(MongoAPIError); - expect(err.message).to.equal('Server record does not share hostname with parent URI'); - }); - it('an SRV with three or more domain levels causes a runtime error', async function () { - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'blogs.evil.com', - port: 27017, - weight: 0, - priority: 0 - } - ]; - }); - const err = await this.configuration - .newClient('mongodb+srv://blogs.mongodb.com', {}) - .connect() - .catch(e => e); - expect(err).to.be.instanceOf(MongoAPIError); - expect(err.message).to.equal('Server record does not share hostname with parent URI'); - }); - } - ); - - context( - '3. When given a host from DNS resolution that is identical to the original SRVs hostname', - function () { - beforeEach(async function () { sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { throw { code: 'ENODATA' }; }); + + sinon.stub(Topology.prototype, 'selectServer').callsFake(async () => { + return new Server( + topologyWithPlaceholderClient([], {} as any), + new ServerDescription('a:1'), + {} as any + ); + }); }); afterEach(async function () { sinon.restore(); + await client.close(); }); - it('an SRV with one domain level causes a runtime error', async function () { - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'localhost', - port: 27017, - weight: 0, - priority: 0 - } - ]; - }); - const err = await this.configuration - .newClient('mongodb+srv://localhost', {}) - .connect() - .catch(e => e); - expect(err).to.be.instanceOf(MongoAPIError); - expect(err.message).to.equal( - 'Server record does not have at least one more domain level than parent URI' - ); + it('does not error on an SRV because it has one domain level', async function () { + client = await this.configuration.newClient('mongodb+srv://localhost', {}); + await client.connect(); }); - it('an SRV with two domain levels causes a runtime error', async function () { - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'mongo.local', - port: 27017, - weight: 0, - priority: 0 - } - ]; - }); - const err = await this.configuration - .newClient('mongodb+srv://mongo.local', {}) - .connect() - .catch(e => e); - expect(err).to.be.instanceOf(MongoAPIError); - expect(err.message).to.equal( - 'Server record does not have at least one more domain level than parent URI' - ); + it('does not error on an SRV because it has two domain levels', async function () { + client = await this.configuration.newClient('mongodb+srv://mongo.localhost', {}); + await client.connect(); }); - } - ); + }); + }); - context( - '4. When given a returned address that does NOT share the domain name of the SRV record because its missing a `.`', - function () { - beforeEach(async function () { - sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { - throw { code: 'ENODATA' }; + describe('2. Throw when return address does not end with SRV domain', function () { + context( + 'when given a host from DNS resolution that does NOT end with the original SRVs domain name', + function () { + /** + * When given a returned address that does NOT end with the original SRV's domain name, throw a runtime error. + * For this test, run each of the following cases: + * - the SRV mongodb+srv://localhost resolving to localhost.mongodb + * - the SRV mongodb+srv://mongo.local resolving to test_1.evil.local + * - the SRV mongodb+srv://blogs.mongodb.com resolving to blogs.evil.com + * Remember, the domain of an SRV with one or two . separated parts is the SRVs entire hostname. + */ + + beforeEach(async function () { + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); }); - }); - afterEach(async function () { - sinon.restore(); - }); + afterEach(async function () { + sinon.restore(); + }); - it('an SRV with one domain level causes a runtime error', async function () { - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'test_1.cluster_1localhost', - port: 27017, - weight: 0, - priority: 0 - } - ]; + it('an SRV with one domain level causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'localhost.mongodb', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://localhost', {}) + .connect() + .catch((e: any) => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); }); - const err = await this.configuration - .newClient('mongodb+srv://localhost', {}) - .connect() - .catch(e => e); - expect(err).to.be.instanceOf(MongoAPIError); - expect(err.message).to.equal('Server record does not share hostname with parent URI'); - }); - it('an SRV with two domain levels causes a runtime error', async function () { - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'test_1.my_hostmongo.local', - port: 27017, - weight: 0, - priority: 0 - } - ]; + it('an SRV with two domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'test_1.evil.local', // this string only ends with part of the domain, not all of it! + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://mongo.local', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); }); - const err = await this.configuration - .newClient('mongodb+srv://mongo.local', {}) - .connect() - .catch(e => e); - expect(err).to.be.instanceOf(MongoAPIError); - expect(err.message).to.equal('Server record does not share hostname with parent URI'); - }); - it('an SRV with three domain levels causes a runtime error', async function () { - sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { - return [ - { - name: 'cluster.testmongodb.com', - port: 27017, - weight: 0, - priority: 0 - } - ]; + it('an SRV with three or more domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'blogs.evil.com', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://blogs.mongodb.com', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); }); - const err = await this.configuration - .newClient('mongodb+srv://blogs.mongodb.com', {}) - .connect() - .catch(e => e); - expect(err).to.be.instanceOf(MongoAPIError); - expect(err.message).to.equal('Server record does not share hostname with parent URI'); - }); - } - ); + } + ); + }); + + describe('3. Throw when return address is identical to SRV hostname', function () { + /** + * When given a returned address that is identical to the SRV hostname and the SRV hostname has fewer than three . separated parts, throw a runtime error. + * For this test, run each of the following cases: + * - the SRV mongodb+srv://localhost resolving to localhost + * - the SRV mongodb+srv://mongo.local resolving to mongo.local + */ + + context( + 'when given a host from DNS resolution that is identical to the original SRVs hostname', + function () { + beforeEach(async function () { + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); + }); + + afterEach(async function () { + sinon.restore(); + }); + + it('an SRV with one domain level causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'localhost', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://localhost', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal( + 'Server record does not have at least one more domain level than parent URI' + ); + }); + + it('an SRV with two domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'mongo.local', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://mongo.local', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal( + 'Server record does not have at least one more domain level than parent URI' + ); + }); + } + ); + }); + + describe('4. Throw when return address does not contain . separating shared part of domain', function () { + /** + * When given a returned address that does NOT share the domain name of the SRV record because it's missing a ., throw a runtime error. + * For this test, run each of the following cases: + * - the SRV mongodb+srv://localhost resolving to test_1.cluster_1localhost + * - the SRV mongodb+srv://mongo.local resolving to test_1.my_hostmongo.local + * - the SRV mongodb+srv://blogs.mongodb.com resolving to cluster.testmongodb.com + */ + + context( + 'when given a returned address that does NOT share the domain name of the SRV record because its missing a `.`', + function () { + beforeEach(async function () { + sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => { + throw { code: 'ENODATA' }; + }); + }); + + afterEach(async function () { + sinon.restore(); + }); + + it('an SRV with one domain level causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'test_1.cluster_1localhost', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://localhost', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + + it('an SRV with two domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'test_1.my_hostmongo.local', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://mongo.local', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + + it('an SRV with three domain levels causes a runtime error', async function () { + sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => { + return [ + { + name: 'cluster.testmongodb.com', + port: 27017, + weight: 0, + priority: 0 + } + ]; + }); + const err = await this.configuration + .newClient('mongodb+srv://blogs.mongodb.com', {}) + .connect() + .catch(e => e); + expect(err).to.be.instanceOf(MongoAPIError); + expect(err.message).to.equal('Server record does not share hostname with parent URI'); + }); + } + ); + }); });