Skip to content

Commit f680380

Browse files
requested changes + review from drivers ticket updates
1 parent 9dd438e commit f680380

File tree

2 files changed

+95
-23
lines changed

2 files changed

+95
-23
lines changed

src/utils.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,24 +1165,23 @@ export function checkParentDomainMatch(address: string, srvHost: string): void {
11651165
// Add leading dot back to string so
11661166
// an srvHostDomain = '.trusted.site'
11671167
// will not satisfy an addressDomain that endsWith '.fake-trusted.site'
1168-
const addressDomain = srvIsLessThanThreeParts
1169-
? normalizedAddress
1170-
: `.${normalizedAddress.replace(allCharacterBeforeFirstDot, '')}`;
1168+
const addressDomain = `.${normalizedAddress.replace(allCharacterBeforeFirstDot, '')}`;
11711169
const srvHostDomain = srvIsLessThanThreeParts
11721170
? normalizedSrvHost
11731171
: `.${normalizedSrvHost.replace(allCharacterBeforeFirstDot, '')}`;
11741172

1175-
if (!addressDomain.endsWith(srvHostDomain)) {
1176-
// TODO(NODE-3484): Replace with MongoConnectionStringError
1177-
throw new MongoAPIError('Server record does not share hostname with parent URI');
1178-
}
11791173
if (
11801174
srvIsLessThanThreeParts &&
11811175
normalizedAddress.split('.').length <= normalizedSrvHost.split('.').length
11821176
) {
11831177
// TODO(NODE-3484): Replace with MongoConnectionStringError
11841178
throw new MongoAPIError('Server record does not have least one more domain than parent URI');
11851179
}
1180+
1181+
if (!('.' + addressDomain).endsWith('.' + srvHostDomain)) {
1182+
// TODO(NODE-3484): Replace with MongoConnectionStringError
1183+
throw new MongoAPIError('Server record does not share hostname with parent URI');
1184+
}
11861185
}
11871186

