From 0d696e08db8a3edfe3198b589ec9a7de078710c2 Mon Sep 17 00:00:00 2001 From: Michael Rose Date: Thu, 15 Jul 2021 12:00:12 +0200 Subject: [PATCH] fix(cli-repl): properly handle host seedlist MONGOSH-901 --- packages/cli-repl/src/cli-repl.spec.ts | 4 +- packages/cli-repl/test/e2e-direct.spec.ts | 27 +++++++++++- .../src/uri-generator.spec.ts | 41 +++++++++++++------ .../src/uri-generator.ts | 17 +++++++- 4 files changed, 70 insertions(+), 19 deletions(-) diff --git a/packages/cli-repl/src/cli-repl.spec.ts b/packages/cli-repl/src/cli-repl.spec.ts index 8a2883a945..802c93f77c 100644 --- a/packages/cli-repl/src/cli-repl.spec.ts +++ b/packages/cli-repl/src/cli-repl.spec.ts @@ -1,16 +1,16 @@ import { MongoshInternalError } from '@mongosh/errors'; +import { bson } from '@mongosh/service-provider-core'; import { once } from 'events'; import { promises as fs } from 'fs'; import http from 'http'; import path from 'path'; import { Duplex, PassThrough } from 'stream'; import { promisify } from 'util'; +import { eventually } from '../../../testing/eventually'; import { MongodSetup, skipIfServerVersion, startTestServer } from '../../../testing/integration-testing-hooks'; import { expect, fakeTTYProps, readReplLogfile, tick, useTmpdir, waitBus, waitCompletion, waitEval } from '../test/repl-helpers'; -import { eventually } from '../../../testing/eventually'; import CliRepl, { CliReplOptions } from './cli-repl'; import { CliReplErrors } from './error-codes'; -import { bson } from '@mongosh/service-provider-core'; const { EJSON } = bson; const delay = promisify(setTimeout); diff --git a/packages/cli-repl/test/e2e-direct.spec.ts b/packages/cli-repl/test/e2e-direct.spec.ts index 79b9419896..b6302d72ec 100644 --- a/packages/cli-repl/test/e2e-direct.spec.ts +++ b/packages/cli-repl/test/e2e-direct.spec.ts @@ -60,6 +60,11 @@ describe('e2e direct connection', () => { shell.assertContainsOutput(`me: '${await rs0.hostport()}'`); shell.assertContainsOutput(`setName: '${replSetId}'`); }); + + await shell.executeLine('use admin'); + await shell.executeLine("db.createUser({ user: 'anna', pwd: 'pwd', roles: [] })"); + shell.assertContainsOutput('ok: 1'); + dbname = `test-${Date.now()}-${(Math.random() * 100000) | 0}`; await shell.executeLine(`use ${dbname}`); await shell.executeLine('db.testcollection.insertOne({})'); @@ -173,7 +178,7 @@ describe('e2e direct connection', () => { shell.assertContainsOutput(`me: '${await rs0.hostport()}'`); shell.assertContainsOutput(`setName: '${replSetId}'`); }); - it('when specifying multiple seeds', async() => { + it('when specifying multiple seeds in a connection string', async() => { const connectionString = 'mongodb://' + await rs2.hostport() + ',' + await rs1.hostport() + ',' + await rs0.hostport(); const shell = TestShell.start({ args: [connectionString] }); await shell.waitForPrompt(); @@ -182,7 +187,18 @@ describe('e2e direct connection', () => { shell.assertContainsOutput(`me: '${await rs0.hostport()}'`); shell.assertContainsOutput(`setName: '${replSetId}'`); }); - it('when specifying multiple seeds through --host', async() => { + it('when specifying multiple seeds without replset through --host', async() => { + const hostlist = await rs2.hostport() + ',' + await rs1.hostport() + ',' + await rs0.hostport(); + const shell = TestShell.start({ args: ['--host', hostlist] }); + await shell.waitForPrompt(); + await shell.executeLine('db.isMaster()'); + await shell.executeLine('({ dbname: db.getName() })'); + shell.assertContainsOutput('ismaster: true'); + shell.assertContainsOutput(`me: '${await rs0.hostport()}'`); + shell.assertContainsOutput(`setName: '${replSetId}'`); + shell.assertContainsOutput("dbname: 'test'"); + }); + it('when specifying multiple seeds with replset through --host', async() => { const hostlist = replSetId + '/' + await rs2.hostport() + ',' + await rs1.hostport() + ',' + await rs0.hostport(); const shell = TestShell.start({ args: ['--host', hostlist] }); await shell.waitForPrompt(); @@ -230,6 +246,13 @@ describe('e2e direct connection', () => { shell.assertContainsOutput('db.testcollection'); }); }); + it('can authenticate when specifying multiple seeds with replset through --host', async() => { + const hostlist = replSetId + '/' + await rs2.hostport() + ',' + await rs1.hostport() + ',' + await rs0.hostport(); + const shell = TestShell.start({ args: ['--host', hostlist, '--username', 'anna', '--password', 'pwd'] }); + await shell.waitForPrompt(); + await shell.executeLine('db.runCommand({connectionStatus: 1})'); + shell.assertContainsOutput("user: 'anna'"); + }); }); describe('fail-fast connections', () => { diff --git a/packages/service-provider-core/src/uri-generator.spec.ts b/packages/service-provider-core/src/uri-generator.spec.ts index 8d8e57e697..70866efa48 100644 --- a/packages/service-provider-core/src/uri-generator.spec.ts +++ b/packages/service-provider-core/src/uri-generator.spec.ts @@ -399,13 +399,13 @@ describe('uri-generator.generate-uri', () => { }); context('when the --host option contains invalid characters', () => { - const options = { host: 'a,b,c' }; + const options = { host: 'a$b,c' }; it('returns the uri', () => { try { generateUri(options); } catch (e) { - expect(e.message).to.contain('The --host argument contains an invalid character: ,'); + expect(e.message).to.contain('The --host argument contains an invalid character: $'); expect(e).to.be.instanceOf(MongoshInvalidInputError); expect(e.code).to.equal(CommonErrors.InvalidArgument); return; @@ -414,20 +414,35 @@ describe('uri-generator.generate-uri', () => { }); }); - context('when the --host option contains a replica set', () => { - it('returns a URI for the hosts and ports specified in --host', () => { - const options = { host: 'replsetname/host1:123,host2,host3:456,' }; - expect(generateUri(options)).to.equal('mongodb://host1:123,host2,host3:456/test?replicaSet=replsetname'); + context('when the --host option contains a seed list', () => { + context('without a replica set', () => { + it('returns a URI for the hosts and ports specified in --host', () => { + const options = { host: 'host1:123,host2,host3:456,' }; + expect(generateUri(options)).to.equal('mongodb://host1:123,host2,host3:456/'); + }); + it('returns a URI for the hosts and ports specified in --host and database name', () => { + const options = { + host: 'host1:123,host2,host3:456,', + connectionSpecifier: 'admin' + }; + expect(generateUri(options)).to.equal('mongodb://host1:123,host2,host3:456/admin'); + }); }); + context('with a replica set', () => { + it('returns a URI for the hosts and ports specified in --host', () => { + const options = { host: 'replsetname/host1:123,host2,host3:456,' }; + expect(generateUri(options)).to.equal('mongodb://host1:123,host2,host3:456/?replicaSet=replsetname'); + }); - it('returns a URI for the hosts and ports specified in --host and database name', () => { - const options = { host: 'replsetname/host1:123,host2,host3:456', connectionSpecifier: 'admin' }; - expect(generateUri(options)).to.equal('mongodb://host1:123,host2,host3:456/admin?replicaSet=replsetname'); - }); + it('returns a URI for the hosts and ports specified in --host and database name', () => { + const options = { host: 'replsetname/host1:123,host2,host3:456', connectionSpecifier: 'admin' }; + expect(generateUri(options)).to.equal('mongodb://host1:123,host2,host3:456/admin?replicaSet=replsetname'); + }); - it('returns a URI for the hosts and ports specified in --host and database name with escaped chars', () => { - const options = { host: 'replsetname/host1:123,host2,host3:456', connectionSpecifier: 'admin?foo=bar' }; - expect(generateUri(options)).to.equal('mongodb://host1:123,host2,host3:456/admin%3Ffoo%3Dbar?replicaSet=replsetname'); + it('returns a URI for the hosts and ports specified in --host and database name with escaped chars', () => { + const options = { host: 'replsetname/host1:123,host2,host3:456', connectionSpecifier: 'admin?foo=bar' }; + expect(generateUri(options)).to.equal('mongodb://host1:123,host2,host3:456/admin%3Ffoo%3Dbar?replicaSet=replsetname'); + }); }); }); }); diff --git a/packages/service-provider-core/src/uri-generator.ts b/packages/service-provider-core/src/uri-generator.ts index 103e3282ce..664b13120f 100644 --- a/packages/service-provider-core/src/uri-generator.ts +++ b/packages/service-provider-core/src/uri-generator.ts @@ -149,15 +149,28 @@ function generateUriNormalized(options: CliOptions): ConnectionString { // If the --host argument contains /, it has the format // /<:port>,<:port>,<...> const replSetHostMatch = (options.host ?? '').match( - /^(?[^/]+)\/(?([A-Za-z0-9.-]+(:\d+)?,?)+)$/); + /^(?[^/]+)\/(?([A-Za-z0-9.-]+(:\d+)?,?)+)$/ + ); if (replSetHostMatch) { const { replSetName, hosts } = replSetHostMatch.groups as { replSetName: string, hosts: string }; - const connectionString = new ConnectionString(`${Scheme.Mongo}replacemeHost/${encodeURIComponent(uri ?? DEFAULT_DB)}`); + const connectionString = new ConnectionString(`${Scheme.Mongo}replacemeHost/${encodeURIComponent(uri || '')}`); connectionString.hosts = hosts.split(',').filter(host => host.trim()); connectionString.searchParams.set('replicaSet', replSetName); return addShellConnectionStringParameters(connectionString); } + // If the --host argument contains multiple hosts as a seed list + // we directly do not do additional host/port parsing + const seedList = (options.host ?? '').match( + /^(?([A-Za-z0-9.-]+(:\d+)?,?)+)$/ + ); + if (seedList && options.host?.includes(',')) { + const { hosts } = seedList.groups as { hosts: string }; + const connectionString = new ConnectionString(`${Scheme.Mongo}replacemeHost/${encodeURIComponent(uri || '')}`); + connectionString.hosts = hosts.split(',').filter(host => host.trim()); + return addShellConnectionStringParameters(connectionString); + } + // There is no URI provided, use default 127.0.0.1:27017 if (!uri) { return new ConnectionString(`${Scheme.Mongo}${generateHost(options)}:${generatePort(options)}/?directConnection=true`);