From 5eb3978a71941b88d877f2121910f9612d15e9e5 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 16 Sep 2022 12:11:54 -0400 Subject: [PATCH] fix(NODE-4621): ipv6 address handling in HostAddress (#3410) --- src/cmap/connection.ts | 5 +- src/sdam/server_description.ts | 4 +- src/utils.ts | 68 ++++++++------- test/integration/crud/misc_cursors.test.js | 40 +++------ test/integration/node-specific/ipv6.test.ts | 94 +++++++++++++++++++++ test/tools/cluster_setup.sh | 6 +- test/tools/cmap_spec_runner.ts | 5 +- test/tools/runner/config.ts | 2 +- test/tools/uri_spec_runner.ts | 6 +- test/unit/connection_string.test.ts | 50 ++++++++++- test/unit/sdam/server_description.test.ts | 12 ++- test/unit/utils.test.ts | 14 +-- 12 files changed, 222 insertions(+), 84 deletions(-) create mode 100644 test/integration/node-specific/ipv6.test.ts diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 1647638045..416b0ee855 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -655,8 +655,9 @@ function streamIdentifier(stream: Stream, options: ConnectionOptions): string { return options.hostAddress.toString(); } - if (typeof stream.address === 'function') { - return `${stream.remoteAddress}:${stream.remotePort}`; + const { remoteAddress, remotePort } = stream; + if (typeof remoteAddress === 'string' && typeof remotePort === 'number') { + return HostAddress.fromHostPort(remoteAddress, remotePort).toString(); } return uuidV4().toString('hex'); diff --git a/src/sdam/server_description.ts b/src/sdam/server_description.ts index 541752cb43..ab5da450a1 100644 --- a/src/sdam/server_description.ts +++ b/src/sdam/server_description.ts @@ -88,8 +88,8 @@ export class ServerDescription { this.address = typeof address === 'string' - ? HostAddress.fromString(address).toString(false) // Use HostAddress to normalize - : address.toString(false); + ? HostAddress.fromString(address).toString() // Use HostAddress to normalize + : address.toString(); this.type = parseServerType(hello, options); this.hosts = hello?.hosts?.map((host: string) => host.toLowerCase()) ?? []; this.passives = hello?.passives?.map((host: string) => host.toLowerCase()) ?? []; diff --git a/src/utils.ts b/src/utils.ts index 82796ae17a..f871df54a7 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1023,44 +1023,51 @@ export class BufferPool { /** @public */ export class HostAddress { - host; - port; - // Driver only works with unix socket path to connect - // SDAM operates only on tcp addresses - socketPath; - isIPv6; + host: string | undefined = undefined; + port: number | undefined = undefined; + socketPath: string | undefined = undefined; + isIPv6 = false; constructor(hostString: string) { const escapedHost = hostString.split(' ').join('%20'); // escape spaces, for socket path hosts - const { hostname, port } = new URL(`mongodb://${escapedHost}`); if (escapedHost.endsWith('.sock')) { // heuristically determine if we're working with a domain socket this.socketPath = decodeURIComponent(escapedHost); - } else if (typeof hostname === 'string') { - this.isIPv6 = false; + return; + } - let normalized = decodeURIComponent(hostname).toLowerCase(); - if (normalized.startsWith('[') && normalized.endsWith(']')) { - this.isIPv6 = true; - normalized = normalized.substring(1, hostname.length - 1); - } + const urlString = `iLoveJS://${escapedHost}`; + let url; + try { + url = new URL(urlString); + } catch (urlError) { + const runtimeError = new MongoRuntimeError(`Unable to parse ${escapedHost} with URL`); + runtimeError.cause = urlError; + throw runtimeError; + } - this.host = normalized.toLowerCase(); + const hostname = url.hostname; + const port = url.port; - if (typeof port === 'number') { - this.port = port; - } else if (typeof port === 'string' && port !== '') { - this.port = Number.parseInt(port, 10); - } else { - this.port = 27017; - } + let normalized = decodeURIComponent(hostname).toLowerCase(); + if (normalized.startsWith('[') && normalized.endsWith(']')) { + this.isIPv6 = true; + normalized = normalized.substring(1, hostname.length - 1); + } - if (this.port === 0) { - throw new MongoParseError('Invalid port (zero) with hostname'); - } + this.host = normalized.toLowerCase(); + + if (typeof port === 'number') { + this.port = port; + } else if (typeof port === 'string' && port !== '') { + this.port = Number.parseInt(port, 10); } else { - throw new MongoInvalidArgumentError('Either socketPath or host must be defined.'); + this.port = 27017; + } + + if (this.port === 0) { + throw new MongoParseError('Invalid port (zero) with hostname'); } Object.freeze(this); } @@ -1070,15 +1077,12 @@ export class HostAddress { } inspect(): string { - return `new HostAddress('${this.toString(true)}')`; + return `new HostAddress('${this.toString()}')`; } - /** - * @param ipv6Brackets - optionally request ipv6 bracket notation required for connection strings - */ - toString(ipv6Brackets = false): string { + toString(): string { if (typeof this.host === 'string') { - if (this.isIPv6 && ipv6Brackets) { + if (this.isIPv6) { return `[${this.host}]:${this.port}`; } return `${this.host}:${this.port}`; diff --git a/test/integration/crud/misc_cursors.test.js b/test/integration/crud/misc_cursors.test.js index 60abe2c757..e8cea657f5 100644 --- a/test/integration/crud/misc_cursors.test.js +++ b/test/integration/crud/misc_cursors.test.js @@ -264,39 +264,21 @@ describe('Cursor', function () { } }); - it('Should correctly execute cursor count with secondary readPreference', { - // Add a tag that our runner can trigger on - // in this case we are setting that node needs to be higher than 0.10.X to run - metadata: { - requires: { topology: 'replicaset' } - }, - - test: function (done) { - const configuration = this.configuration; - const client = configuration.newClient(configuration.writeConcernMax(), { - maxPoolSize: 1, - monitorCommands: true - }); - + it('should correctly execute cursor count with secondary readPreference', { + metadata: { requires: { topology: 'replicaset' } }, + async test() { const bag = []; client.on('commandStarted', filterForCommands(['count'], bag)); - client.connect((err, client) => { - expect(err).to.not.exist; - this.defer(() => client.close()); - - const db = client.db(configuration.db); - const cursor = db.collection('countTEST').find({ qty: { $gt: 4 } }); - cursor.count({ readPreference: ReadPreference.SECONDARY }, err => { - expect(err).to.not.exist; - - const selectedServerAddress = bag[0].address.replace('127.0.0.1', 'localhost'); - const selectedServer = client.topology.description.servers.get(selectedServerAddress); - expect(selectedServer).property('type').to.equal(ServerType.RSSecondary); + const cursor = client + .db() + .collection('countTEST') + .find({ qty: { $gt: 4 } }); + await cursor.count({ readPreference: ReadPreference.SECONDARY }); - done(); - }); - }); + const selectedServerAddress = bag[0].address.replace('127.0.0.1', 'localhost'); + const selectedServer = client.topology.description.servers.get(selectedServerAddress); + expect(selectedServer).property('type').to.equal(ServerType.RSSecondary); } }); diff --git a/test/integration/node-specific/ipv6.test.ts b/test/integration/node-specific/ipv6.test.ts new file mode 100644 index 0000000000..6b19d59c86 --- /dev/null +++ b/test/integration/node-specific/ipv6.test.ts @@ -0,0 +1,94 @@ +import { expect } from 'chai'; +import * as net from 'net'; +import * as process from 'process'; +import * as sinon from 'sinon'; + +import { ConnectionCreatedEvent, MongoClient, ReadPreference, TopologyType } from '../../../src'; +import { byStrings, sorted } from '../../tools/utils'; + +describe('IPv6 Addresses', () => { + let client: MongoClient; + let ipv6Hosts: string[]; + + beforeEach(async function () { + if ( + process.platform === 'linux' || + this.configuration.topologyType !== TopologyType.ReplicaSetWithPrimary + ) { + if (this.currentTest) { + // Ubuntu 18 (linux) does not support localhost AAAA lookups (IPv6) + // Windows (VS2019) has the AAAA lookup + // We do not run a replica set on macos + this.currentTest.skipReason = + 'We are only running this on windows currently because it has the IPv6 translation for localhost'; + } + return this.skip(); + } + + ipv6Hosts = this.configuration.options.hostAddresses.map(({ port }) => `[::1]:${port}`); + client = this.configuration.newClient(`mongodb://${ipv6Hosts.join(',')}/test`, { + [Symbol.for('@@mdb.skipPingOnConnect')]: true, + maxPoolSize: 1 + }); + }); + + afterEach(async function () { + sinon.restore(); + await client?.close(); + }); + + it('should have IPv6 loopback addresses set on the client', function () { + const ipv6LocalhostAddresses = this.configuration.options.hostAddresses.map(({ port }) => ({ + host: '::1', + port, + isIPv6: true, + socketPath: undefined + })); + expect(client.options.hosts).to.deep.equal(ipv6LocalhostAddresses); + }); + + it('should successfully connect using IPv6 loopback addresses', async function () { + const localhostHosts = this.configuration.options.hostAddresses.map( + ({ port }) => `localhost:${port}` // ::1 will be swapped out for localhost + ); + await client.db().command({ ping: 1 }); + // After running the first command we should receive the hosts back as reported by the mongod in a hello response + // mongodb will report the bound host address, in this case "localhost" + expect(client.topology).to.exist; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + expect(sorted(client.topology!.s.description.servers.keys(), byStrings)).to.deep.equal( + localhostHosts + ); + }); + + it('should createConnection with IPv6 addresses initially then switch to mongodb bound addresses', async () => { + const createConnectionSpy = sinon.spy(net, 'createConnection'); + + const connectionCreatedEvents: ConnectionCreatedEvent[] = []; + client.on('connectionCreated', ev => connectionCreatedEvents.push(ev)); + + await client.db().command({ ping: 1 }, { readPreference: ReadPreference.primary }); + + const callArgs = createConnectionSpy.getCalls().map(({ args }) => args[0]); + + // This is 7 because we create 3 monitoring connections with ::1, then another 3 with localhost + // and then 1 more in the connection pool for the operation, that is why we are checking for the connectionCreated event + expect(callArgs).to.be.lengthOf(7); + expect(connectionCreatedEvents).to.have.lengthOf(1); + expect(connectionCreatedEvents[0]).to.have.property('address').that.includes('localhost'); + + for (let index = 0; index < 3; index++) { + // The first 3 connections (monitoring) are made using the user provided addresses + expect(callArgs[index]).to.have.property('host', '::1'); + } + + for (let index = 3; index < 6; index++) { + // MongoDB sends back hellos that have the bound address 'localhost' + // We make new connection using that address instead + expect(callArgs[index]).to.have.property('host', 'localhost'); + } + + // Operation connection + expect(callArgs[6]).to.have.property('host', 'localhost'); + }); +}); diff --git a/test/tools/cluster_setup.sh b/test/tools/cluster_setup.sh index 3e665e3867..6507321645 100755 --- a/test/tools/cluster_setup.sh +++ b/test/tools/cluster_setup.sh @@ -13,15 +13,15 @@ SHARDED_DIR=${SHARDED_DIR:-$DATA_DIR/sharded_cluster} if [[ $1 == "replica_set" ]]; then mkdir -p $REPLICASET_DIR # user / password - mlaunch init --dir $REPLICASET_DIR --auth --username "bob" --password "pwd123" --replicaset --nodes 3 --arbiter --name rs --port 31000 --enableMajorityReadConcern --setParameter enableTestCommands=1 + mlaunch init --dir $REPLICASET_DIR --ipv6 --auth --username "bob" --password "pwd123" --replicaset --nodes 3 --arbiter --name rs --port 31000 --enableMajorityReadConcern --setParameter enableTestCommands=1 echo "mongodb://bob:pwd123@localhost:31000,localhost:31001,localhost:31002/?replicaSet=rs" elif [[ $1 == "sharded_cluster" ]]; then mkdir -p $SHARDED_DIR - 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 + 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 echo "mongodb://bob:pwd123@localhost:51000,localhost:51001" elif [[ $1 == "server" ]]; then mkdir -p $SINGLE_DIR - mlaunch init --dir $SINGLE_DIR --auth --username "bob" --password "pwd123" --single --setParameter enableTestCommands=1 + mlaunch init --dir $SINGLE_DIR --ipv6 --auth --username "bob" --password "pwd123" --single --setParameter enableTestCommands=1 echo "mongodb://bob:pwd123@localhost:27017" else echo "unsupported topology: $1" diff --git a/test/tools/cmap_spec_runner.ts b/test/tools/cmap_spec_runner.ts index 54fc94cfd6..81c081e857 100644 --- a/test/tools/cmap_spec_runner.ts +++ b/test/tools/cmap_spec_runner.ts @@ -391,7 +391,10 @@ async function runCmapTest(test: CmapTest, threadContext: ThreadContext) { if (expectedError) { expect(actualError).to.exist; const { type: errorType, message: errorMessage, ...errorPropsToCheck } = expectedError; - expect(actualError).to.have.property('name', `Mongo${errorType}`); + expect( + actualError, + `${actualError.name} does not match "Mongo${errorType}", ${actualError.message} ${actualError.stack}` + ).to.have.property('name', `Mongo${errorType}`); if (errorMessage) { if ( errorMessage === 'Timed out while checking out a connection from connection pool' && diff --git a/test/tools/runner/config.ts b/test/tools/runner/config.ts index f5021963d1..525259ff4b 100644 --- a/test/tools/runner/config.ts +++ b/test/tools/runner/config.ts @@ -58,7 +58,7 @@ export class TestConfiguration { buildInfo: Record; options: { hosts?: string[]; - hostAddresses?: HostAddress[]; + hostAddresses: HostAddress[]; hostAddress?: HostAddress; host?: string; port?: number; diff --git a/test/tools/uri_spec_runner.ts b/test/tools/uri_spec_runner.ts index f81ef86d3e..e8a19b5646 100644 --- a/test/tools/uri_spec_runner.ts +++ b/test/tools/uri_spec_runner.ts @@ -1,6 +1,6 @@ import { expect } from 'chai'; -import { MongoAPIError, MongoParseError } from '../../src'; +import { MongoAPIError, MongoParseError, MongoRuntimeError } from '../../src'; import { MongoClient } from '../../src/mongo_client'; type HostObject = { @@ -69,8 +69,8 @@ export function executeUriValidationTest( new MongoClient(test.uri); expect.fail(`Expected "${test.uri}" to be invalid${test.valid ? ' because of warning' : ''}`); } catch (err) { - if (err instanceof TypeError) { - expect(err).to.have.property('code').equal('ERR_INVALID_URL'); + if (err instanceof MongoRuntimeError) { + expect(err).to.have.nested.property('cause.code').equal('ERR_INVALID_URL'); } else if ( // most of our validation is MongoParseError, which does not extend from MongoAPIError !(err instanceof MongoParseError) && diff --git a/test/unit/connection_string.test.ts b/test/unit/connection_string.test.ts index 21b603e1b2..08441bb65e 100644 --- a/test/unit/connection_string.test.ts +++ b/test/unit/connection_string.test.ts @@ -10,7 +10,8 @@ import { MongoAPIError, MongoDriverError, MongoInvalidArgumentError, - MongoParseError + MongoParseError, + MongoRuntimeError } from '../../src/error'; import { MongoClient, MongoOptions } from '../../src/mongo_client'; @@ -573,4 +574,51 @@ describe('Connection String', function () { expect(client.s.options).to.have.property(flag, null); }); }); + + describe('IPv6 host addresses', () => { + it('should not allow multiple unbracketed portless localhost IPv6 addresses', () => { + // Note there is no "port-full" version of this test, there's no way to distinguish when a port begins without brackets + expect(() => new MongoClient('mongodb://::1,::1,::1/test')).to.throw( + /invalid connection string/i + ); + }); + + it('should not allow multiple unbracketed portless remote IPv6 addresses', () => { + expect( + () => + new MongoClient( + 'mongodb://ABCD:f::abcd:abcd:abcd:abcd,ABCD:f::abcd:abcd:abcd:abcd,ABCD:f::abcd:abcd:abcd:abcd/test' + ) + ).to.throw(MongoRuntimeError); + }); + + it('should allow multiple bracketed portless localhost IPv6 addresses', () => { + const client = new MongoClient('mongodb://[::1],[::1],[::1]/test'); + expect(client.options.hosts).to.deep.equal([ + { host: '::1', port: 27017, isIPv6: true, socketPath: undefined }, + { host: '::1', port: 27017, isIPv6: true, socketPath: undefined }, + { host: '::1', port: 27017, isIPv6: true, socketPath: undefined } + ]); + }); + + it('should allow multiple bracketed portless remote IPv6 addresses', () => { + const client = new MongoClient( + 'mongodb://[ABCD:f::abcd:abcd:abcd:abcd],[ABCD:f::abcd:abcd:abcd:abcd],[ABCD:f::abcd:abcd:abcd:abcd]/test' + ); + expect(client.options.hosts).to.deep.equal([ + { host: 'abcd:f::abcd:abcd:abcd:abcd', port: 27017, isIPv6: true, socketPath: undefined }, + { host: 'abcd:f::abcd:abcd:abcd:abcd', port: 27017, isIPv6: true, socketPath: undefined }, + { host: 'abcd:f::abcd:abcd:abcd:abcd', port: 27017, isIPv6: true, socketPath: undefined } + ]); + }); + + it('should allow multiple bracketed IPv6 addresses with specified ports', () => { + const client = new MongoClient('mongodb://[::1]:27018,[::1]:27019,[::1]:27020/test'); + expect(client.options.hosts).to.deep.equal([ + { host: '::1', port: 27018, isIPv6: true, socketPath: undefined }, + { host: '::1', port: 27019, isIPv6: true, socketPath: undefined }, + { host: '::1', port: 27020, isIPv6: true, socketPath: undefined } + ]); + }); + }); }); diff --git a/test/unit/sdam/server_description.test.ts b/test/unit/sdam/server_description.test.ts index 83331a1f4c..29de0db89b 100644 --- a/test/unit/sdam/server_description.test.ts +++ b/test/unit/sdam/server_description.test.ts @@ -63,9 +63,15 @@ describe('ServerDescription', function () { } }); - it('should sensibly parse an ipv6 address', function () { - const description = new ServerDescription('[ABCD:f::abcd:abcd:abcd:abcd]:27017'); - expect(description.host).to.equal('abcd:f::abcd:abcd:abcd:abcd'); + it('should normalize an IPv6 address with brackets and toLowered characters', function () { + const description = new ServerDescription('[ABCD:f::abcd:abcd:abcd:abcd]:1234'); + expect(description.host).to.equal('[abcd:f::abcd:abcd:abcd:abcd]'); // IPv6 Addresses must always be bracketed if there is a port + expect(description.port).to.equal(1234); + }); + + it('should normalize an IPv6 address with brackets and toLowered characters even when the port is omitted', function () { + const description = new ServerDescription('[ABCD:f::abcd:abcd:abcd:abcd]'); + expect(description.host).to.equal('[abcd:f::abcd:abcd:abcd:abcd]'); expect(description.port).to.equal(27017); }); diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts index 9b4f7a0118..db4e589bc6 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.ts @@ -306,27 +306,27 @@ describe('driver utils', function () { it('should handle decoded unix socket path', () => { const ha = new HostAddress(socketPath); expect(ha).to.have.property('socketPath', socketPath); - expect(ha).to.not.have.property('port'); + expect(ha).to.have.property('port', undefined); }); it('should handle encoded unix socket path', () => { const ha = new HostAddress(encodeURIComponent(socketPath)); expect(ha).to.have.property('socketPath', socketPath); - expect(ha).to.not.have.property('port'); + expect(ha).to.have.property('port', undefined); }); it('should handle encoded unix socket path with an unencoded space', () => { const socketPathWithSpaces = '/tmp/some directory/mongodb-27017.sock'; const ha = new HostAddress(socketPathWithSpaces); expect(ha).to.have.property('socketPath', socketPathWithSpaces); - expect(ha).to.not.have.property('port'); + expect(ha).to.have.property('port', undefined); }); it('should handle unix socket path that does not begin with a slash', () => { const socketPathWithoutSlash = 'my_local/directory/mustEndWith.sock'; const ha = new HostAddress(socketPathWithoutSlash); expect(ha).to.have.property('socketPath', socketPathWithoutSlash); - expect(ha).to.not.have.property('port'); + expect(ha).to.have.property('port', undefined); }); it('should only set the socketPath property on HostAddress when hostString ends in .sock', () => { @@ -335,8 +335,8 @@ describe('driver utils', function () { const hostnameThatEndsWithSock = 'iLoveJavascript.sock'; const ha = new HostAddress(hostnameThatEndsWithSock); expect(ha).to.have.property('socketPath', hostnameThatEndsWithSock); - expect(ha).to.not.have.property('port'); - expect(ha).to.not.have.property('host'); + expect(ha).to.have.property('port', undefined); + expect(ha).to.have.property('host', undefined); }); it('should set the host and port property on HostAddress even when hostname ends in .sock if there is a port number specified', () => { @@ -344,7 +344,7 @@ describe('driver utils', function () { // the port number at the end of the hostname (even if it is the default) const hostnameThatEndsWithSockHasPort = 'iLoveJavascript.sock:27017'; const ha = new HostAddress(hostnameThatEndsWithSockHasPort); - expect(ha).to.not.have.property('socketPath'); + expect(ha).to.have.property('socketPath', undefined); expect(ha).to.have.property('host', 'iLoveJavascript.sock'.toLowerCase()); expect(ha).to.have.property('port', 27017); });