Skip to content

Commit f94c18c

Browse files
committed
fix(NODE-4621): ipv6 address handling in HostAddress
1 parent 085471d commit f94c18c

File tree

11 files changed

+214
-73
lines changed

11 files changed

+214
-73
lines changed

src/cmap/connection.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -655,8 +655,9 @@ function streamIdentifier(stream: Stream, options: ConnectionOptions): string {
655655
return options.hostAddress.toString();
656656
}
657657

658-
if (typeof stream.address === 'function') {
659-
return `${stream.remoteAddress}:${stream.remotePort}`;
658+
const { remoteAddress, remotePort } = stream;
659+
if (typeof remoteAddress === 'string' && typeof remotePort === 'number') {
660+
return HostAddress.fromHostPort(remoteAddress, remotePort).toString();
660661
}
661662

662663
return uuidV4().toString('hex');

src/sdam/server_description.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ export class ServerDescription {
8888

8989
this.address =
9090
typeof address === 'string'
91-
? HostAddress.fromString(address).toString(false) // Use HostAddress to normalize
92-
: address.toString(false);
91+
? HostAddress.fromString(address).toString() // Use HostAddress to normalize
92+
: address.toString();
9393
this.type = parseServerType(hello, options);
9494
this.hosts = hello?.hosts?.map((host: string) => host.toLowerCase()) ?? [];
9595
this.passives = hello?.passives?.map((host: string) => host.toLowerCase()) ?? [];

src/utils.ts

Lines changed: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1142,44 +1142,55 @@ export class BufferPool {
11421142

11431143
/** @public */
11441144
export class HostAddress {
1145-
host;
1146-
port;
1145+
host: string | undefined;
1146+
port: number | undefined;
11471147
// Driver only works with unix socket path to connect
11481148
// SDAM operates only on tcp addresses
1149-
socketPath;
1150-
isIPv6;
1149+
socketPath: string | undefined;
1150+
isIPv6 = false;
11511151

11521152
constructor(hostString: string) {
11531153
const escapedHost = hostString.split(' ').join('%20'); // escape spaces, for socket path hosts
1154-
const { hostname, port } = new URL(`mongodb://${escapedHost}`);
11551154

11561155
if (escapedHost.endsWith('.sock')) {
11571156
// heuristically determine if we're working with a domain socket
11581157
this.socketPath = decodeURIComponent(escapedHost);
1159-
} else if (typeof hostname === 'string') {
1160-
this.isIPv6 = false;
1158+
delete this.port;
1159+
delete this.host;
1160+
return;
1161+
}
11611162

1162-
let normalized = decodeURIComponent(hostname).toLowerCase();
1163-
if (normalized.startsWith('[') && normalized.endsWith(']')) {
1164-
this.isIPv6 = true;
1165-
normalized = normalized.substring(1, hostname.length - 1);
1166-
}
1163+
const urlString = `iLoveJS://${escapedHost}`;
1164+
let url;
1165+
try {
1166+
url = new URL(urlString);
1167+
} catch (urlError) {
1168+
const runtimeError = new MongoRuntimeError(`Unable to parse ${escapedHost} with URL`);
1169+
runtimeError.cause = urlError;
1170+
throw runtimeError;
1171+
}
11671172

1168-
this.host = normalized.toLowerCase();
1173+
const hostname = url.hostname;
1174+
const port = url.port;
11691175

1170-
if (typeof port === 'number') {
1171-
this.port = port;
1172-
} else if (typeof port === 'string' && port !== '') {
1173-
this.port = Number.parseInt(port, 10);
1174-
} else {
1175-
this.port = 27017;
1176-
}
1176+
let normalized = decodeURIComponent(hostname).toLowerCase();
1177+
if (normalized.startsWith('[') && normalized.endsWith(']')) {
1178+
this.isIPv6 = true;
1179+
normalized = normalized.substring(1, hostname.length - 1);
1180+
}
11771181

1178-
if (this.port === 0) {
1179-
throw new MongoParseError('Invalid port (zero) with hostname');
1180-
}
1182+
this.host = normalized.toLowerCase();
1183+
1184+
if (typeof port === 'number') {
1185+
this.port = port;
1186+
} else if (typeof port === 'string' && port !== '') {
1187+
this.port = Number.parseInt(port, 10);
11811188
} else {
1182-
throw new MongoInvalidArgumentError('Either socketPath or host must be defined.');
1189+
this.port = 27017;
1190+
}
1191+
1192+
if (this.port === 0) {
1193+
throw new MongoParseError('Invalid port (zero) with hostname');
11831194
}
11841195
Object.freeze(this);
11851196
}
@@ -1189,15 +1200,12 @@ export class HostAddress {
11891200
}
11901201

11911202
inspect(): string {
1192-
return `new HostAddress('${this.toString(true)}')`;
1203+
return `new HostAddress('${this.toString()}')`;
11931204
}
11941205

1195-
/**
1196-
* @param ipv6Brackets - optionally request ipv6 bracket notation required for connection strings
1197-
*/
1198-
toString(ipv6Brackets = false): string {
1206+
toString(): string {
11991207
if (typeof this.host === 'string') {
1200-
if (this.isIPv6 && ipv6Brackets) {
1208+
if (this.isIPv6) {
12011209
return `[${this.host}]:${this.port}`;
12021210
}
12031211
return `${this.host}:${this.port}`;

test/integration/crud/misc_cursors.test.js

Lines changed: 11 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -264,39 +264,21 @@ describe('Cursor', function () {
264264
}
265265
});
266266

267-
it('Should correctly execute cursor count with secondary readPreference', {
268-
// Add a tag that our runner can trigger on
269-
// in this case we are setting that node needs to be higher than 0.10.X to run
270-
metadata: {
271-
requires: { topology: 'replicaset' }
272-
},
273-
274-
test: function (done) {
275-
const configuration = this.configuration;
276-
const client = configuration.newClient(configuration.writeConcernMax(), {
277-
maxPoolSize: 1,
278-
monitorCommands: true
279-
});
280-
267+
it('should correctly execute cursor count with secondary readPreference', {
268+
metadata: { requires: { topology: 'replicaset' } },
269+
async test() {
281270
const bag = [];
282271
client.on('commandStarted', filterForCommands(['count'], bag));
283272

284-
client.connect((err, client) => {
285-
expect(err).to.not.exist;
286-
this.defer(() => client.close());
287-
288-
const db = client.db(configuration.db);
289-
const cursor = db.collection('countTEST').find({ qty: { $gt: 4 } });
290-
cursor.count({ readPreference: ReadPreference.SECONDARY }, err => {
291-
expect(err).to.not.exist;
292-
293-
const selectedServerAddress = bag[0].address.replace('127.0.0.1', 'localhost');
294-
const selectedServer = client.topology.description.servers.get(selectedServerAddress);
295-
expect(selectedServer).property('type').to.equal(ServerType.RSSecondary);
273+
const cursor = client
274+
.db()
275+
.collection('countTEST')
276+
.find({ qty: { $gt: 4 } });
277+
await cursor.count({ readPreference: ReadPreference.SECONDARY });
296278

297-
done();
298-
});
299-
});
279+
const selectedServerAddress = bag[0].address.replace('127.0.0.1', 'localhost');
280+
const selectedServer = client.topology.description.servers.get(selectedServerAddress);
281+
expect(selectedServer).property('type').to.equal(ServerType.RSSecondary);
300282
}
301283
});
302284

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
import { expect } from 'chai';
2+
import * as net from 'net';
3+
import * as process from 'process';
4+
import * as sinon from 'sinon';
5+
6+
import { ConnectionCreatedEvent, ReadPreference, TopologyType } from '../../../src';
7+
import { byStrings, sorted } from '../../tools/utils';
8+
9+
describe('IPv6 Addresses', () => {
10+
let client;
11+
let ipv6Hosts;
12+
13+
beforeEach(async function () {
14+
if (
15+
process.platform !== 'win32' ||
16+
this.configuration.topologyType !== TopologyType.ReplicaSetWithPrimary
17+
) {
18+
if (this.currentTest) {
19+
// Ubuntu 18 does not support localhost AAAA lookups (IPv6)
20+
// Windows (VS2019) has the AAAA lookup
21+
// We do not run a replica set on macos
22+
this.currentTest.skipReason =
23+
'We are only running this on windows currently because it has the IPv6 translation for localhost';
24+
}
25+
return this.skip();
26+
}
27+
28+
ipv6Hosts = this.configuration.options.hostAddresses.map(({ port }) => `[::1]:${port}`);
29+
client = this.configuration.newClient(`mongodb://${ipv6Hosts.join(',')}/test`, {
30+
[Symbol.for('@@mdb.skipPingOnConnect')]: true,
31+
maxPoolSize: 1
32+
});
33+
});
34+
35+
afterEach(async function () {
36+
sinon.restore();
37+
await client?.close();
38+
});
39+
40+
it('should have three localhost IPv6 addresses set', function () {
41+
const ipv6LocalhostAddresses = this.configuration.options.hostAddresses.map(({ port }) => ({
42+
host: '::1',
43+
port,
44+
isIPv6: true
45+
}));
46+
expect(client.options.hosts).to.deep.equal(ipv6LocalhostAddresses);
47+
});
48+
49+
it('should successfully connect using IPv6', async function () {
50+
const localhostHosts = this.configuration.options.hostAddresses.map(
51+
({ port }) => `localhost:${port}`
52+
);
53+
await client.db().command({ ping: 1 });
54+
// After running the first command we should receive the hosts back as reported by the mongod in a hello response
55+
// mongod will report the bound host address, in this case "localhost"
56+
expect(sorted(client.topology.s.description.servers.keys(), byStrings)).to.deep.equal(
57+
localhostHosts
58+
);
59+
});
60+
61+
it('should createConnection with IPv6 addresses initially then switch to mongodb bound addresses', async () => {
62+
const createConnectionSpy = sinon.spy(net, 'createConnection');
63+
64+
const connectionCreatedEvents: ConnectionCreatedEvent[] = [];
65+
client.on('connectionCreated', ev => connectionCreatedEvents.push(ev));
66+
67+
await client.db().command({ ping: 1 }, { readPreference: ReadPreference.primary });
68+
69+
const callArgs = createConnectionSpy.getCalls().map(({ args }) => args[0]);
70+
71+
// This is 7 because we create 3 monitoring connections with ::1, then another 3 with localhost
72+
// and then 1 more in the connection pool for the operation, that is why we are checking for the connectionCreated event
73+
expect(callArgs).to.be.lengthOf(7);
74+
expect(connectionCreatedEvents).to.have.lengthOf(1);
75+
expect(connectionCreatedEvents[0]).to.have.property('address').that.includes('localhost');
76+
77+
for (let index = 0; index < 3; index++) {
78+
// The first 3 connections (monitoring) are made using the user provided addresses
79+
expect(callArgs[index]).to.have.property('host', '::1');
80+
}
81+
82+
for (let index = 3; index < 6; index++) {
83+
// MongoDB sends use back hellos that have the bound address 'localhost'
84+
// We make new connection using that address instead
85+
expect(callArgs[index]).to.have.property('host', 'localhost');
86+
}
87+
88+
// Operation connection
89+
expect(callArgs[6]).to.have.property('host', 'localhost');
90+
});
91+
});

test/tools/cluster_setup.sh

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,15 @@ SHARDED_DIR=${SHARDED_DIR:-$DATA_DIR/sharded_cluster}
1313

1414
if [[ $1 == "replica_set" ]]; then
1515
mkdir -p $REPLICASET_DIR # user / password
16-
mlaunch init --dir $REPLICASET_DIR --auth --username "bob" --password "pwd123" --replicaset --nodes 3 --arbiter --name rs --port 31000 --enableMajorityReadConcern --setParameter enableTestCommands=1
16+
mlaunch init --dir $REPLICASET_DIR --ipv6 --auth --username "bob" --password "pwd123" --replicaset --nodes 3 --arbiter --name rs --port 31000 --enableMajorityReadConcern --setParameter enableTestCommands=1
1717
echo "mongodb://bob:pwd123@localhost:31000,localhost:31001,localhost:31002/?replicaSet=rs"
1818
elif [[ $1 == "sharded_cluster" ]]; then
1919
mkdir -p $SHARDED_DIR
20-
mlaunch init --dir $SHARDED_DIR --auth --username "bob" --password "pwd123" --replicaset --nodes 3 --name rs --port 51000 --enableMajorityReadConcern --setParameter enableTestCommands=1 --sharded 1 --mongos 2
20+
mlaunch init --dir $SHARDED_DIR --ipv6 --auth --username "bob" --password "pwd123" --replicaset --nodes 3 --name rs --port 51000 --enableMajorityReadConcern --setParameter enableTestCommands=1 --sharded 1 --mongos 2
2121
echo "mongodb://bob:pwd123@localhost:51000,localhost:51001"
2222
elif [[ $1 == "server" ]]; then
2323
mkdir -p $SINGLE_DIR
24-
mlaunch init --dir $SINGLE_DIR --auth --username "bob" --password "pwd123" --single --setParameter enableTestCommands=1
24+
mlaunch init --dir $SINGLE_DIR --ipv6 --auth --username "bob" --password "pwd123" --single --setParameter enableTestCommands=1
2525
echo "mongodb://bob:pwd123@localhost:27017"
2626
else
2727
echo "unsupported topology: $1"

test/tools/cmap_spec_runner.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,10 @@ async function runCmapTest(test: CmapTest, threadContext: ThreadContext) {
391391
if (expectedError) {
392392
expect(actualError).to.exist;
393393
const { type: errorType, message: errorMessage, ...errorPropsToCheck } = expectedError;
394-
expect(actualError).to.have.property('name', `Mongo${errorType}`);
394+
expect(
395+
actualError,
396+
`${actualError.name} does not match "Mongo${errorType}", ${actualError.message} ${actualError.stack}`
397+
).to.have.property('name', `Mongo${errorType}`);
395398
if (errorMessage) {
396399
if (
397400
errorMessage === 'Timed out while checking out a connection from connection pool' &&

test/tools/runner/config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export class TestConfiguration {
5858
buildInfo: Record<string, any>;
5959
options: {
6060
hosts?: string[];
61-
hostAddresses?: HostAddress[];
61+
hostAddresses: HostAddress[];
6262
hostAddress?: HostAddress;
6363
host?: string;
6464
port?: number;

test/tools/uri_spec_runner.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { expect } from 'chai';
22

3-
import { MongoAPIError, MongoParseError } from '../../src';
3+
import { MongoAPIError, MongoParseError, MongoRuntimeError } from '../../src';
44
import { MongoClient } from '../../src/mongo_client';
55

66
type HostObject = {
@@ -71,6 +71,8 @@ export function executeUriValidationTest(
7171
} catch (err) {
7272
if (err instanceof TypeError) {
7373
expect(err).to.have.property('code').equal('ERR_INVALID_URL');
74+
} else if (err instanceof MongoRuntimeError) {
75+
expect(err).to.have.nested.property('cause.code').equal('ERR_INVALID_URL');
7476
} else if (
7577
// most of our validation is MongoParseError, which does not extend from MongoAPIError
7678
!(err instanceof MongoParseError) &&

test/unit/connection_string.test.ts

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ import {
1010
MongoAPIError,
1111
MongoDriverError,
1212
MongoInvalidArgumentError,
13-
MongoParseError
13+
MongoParseError,
14+
MongoRuntimeError
1415
} from '../../src/error';
1516
import { MongoClient, MongoOptions } from '../../src/mongo_client';
1617

@@ -573,4 +574,51 @@ describe('Connection String', function () {
573574
expect(client.s.options).to.have.property(flag, null);
574575
});
575576
});
577+
578+
describe('IPv6 host addresses', () => {
579+
it('should not allow multiple unbracketed portless localhost IPv6 addresses', () => {
580+
// Note there is no "port-full" version of this test, there's no way to distinguish when a port begins without brackets
581+
expect(() => new MongoClient('mongodb://::1,::1,::1/test')).to.throw(
582+
/invalid connection string/i
583+
);
584+
});
585+
586+
it('should not allow multiple unbracketed portless remote IPv6 addresses', () => {
587+
expect(
588+
() =>
589+
new MongoClient(
590+
'mongodb://ABCD:f::abcd:abcd:abcd:abcd,ABCD:f::abcd:abcd:abcd:abcd,ABCD:f::abcd:abcd:abcd:abcd/test'
591+
)
592+
).to.throw(MongoRuntimeError);
593+
});
594+
595+
it('should allow multiple bracketed portless localhost IPv6 addresses', () => {
596+
const client = new MongoClient('mongodb://[::1],[::1],[::1]/test');
597+
expect(client.options.hosts).to.deep.equal([
598+
{ host: '::1', port: 27017, isIPv6: true },
599+
{ host: '::1', port: 27017, isIPv6: true },
600+
{ host: '::1', port: 27017, isIPv6: true }
601+
]);
602+
});
603+
604+
it('should allow multiple bracketed portless localhost IPv6 addresses', () => {
605+
const client = new MongoClient(
606+
'mongodb://[ABCD:f::abcd:abcd:abcd:abcd],[ABCD:f::abcd:abcd:abcd:abcd],[ABCD:f::abcd:abcd:abcd:abcd]/test'
607+
);
608+
expect(client.options.hosts).to.deep.equal([
609+
{ host: 'abcd:f::abcd:abcd:abcd:abcd', port: 27017, isIPv6: true },
610+
{ host: 'abcd:f::abcd:abcd:abcd:abcd', port: 27017, isIPv6: true },
611+
{ host: 'abcd:f::abcd:abcd:abcd:abcd', port: 27017, isIPv6: true }
612+
]);
613+
});
614+
615+
it('should allow multiple bracketed port-full IPv6 addresses', () => {
616+
const client = new MongoClient('mongodb://[::1]:27018,[::1]:27019,[::1]:27020/test');
617+
expect(client.options.hosts).to.deep.equal([
618+
{ host: '::1', port: 27018, isIPv6: true },
619+
{ host: '::1', port: 27019, isIPv6: true },
620+
{ host: '::1', port: 27020, isIPv6: true }
621+
]);
622+
});
623+
});
576624
});

0 commit comments

Comments
 (0)