Skip to content

Commit 5eb3978

Browse files
authoredSep 16, 2022
fix(NODE-4621): ipv6 address handling in HostAddress (#3410)
1 parent 9883993 commit 5eb3978

12 files changed

+222
-84
lines changed
 

‎src/cmap/connection.ts

+3-2
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

+2-2
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

+36-32
Original file line numberDiff line numberDiff line change
@@ -1023,44 +1023,51 @@ export class BufferPool {
10231023

10241024
/** @public */
10251025
export class HostAddress {
1026-
host;
1027-
port;
1028-
// Driver only works with unix socket path to connect
1029-
// SDAM operates only on tcp addresses
1030-
socketPath;
1031-
isIPv6;
1026+
host: string | undefined = undefined;
1027+
port: number | undefined = undefined;
1028+
socketPath: string | undefined = undefined;
1029+
isIPv6 = false;
10321030

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

10371034
if (escapedHost.endsWith('.sock')) {
10381035
// heuristically determine if we're working with a domain socket
10391036
this.socketPath = decodeURIComponent(escapedHost);
1040-
} else if (typeof hostname === 'string') {
1041-
this.isIPv6 = false;
1037+
return;
1038+
}
10421039

1043-
let normalized = decodeURIComponent(hostname).toLowerCase();
1044-
if (normalized.startsWith('[') && normalized.endsWith(']')) {
1045-
this.isIPv6 = true;
1046-
normalized = normalized.substring(1, hostname.length - 1);
1047-
}
1040+
const urlString = `iLoveJS://${escapedHost}`;
1041+
let url;
1042+
try {
1043+
url = new URL(urlString);
1044+
} catch (urlError) {
1045+
const runtimeError = new MongoRuntimeError(`Unable to parse ${escapedHost} with URL`);
1046+
runtimeError.cause = urlError;
1047+
throw runtimeError;
1048+
}
10481049

1049-
this.host = normalized.toLowerCase();
1050+
const hostname = url.hostname;
1051+
const port = url.port;
10501052

1051-
if (typeof port === 'number') {
1052-
this.port = port;
1053-
} else if (typeof port === 'string' && port !== '') {
1054-
this.port = Number.parseInt(port, 10);
1055-
} else {
1056-
this.port = 27017;
1057-
}
1053+
let normalized = decodeURIComponent(hostname).toLowerCase();
1054+
if (normalized.startsWith('[') && normalized.endsWith(']')) {
1055+
this.isIPv6 = true;
1056+
normalized = normalized.substring(1, hostname.length - 1);
1057+
}
10581058

1059-
if (this.port === 0) {
1060-
throw new MongoParseError('Invalid port (zero) with hostname');
1061-
}
1059+
this.host = normalized.toLowerCase();
1060+
1061+
if (typeof port === 'number') {
1062+
this.port = port;
1063+
} else if (typeof port === 'string' && port !== '') {
1064+
this.port = Number.parseInt(port, 10);
10621065
} else {
1063-
throw new MongoInvalidArgumentError('Either socketPath or host must be defined.');
1066+
this.port = 27017;
1067+
}
1068+
1069+
if (this.port === 0) {
1070+
throw new MongoParseError('Invalid port (zero) with hostname');
10641071
}
10651072
Object.freeze(this);
10661073
}
@@ -1070,15 +1077,12 @@ export class HostAddress {
10701077
}
10711078

10721079
inspect(): string {
1073-
return `new HostAddress('${this.toString(true)}')`;
1080+
return `new HostAddress('${this.toString()}')`;
10741081
}
10751082

1076-
/**
1077-
* @param ipv6Brackets - optionally request ipv6 bracket notation required for connection strings
1078-
*/
1079-
toString(ipv6Brackets = false): string {
1083+
toString(): string {
10801084
if (typeof this.host === 'string') {
1081-
if (this.isIPv6 && ipv6Brackets) {
1085+
if (this.isIPv6) {
10821086
return `[${this.host}]:${this.port}`;
10831087
}
10841088
return `${this.host}:${this.port}`;

‎test/integration/crud/misc_cursors.test.js

+11-29
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

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
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, MongoClient, ReadPreference, TopologyType } from '../../../src';
7+
import { byStrings, sorted } from '../../tools/utils';
8+
9+
describe('IPv6 Addresses', () => {
10+
let client: MongoClient;
11+
let ipv6Hosts: string[];
12+
13+
beforeEach(async function () {
14+
if (
15+
process.platform === 'linux' ||
16+
this.configuration.topologyType !== TopologyType.ReplicaSetWithPrimary
17+
) {
18+
if (this.currentTest) {
19+
// Ubuntu 18 (linux) 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 IPv6 loopback addresses set on the client', function () {
41+
const ipv6LocalhostAddresses = this.configuration.options.hostAddresses.map(({ port }) => ({
42+
host: '::1',
43+
port,
44+
isIPv6: true,
45+
socketPath: undefined
46+
}));
47+
expect(client.options.hosts).to.deep.equal(ipv6LocalhostAddresses);
48+
});
49+
50+
it('should successfully connect using IPv6 loopback addresses', async function () {
51+
const localhostHosts = this.configuration.options.hostAddresses.map(
52+
({ port }) => `localhost:${port}` // ::1 will be swapped out for localhost
53+
);
54+
await client.db().command({ ping: 1 });
55+
// After running the first command we should receive the hosts back as reported by the mongod in a hello response
56+
// mongodb will report the bound host address, in this case "localhost"
57+
expect(client.topology).to.exist;
58+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
59+
expect(sorted(client.topology!.s.description.servers.keys(), byStrings)).to.deep.equal(
60+
localhostHosts
61+
);
62+
});
63+
64+
it('should createConnection with IPv6 addresses initially then switch to mongodb bound addresses', async () => {
65+
const createConnectionSpy = sinon.spy(net, 'createConnection');
66+
67+
const connectionCreatedEvents: ConnectionCreatedEvent[] = [];
68+
client.on('connectionCreated', ev => connectionCreatedEvents.push(ev));
69+
70+
await client.db().command({ ping: 1 }, { readPreference: ReadPreference.primary });
71+
72+
const callArgs = createConnectionSpy.getCalls().map(({ args }) => args[0]);
73+
74+
// This is 7 because we create 3 monitoring connections with ::1, then another 3 with localhost
75+
// and then 1 more in the connection pool for the operation, that is why we are checking for the connectionCreated event
76+
expect(callArgs).to.be.lengthOf(7);
77+
expect(connectionCreatedEvents).to.have.lengthOf(1);
78+
expect(connectionCreatedEvents[0]).to.have.property('address').that.includes('localhost');
79+
80+
for (let index = 0; index < 3; index++) {
81+
// The first 3 connections (monitoring) are made using the user provided addresses
82+
expect(callArgs[index]).to.have.property('host', '::1');
83+
}
84+
85+
for (let index = 3; index < 6; index++) {
86+
// MongoDB sends back hellos that have the bound address 'localhost'
87+
// We make new connection using that address instead
88+
expect(callArgs[index]).to.have.property('host', 'localhost');
89+
}
90+
91+
// Operation connection
92+
expect(callArgs[6]).to.have.property('host', 'localhost');
93+
});
94+
});

‎test/tools/cluster_setup.sh

+3-3
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

+4-1
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

+1-1
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

+3-3
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 = {
@@ -69,8 +69,8 @@ export function executeUriValidationTest(
6969
new MongoClient(test.uri);
7070
expect.fail(`Expected "${test.uri}" to be invalid${test.valid ? ' because of warning' : ''}`);
7171
} catch (err) {
72-
if (err instanceof TypeError) {
73-
expect(err).to.have.property('code').equal('ERR_INVALID_URL');
72+
if (err instanceof MongoRuntimeError) {
73+
expect(err).to.have.nested.property('cause.code').equal('ERR_INVALID_URL');
7474
} else if (
7575
// most of our validation is MongoParseError, which does not extend from MongoAPIError
7676
!(err instanceof MongoParseError) &&

‎test/unit/connection_string.test.ts

+49-1
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, socketPath: undefined },
599+
{ host: '::1', port: 27017, isIPv6: true, socketPath: undefined },
600+
{ host: '::1', port: 27017, isIPv6: true, socketPath: undefined }
601+
]);
602+
});
603+
604+
it('should allow multiple bracketed portless remote 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, socketPath: undefined },
610+
{ host: 'abcd:f::abcd:abcd:abcd:abcd', port: 27017, isIPv6: true, socketPath: undefined },
611+
{ host: 'abcd:f::abcd:abcd:abcd:abcd', port: 27017, isIPv6: true, socketPath: undefined }
612+
]);
613+
});
614+
615+
it('should allow multiple bracketed IPv6 addresses with specified ports', () => {
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, socketPath: undefined },
619+
{ host: '::1', port: 27019, isIPv6: true, socketPath: undefined },
620+
{ host: '::1', port: 27020, isIPv6: true, socketPath: undefined }
621+
]);
622+
});
623+
});
576624
});

