fix(service-provider-core): allow non-hostname chars in DB name MONGOSH-792 (#906)

This also means that e.g. `host:port?option=value` is not allowed,
and instead any optional part after the port in the non-URL variant
needs to be separated by a slash (which was also the case in
the legacy shell).

Author: addaleax

packages/cli-repl/test/e2e-direct.spec.ts

@@ -185,7 +185,7 @@ describe('e2e direct connection', () => {
         });
         it('when specifying multiple seeds through --host with a wrong replsetid', async() => {
           const hostlist = 'wrongreplset/' + await rs2.hostport() + ',' + await rs1.hostport() + ',' + await rs0.hostport();
-          const shell = TestShell.start({ args: ['--host', hostlist, 'admin?serverSelectionTimeoutMS=2000'] });
+          const shell = TestShell.start({ args: ['--host', hostlist, 'admin'] });
           await shell.waitForExit();
           shell.assertContainsOutput('MongoServerSelectionError');
         });

packages/cli-repl/test/e2e.spec.ts

@@ -77,57 +77,51 @@ describe('e2e', function() {
       expect(await onExit).to.equal(0);
     });
   });
-
   describe('set db', () => {
-    describe('via host:port/test', () => {
-      let shell;
-      beforeEach(async() => {
-        shell = TestShell.start({ args: [`${await testServer.hostport()}/testdb1`] });
-        await shell.waitForPrompt();
-        shell.assertNoErrors();
-      });
-      it('db set correctly', async() => {
-        await shell.executeLine('db');
-        shell.assertNoErrors();
-
-        await eventually(() => {
-          shell.assertContainsOutput('testdb1');
+    for (const { mode, dbname, dbnameUri } of [
+      { mode: 'no special characetrs', dbname: 'testdb1', dbnameUri: 'testdb1' },
+      { mode: 'special characters', dbname: 'ä:-,🐈_\'[!?%', dbnameUri: 'ä:-,🐈_\'[!%3F%25' }
+    ]) {
+      context(mode, () => {
+        describe('via host:port/test', () => {
+          let shell;
+          beforeEach(async() => {
+            shell = TestShell.start({ args: [`${await testServer.hostport()}/${dbname}`] });
+            await shell.waitForPrompt();
+            shell.assertNoErrors();
+          });
+          it('db set correctly', async() => {
+            expect(await shell.executeLine('db')).to.include(dbname);
+            shell.assertNoErrors();
+          });
         });
-      });
-    });
-    describe('via mongodb://uri', () => {
-      let shell;
-      beforeEach(async() => {
-        shell = TestShell.start({ args: [`mongodb://${await testServer.hostport()}/testdb2`] });
-        await shell.waitForPrompt();
-        shell.assertNoErrors();
-      });
-      it('db set correctly', async() => {
-        await shell.executeLine('db');
-        shell.assertNoErrors();
-
-        await eventually(() => {
-          shell.assertContainsOutput('testdb2');
+        describe('via mongodb://uri', () => {
+          let shell;
+          beforeEach(async() => {
+            shell = TestShell.start({ args: [`mongodb://${await testServer.hostport()}/${dbnameUri}`] });
+            await shell.waitForPrompt();
+            shell.assertNoErrors();
+          });
+          it('db set correctly', async() => {
+            expect(await shell.executeLine('db')).to.include(dbname);
+            shell.assertNoErrors();
+          });
         });
-      });
-    });
-    describe('legacy db only', () => {
-      let shell;
-      beforeEach(async() => {
-        const port = await testServer.port();
-        shell = TestShell.start({ args: ['testdb3', `--port=${port}`] });
-        await shell.waitForPrompt();
-        shell.assertNoErrors();
-      });
-      it('db set correctly', async() => {
-        await shell.executeLine('db');
-        shell.assertNoErrors();
-
-        await eventually(() => {
-          shell.assertContainsOutput('testdb3');
+        describe('legacy db only', () => {
+          let shell;
+          beforeEach(async() => {
+            const port = await testServer.port();
+            shell = TestShell.start({ args: [dbname, `--port=${port}`] });
+            await shell.waitForPrompt();
+            shell.assertNoErrors();
+          });
+          it('db set correctly', async() => {
+            expect(await shell.executeLine('db')).to.include(dbname);
+            shell.assertNoErrors();
+          });
         });
       });
-    });
+    }
   });
 
   describe('with connection string', () => {

packages/service-provider-core/src/uri-generator.spec.ts

@@ -183,6 +183,30 @@ describe('uri-generator.generate-uri', () => {
       });
     });
 
+    context('when no additional options are provided with db with special characters', () => {
+      const uri = '192.0.0.1:27018/föö-:?%ab💙,\'_.c';
+      const options = { connectionSpecifier: uri };
+
+      it('returns the uri with the scheme', () => {
+        expect(generateUri(options)).to.equal('mongodb://192.0.0.1:27018/f%C3%B6%C3%B6-%3A%3F%25ab%F0%9F%92%99%2C\'_.c?directConnection=true');
+      });
+    });
+
+    context('when the db part does not start with a slash', () => {
+      const uri = '192.0.0.1:27018?foo=bar';
+      const options = { connectionSpecifier: uri };
+
+      it('throws an exception', () => {
+        try {
+          generateUri(options);
+          expect.fail('expected error');
+        } catch (e) {
+          expect(e).to.be.instanceOf(MongoshInvalidInputError);
+          expect(e.code).to.equal(CommonErrors.InvalidArgument);
+        }
+      });
+    });
+
     context('when additional options are provided', () => {
       context('when providing host with URI', () => {
         const uri = '192.0.0.1:27018/foo';
@@ -277,23 +301,23 @@ describe('uri-generator.generate-uri', () => {
 
     context('when providing a URI with query parameters', () => {
       context('that do not conflict with directConnection', () => {
-        const uri = '192.0.0.1:27018?readPreference=primary';
+        const uri = 'mongodb://192.0.0.1:27018/?readPreference=primary';
         const options = { connectionSpecifier: uri };
         it('still includes directConnection', () => {
           expect(generateUri(options)).to.equal('mongodb://192.0.0.1:27018/?readPreference=primary&directConnection=true');
         });
       });
 
       context('including replicaSet', () => {
-        const uri = '192.0.0.1:27018/db?replicaSet=replicaset';
+        const uri = 'mongodb://192.0.0.1:27018/db?replicaSet=replicaset';
         const options = { connectionSpecifier: uri };
         it('does not add the directConnection parameter', () => {
-          expect(generateUri(options)).to.equal(`mongodb://${uri}`);
+          expect(generateUri(options)).to.equal(uri);
         });
       });
 
       context('including explicit directConnection', () => {
-        const uri = '192.0.0.1:27018?directConnection=false';
+        const uri = 'mongodb://192.0.0.1:27018/?directConnection=false';
         const options = { connectionSpecifier: uri };
         it('does not change the directConnection parameter', () => {
           expect(generateUri(options)).to.equal('mongodb://192.0.0.1:27018/?directConnection=false');
@@ -346,5 +370,10 @@ describe('uri-generator.generate-uri', () => {
       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');
+    });
   });
 });

packages/service-provider-core/src/uri-generator.ts

@@ -129,8 +129,8 @@ function generateUri(options: CliOptions): string {
     return '';
   }
   const connectionString = generateUriNormalized(options);
-  if (connectionString.hosts.length === 1 &&
-      ['localhost', '127.0.0.1'].includes(connectionString.hosts[0].split(':')[0])) {
+  if (connectionString.hosts.every(host =>
+    ['localhost', '127.0.0.1'].includes(host.split(':')[0]))) {
     const params = connectionString.searchParams;
     if (!params.has('serverSelectionTimeoutMS')) {
       params.set('serverSelectionTimeoutMS', '2000');
@@ -147,7 +147,7 @@ function generateUriNormalized(options: CliOptions): ConnectionString {
     /^(?<replSetName>[^/]+)\/(?<hosts>([A-Za-z0-9.-]+(:\d+)?,?)+)$/);
   if (replSetHostMatch) {
     const { replSetName, hosts } = replSetHostMatch.groups as { replSetName: string, hosts: string };
-    const connectionString = new ConnectionString(`${Scheme.Mongo}replacemeHost/${uri ?? DEFAULT_DB}`);
+    const connectionString = new ConnectionString(`${Scheme.Mongo}replacemeHost/${encodeURIComponent(uri ?? DEFAULT_DB)}`);
     connectionString.hosts = hosts.split(',').filter(host => host.trim());
     connectionString.searchParams.set('replicaSet', replSetName);
     return addShellConnectionStringParameters(connectionString);
@@ -169,17 +169,22 @@ function generateUriNormalized(options: CliOptions): ConnectionString {
   }
 
   // Capture host, port and db from the string and generate a URI from
-  // the parts.
-  const uriMatch = /^([A-Za-z0-9][A-Za-z0-9.-]+):?(\d+)?[\/]?(\S+)?$/gi;
-  const parts = uriMatch.exec(uri);
+  // the parts. If there is a db part, it *must* start with /.
+  const uriMatch = /^([A-Za-z0-9][A-Za-z0-9.-]+):?(\d+)?(?:\/(\S*))?$/gi;
+  let parts: string[] | null = uriMatch.exec(uri);
 
   if (parts === null) {
-    throw new MongoshInvalidInputError(`Invalid URI: ${uri}`, CommonErrors.InvalidArgument);
+    if (/[/\\. "$]/.test(uri)) {
+      // This cannot be a database name because 'uri' contains characters invalid in a database.
+      throw new MongoshInvalidInputError(`Invalid URI: ${uri}`, CommonErrors.InvalidArgument);
+    } else {
+      parts = [ uri, uri ];
+    }
   }
 
-  let host: string | undefined = parts[1];
-  const port = parts[2];
-  let dbAndQueryString = parts[3];
+  let host: string | undefined = parts?.[1];
+  const port = parts?.[2];
+  let dbAndQueryString = parts?.[3];
 
   // If there is no port and db, host becomes db if there is no
   // '.' in the string. (legacy shell behaviour)
@@ -193,9 +198,8 @@ function generateUriNormalized(options: CliOptions): ConnectionString {
   if (host || port) {
     validateConflicts(options);
   }
-
   return addShellConnectionStringParameters(new ConnectionString(
-    `${Scheme.Mongo}${host || generateHost(options)}:${port || generatePort(options)}/${dbAndQueryString ?? DEFAULT_DB}`));
+    `${Scheme.Mongo}${host || generateHost(options)}:${port || generatePort(options)}/${encodeURIComponent(dbAndQueryString || DEFAULT_DB)}`));
 }
 
 /**