From a0107ae45630d87f7562c49486472179ae127186 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 19 Feb 2021 11:34:55 +0100 Subject: [PATCH 1/2] fix(service-provider-core): validate --host option more strictly This came up in a recent conversation in slack, where passing multiple hosts through the `--host` option separated by commas did not fail properly and instead led to connecting to the wrong set of hosts. --- packages/i18n/src/locales/en_US.ts | 3 ++- .../src/uri-generator.spec.ts | 16 +++++++++++++ .../src/uri-generator.ts | 23 ++++++++++++++++++- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/packages/i18n/src/locales/en_US.ts b/packages/i18n/src/locales/en_US.ts index a1b85647c1..090fbe28ae 100644 --- a/packages/i18n/src/locales/en_US.ts +++ b/packages/i18n/src/locales/en_US.ts @@ -70,7 +70,8 @@ const translations: Catalog = { } }, 'uri-generator': { - 'no-host-port': 'If a full URI is provided, you cannot also specify --host or --port' + 'no-host-port': 'If a full URI is provided, you cannot also specify --host or --port', + 'invalid-host': 'The --host argument contains an invalid character' } }, 'service-provider-browser': {}, diff --git a/packages/service-provider-core/src/uri-generator.spec.ts b/packages/service-provider-core/src/uri-generator.spec.ts index 22bccb587e..9d4789bf4f 100644 --- a/packages/service-provider-core/src/uri-generator.spec.ts +++ b/packages/service-provider-core/src/uri-generator.spec.ts @@ -292,4 +292,20 @@ describe('uri-generator.generate-uri', () => { expect.fail('expected error'); }); }); + + context('when the --host option contains invalid characters', () => { + 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).to.be.instanceOf(MongoshInvalidInputError); + expect(e.code).to.equal(CommonErrors.InvalidArgument); + return; + } + expect.fail('expected error'); + }); + }); }); diff --git a/packages/service-provider-core/src/uri-generator.ts b/packages/service-provider-core/src/uri-generator.ts index 395a3f7ef9..8deaabf66f 100644 --- a/packages/service-provider-core/src/uri-generator.ts +++ b/packages/service-provider-core/src/uri-generator.ts @@ -41,17 +41,36 @@ const DEFAULT_PORT = '27017'; */ const CONFLICT = 'cli-repl.uri-generator.no-host-port'; +/** + * Invalid host message. + */ +const INVALID_HOST = 'cli-repl.uri-generator.invalid-host'; + /** * Validate conflicts in the options. * * @param {CliOptions} options - The options. */ -function validateConflicts(options: CliOptions): any { +function validateConflicts(options: CliOptions): void { if (options.host || options.port) { throw new MongoshInvalidInputError(i18n.__(CONFLICT), CommonErrors.InvalidArgument); } } +/** + * Perform basic validation of the --host option. + * + * @param {string} host - The value of the --host option. + */ +function validateHost(host: string): void { + const invalidCharacter = host.match(/[^a-zA-Z0-9\.-:\[\]]/); + if (invalidCharacter) { + throw new MongoshInvalidInputError( + i18n.__(INVALID_HOST) + ': ' + invalidCharacter[0], + CommonErrors.InvalidArgument); + } +} + /** * Generate the host from the options or default. * @@ -61,6 +80,7 @@ function validateConflicts(options: CliOptions): any { */ function generateHost(options: CliOptions): string { if (options.host) { + validateHost(options.host); if (options.host.includes(':')) { return options.host.split(':')[0]; } @@ -78,6 +98,7 @@ function generateHost(options: CliOptions): string { */ function generatePort(options: CliOptions): string { if (options.host && options.host.includes(':')) { + validateHost(options.host); const port = options.host.split(':')[1]; if (!options.port || options.port === port) { return port; From 6c6bf4e2fc647ac38600e2fc598ba8b18dfe0ab5 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 19 Feb 2021 12:18:55 +0100 Subject: [PATCH 2/2] fixup Co-authored-by: Michael Rose --- packages/service-provider-core/src/uri-generator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/service-provider-core/src/uri-generator.ts b/packages/service-provider-core/src/uri-generator.ts index 8deaabf66f..f80d98101e 100644 --- a/packages/service-provider-core/src/uri-generator.ts +++ b/packages/service-provider-core/src/uri-generator.ts @@ -63,7 +63,7 @@ function validateConflicts(options: CliOptions): void { * @param {string} host - The value of the --host option. */ function validateHost(host: string): void { - const invalidCharacter = host.match(/[^a-zA-Z0-9\.-:\[\]]/); + const invalidCharacter = host.match(/[^a-zA-Z0-9.:\[\]-]/); if (invalidCharacter) { throw new MongoshInvalidInputError( i18n.__(INVALID_HOST) + ': ' + invalidCharacter[0],