‎test/unit/sdam/server_description.test.ts

+9-3
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,15 @@ describe('ServerDescription', function () {
6363
}
6464
});
6565

66-
it('should sensibly parse an ipv6 address', function () {
67-
const description = new ServerDescription('[ABCD:f::abcd:abcd:abcd:abcd]:27017');
68-
expect(description.host).to.equal('abcd:f::abcd:abcd:abcd:abcd');
66+
it('should normalize an IPv6 address with brackets and toLowered characters', function () {
67+
const description = new ServerDescription('[ABCD:f::abcd:abcd:abcd:abcd]:1234');
68+
expect(description.host).to.equal('[abcd:f::abcd:abcd:abcd:abcd]'); // IPv6 Addresses must always be bracketed if there is a port
69+
expect(description.port).to.equal(1234);
70+
});
71+
72+
it('should normalize an IPv6 address with brackets and toLowered characters even when the port is omitted', function () {
73+
const description = new ServerDescription('[ABCD:f::abcd:abcd:abcd:abcd]');
74+
expect(description.host).to.equal('[abcd:f::abcd:abcd:abcd:abcd]');
6975
expect(description.port).to.equal(27017);
7076
});
7177

‎test/unit/utils.test.ts

+7-7
Original file line numberDiff line numberDiff line change
@@ -306,27 +306,27 @@ describe('driver utils', function () {
306306
it('should handle decoded unix socket path', () => {
307307
const ha = new HostAddress(socketPath);
308308
expect(ha).to.have.property('socketPath', socketPath);
309-
expect(ha).to.not.have.property('port');
309+
expect(ha).to.have.property('port', undefined);
310310
});
311311

312312
it('should handle encoded unix socket path', () => {
313313
const ha = new HostAddress(encodeURIComponent(socketPath));
314314
expect(ha).to.have.property('socketPath', socketPath);
315-
expect(ha).to.not.have.property('port');
315+
expect(ha).to.have.property('port', undefined);
316316
});
317317

318318
it('should handle encoded unix socket path with an unencoded space', () => {
319319
const socketPathWithSpaces = '/tmp/some directory/mongodb-27017.sock';
320320
const ha = new HostAddress(socketPathWithSpaces);
321321
expect(ha).to.have.property('socketPath', socketPathWithSpaces);
322-
expect(ha).to.not.have.property('port');
322+
expect(ha).to.have.property('port', undefined);
323323
});
324324

325325
it('should handle unix socket path that does not begin with a slash', () => {
326326
const socketPathWithoutSlash = 'my_local/directory/mustEndWith.sock';
327327
const ha = new HostAddress(socketPathWithoutSlash);
328328
expect(ha).to.have.property('socketPath', socketPathWithoutSlash);
329-
expect(ha).to.not.have.property('port');
329+
expect(ha).to.have.property('port', undefined);
330330
});
331331

332332
it('should only set the socketPath property on HostAddress when hostString ends in .sock', () => {
@@ -335,16 +335,16 @@ describe('driver utils', function () {
335335
const hostnameThatEndsWithSock = 'iLoveJavascript.sock';
336336
const ha = new HostAddress(hostnameThatEndsWithSock);
337337
expect(ha).to.have.property('socketPath', hostnameThatEndsWithSock);
338-
expect(ha).to.not.have.property('port');
339-
expect(ha).to.not.have.property('host');
338+
expect(ha).to.have.property('port', undefined);
339+
expect(ha).to.have.property('host', undefined);
340340
});
341341

342342
it('should set the host and port property on HostAddress even when hostname ends in .sock if there is a port number specified', () => {
343343
// "should determine unix socket usage based on .sock ending" can be worked around by putting
344344
// the port number at the end of the hostname (even if it is the default)
345345
const hostnameThatEndsWithSockHasPort = 'iLoveJavascript.sock:27017';
346346
const ha = new HostAddress(hostnameThatEndsWithSockHasPort);
347-
expect(ha).to.not.have.property('socketPath');
347+
expect(ha).to.have.property('socketPath', undefined);
348348
expect(ha).to.have.property('host', 'iLoveJavascript.sock'.toLowerCase());
349349
expect(ha).to.have.property('port', 27017);
350350
});

0 commit comments

Comments
 (0)
Please sign in to comment.