11881187
interface RequestOptions {

test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts

Lines changed: 89 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,16 @@ import { MongoAPIError, Server, ServerDescription, Topology } from '../../mongod
66
import { topologyWithPlaceholderClient } from '../../tools/utils';
77

88
describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
9-
context('1) When running validation on an SRV string before DNS resolution', function () {
9+
context('1. When running validation on an SRV string before DNS resolution', function () {
10+
let client;
11+
1012
beforeEach(async function () {
1113
// this fn stubs DNS resolution to always pass - so we are only checking pre-DNS validation
1214

1315
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
1416
return [
1517
{
16-
name: 'resolved.mongodb.localhost',
18+
name: 'resolved.mongo.localhost',
1719
port: 27017,
1820
weight: 0,
1921
priority: 0
@@ -36,23 +38,22 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
3638

3739
afterEach(async function () {
3840
sinon.restore();
41+
client.close();
3942
});
4043

4144
it('does not error on an SRV because it has one domain level', async function () {
42-
const client = await this.configuration.newClient('mongodb+srv://localhost', {});
43-
client.connect();
44-
client.close();
45+
client = await this.configuration.newClient('mongodb+srv://localhost', {});
46+
await client.connect();
4547
});
4648

4749
it('does not error on an SRV because it has two domain levels', async function () {
48-
const client = await this.configuration.newClient('mongodb+srv://mongodb.localhost', {});
49-
client.connect();
50-
client.close();
50+
client = await this.configuration.newClient('mongodb+srv://mongo.localhost', {});
51+
await client.connect();
5152
});
5253
});
5354

5455
context(
55-
'2) When given a host from DNS resolution that does NOT end with the original SRVs domain name',
56+
'2. When given a host from DNS resolution that does NOT end with the original SRVs domain name',
5657
function () {
5758
beforeEach(async function () {
5859
sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => {
@@ -68,7 +69,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
6869
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
6970
return [
7071
{
71-
name: 'localhost.mongodb', // this string contains the SRV but does not end with it
72+
name: 'localhost.mongodb',
7273
port: 27017,
7374
weight: 0,
7475
priority: 0
@@ -87,15 +88,15 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
8788
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
8889
return [
8990
{
90-
name: 'evil.localhost', // this string only ends with part of the domain, not all of it!
91+
name: 'test_1.evil.local', // this string only ends with part of the domain, not all of it!
9192
port: 27017,
9293
weight: 0,
9394
priority: 0
9495
}
9596
];
9697
});
9798
const err = await this.configuration
98-
.newClient('mongodb+srv://mongodb.localhost', {})
99+
.newClient('mongodb+srv://mongo.local', {})
99100
.connect()
100101
.catch(e => e);
101102
expect(err).to.be.instanceOf(MongoAPIError);
@@ -106,7 +107,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
106107
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
107108
return [
108109
{
109-
name: 'blogs.evil.co.uk',
110+
name: 'blogs.evil.com',
110111
port: 27017,
111112
weight: 0,
112113
priority: 0
@@ -124,7 +125,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
124125
);
125126

126127
context(
127-
'3) When given a host from DNS resolution that is identical to the original SRVs hostname',
128+
'3. When given a host from DNS resolution that is identical to the original SRVs hostname',
128129
function () {
129130
beforeEach(async function () {
130131
sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => {
@@ -161,15 +162,15 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
161162
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
162163
return [
163164
{
164-
name: 'mongodb.localhost',
165+
name: 'mongo.local',
165166
port: 27017,
166167
weight: 0,
167168
priority: 0
168169
}
169170
];
170171
});
171172
const err = await this.configuration
172-
.newClient('mongodb+srv://mongodb.localhost', {})
173+
.newClient('mongodb+srv://mongo.local', {})
173174
.connect()
174175
.catch(e => e);
175176
expect(err).to.be.instanceOf(MongoAPIError);
@@ -179,4 +180,76 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
179180
});
180181
}
181182
);
183+
184+
context(
185+
'4. When given a returned address that does NOT share the domain name of the SRV record because its missing a `.`',
186+
function () {
187+
beforeEach(async function () {
188+
sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => {
189+
throw { code: 'ENODATA' };
190+
});
191+
});
192+
193+
afterEach(async function () {
194+
sinon.restore();
195+
});
196+
197+
it('an SRV with one domain level causes a runtime error', async function () {
198+
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
199+
return [
200+
{
201+
name: 'test_1.cluster_1localhost',
202+
port: 27017,
203+
weight: 0,
204+
priority: 0
205+
}
206+
];
207+
});
208+
const err = await this.configuration
209+
.newClient('mongodb+srv://localhost', {})
210+
.connect()
211+
.catch(e => e);
212+
expect(err).to.be.instanceOf(MongoAPIError);
213+
expect(err.message).to.equal('Server record does not share hostname with parent URI');
214+
});
215+
216+
it('an SRV with two domain levels causes a runtime error', async function () {
217+
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
218+
return [
219+
{
220+
name: 'test_1.my_hostmongo.local',
221+
port: 27017,
222+
weight: 0,
223+
priority: 0
224+
}
225+
];
226+
});
227+
const err = await this.configuration
228+
.newClient('mongodb+srv://mongo.local', {})
229+
.connect()
230+
.catch(e => e);
231+
expect(err).to.be.instanceOf(MongoAPIError);
232+
expect(err.message).to.equal('Server record does not share hostname with parent URI');
233+
});
234+
235+
it('an SRV with three domain levels causes a runtime error', async function () {
236+
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
237+
return [
238+
{
239+
name: 'cluster.testmongodb.com',
240+
port: 27017,
241+
weight: 0,
242+
priority: 0
243+
}
244+
];
245+
});
246+
const err = await this.configuration
247+
.newClient('mongodb+srv://blogs.mongodb.com', {})
248+
.connect()
249+
.catch(e => e);
250+
expect(err).to.be.instanceOf(MongoAPIError);
251+
expect(err.message).to.equal('Server record does not share hostname with parent URI');
252+
});
253+
}
254+
);
182255
});

0 commit comments

Comments
 (0)