fix(NODE-5496): remove client-side collection and database name check validation (#3873)

Author: aditi-khare-mongoDB

src/cmap/command_monitoring_events.ts

@@ -7,7 +7,7 @@ import {
   LEGACY_HELLO_COMMAND_CAMEL_CASE
 } from '../constants';
 import { calculateDurationInMs, deepCopy } from '../utils';
-import { Msg, type WriteProtocolMessageType } from './commands';
+import { Msg, type Query, type WriteProtocolMessageType } from './commands';
 import type { Connection } from './connection';
 
 /**
@@ -49,7 +49,7 @@ export class CommandStartedEvent {
     this.connectionId = connectionId;
     this.serviceId = serviceId;
     this.requestId = command.requestId;
-    this.databaseName = databaseName(command);
+    this.databaseName = command.databaseName;
     this.commandName = commandName;
     this.command = maybeRedact(commandName, cmd, cmd);
   }
@@ -181,9 +181,8 @@ const HELLO_COMMANDS = new Set(['hello', LEGACY_HELLO_COMMAND, LEGACY_HELLO_COMM
 
 // helper methods
 const extractCommandName = (commandDoc: Document) => Object.keys(commandDoc)[0];
-const namespace = (command: WriteProtocolMessageType) => command.ns;
-const databaseName = (command: WriteProtocolMessageType) => command.ns.split('.')[0];
-const collectionName = (command: WriteProtocolMessageType) => command.ns.split('.')[1];
+const namespace = (command: Query) => command.ns;
+const collectionName = (command: Query) => command.ns.split('.')[1];
 const maybeRedact = (commandName: string, commandDoc: Document, result: Error | Document) =>
   SENSITIVE_COMMANDS.has(commandName) ||
   (HELLO_COMMANDS.has(commandName) && commandDoc.speculativeAuthenticate)

src/cmap/commands.ts

@@ -3,7 +3,6 @@ import * as BSON from '../bson';
 import { MongoInvalidArgumentError, MongoRuntimeError } from '../error';
 import { ReadPreference } from '../read_preference';
 import type { ClientSession } from '../sessions';
-import { databaseNamespace } from '../utils';
 import type { CommandOptions } from './connection';
 import { OP_MSG, OP_QUERY } from './wire_protocol/constants';
 
@@ -55,7 +54,6 @@ export interface OpQueryOptions extends CommandOptions {
 /** @internal */
 export class Query {
   ns: string;
-  query: Document;
   numberToSkip: number;
   numberToReturn: number;
   returnFieldSelector?: Document;
@@ -75,10 +73,13 @@ export class Query {
   partial: boolean;
   documentsReturnedIn?: string;
 
-  constructor(ns: string, query: Document, options: OpQueryOptions) {
+  constructor(public databaseName: string, public query: Document, options: OpQueryOptions) {
     // Basic options needed to be passed in
     // TODO(NODE-3483): Replace with MongoCommandError
-    if (ns == null) throw new MongoRuntimeError('Namespace must be specified for query');
+    const ns = `${databaseName}.$cmd`;
+    if (typeof databaseName !== 'string') {
+      throw new MongoRuntimeError('Database name must be a string for a query');
+    }
     // TODO(NODE-3483): Replace with MongoCommandError
     if (query == null) throw new MongoRuntimeError('A query document must be specified for query');
 
@@ -90,7 +91,6 @@ export class Query {
 
     // Basic options
     this.ns = ns;
-    this.query = query;
 
     // Additional options
     this.numberToSkip = options.numberToSkip || 0;
@@ -473,9 +473,6 @@ export interface OpMsgOptions {
 
 /** @internal */
 export class Msg {
-  ns: string;
-  command: Document;
-  options: OpQueryOptions;
   requestId: number;
   serializeFunctions: boolean;
   ignoreUndefined: boolean;
@@ -485,15 +482,17 @@ export class Msg {
   moreToCome: boolean;
   exhaustAllowed: boolean;
 
-  constructor(ns: string, command: Document, options: OpQueryOptions) {
+  constructor(
+    public databaseName: string,
+    public command: Document,
+    public options: OpQueryOptions
+  ) {
     // Basic options needed to be passed in
     if (command == null)
       throw new MongoInvalidArgumentError('Query document must be specified for query');
 
     // Basic options
-    this.ns = ns;
-    this.command = command;
-    this.command.$db = databaseNamespace(ns);
+    this.command.$db = databaseName;
 
     if (options.readPreference && options.readPreference.mode !== ReadPreference.PRIMARY) {
       this.command.$readPreference = options.readPreference.toJSON();

src/cmap/connection.ts

@@ -539,10 +539,9 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
       options
     );
 
-    const cmdNs = `${ns.db}.$cmd`;
     const message = shouldUseOpMsg
-      ? new Msg(cmdNs, cmd, commandOptions)
-      : new Query(cmdNs, cmd, commandOptions);
+      ? new Msg(ns.db, cmd, commandOptions)
+      : new Query(ns.db, cmd, commandOptions);
 
     try {
       write(this, message, commandOptions, callback);

src/collection.ts

@@ -90,7 +90,6 @@ import {
 import { ReadConcern, type ReadConcernLike } from './read_concern';
 import { ReadPreference, type ReadPreferenceLike } from './read_preference';
 import {
-  checkCollectionName,
   DEFAULT_PK_FACTORY,
   MongoDBCollectionNamespace,
   normalizeHintField,
@@ -164,8 +163,6 @@ export class Collection<TSchema extends Document = Document> {
    * @internal
    */
   constructor(db: Db, name: string, options?: CollectionOptions) {
-    checkCollectionName(name);
-
     // Internal state
     this.s = {
       db,

src/db.ts

@@ -6,7 +6,7 @@ import * as CONSTANTS from './constants';
 import { AggregationCursor } from './cursor/aggregation_cursor';
 import { ListCollectionsCursor } from './cursor/list_collections_cursor';
 import { RunCommandCursor, type RunCursorCommandOptions } from './cursor/run_command_cursor';
-import { MongoAPIError, MongoInvalidArgumentError } from './error';
+import { MongoInvalidArgumentError } from './error';
 import type { MongoClient, PkFactory } from './mongo_client';
 import type { TODO_NODE_3286 } from './mongo_types';
 import type { AggregateOptions } from './operations/aggregate';
@@ -134,20 +134,24 @@ export class Db {
   public static SYSTEM_JS_COLLECTION = CONSTANTS.SYSTEM_JS_COLLECTION;
 
   /**
-   * Creates a new Db instance
+   * Creates a new Db instance.
+   *
+   * Db name cannot contain a dot, the server may apply more restrictions when an operation is run.
    *
    * @param client - The MongoClient for the database.
    * @param databaseName - The name of the database this instance represents.
-   * @param options - Optional settings for Db construction
+   * @param options - Optional settings for Db construction.
    */
   constructor(client: MongoClient, databaseName: string, options?: DbOptions) {
     options = options ?? {};
 
     // Filter the options
     options = filterOptions(options, DB_OPTIONS_ALLOW_LIST);
 
-    // Ensure we have a valid db name
-    validateDatabaseName(databaseName);
+    // Ensure there are no dots in database name
+    if (typeof databaseName === 'string' && databaseName.includes('.')) {
+      throw new MongoInvalidArgumentError(`Database names cannot contain the character '.'`);
+    }
 
     // Internal state of the db object
     this.s = {
@@ -218,6 +222,8 @@ export class Db {
    * Create a new collection on a server with the specified options. Use this to create capped collections.
    * More information about command options available at https://www.mongodb.com/docs/manual/reference/command/create/
    *
+   * Collection namespace validation is performed server-side.
+   *
    * @param name - The name of the collection to create
    * @param options - Optional settings for the command
    */
@@ -294,6 +300,8 @@ export class Db {
   /**
    * Returns a reference to a MongoDB Collection. If it does not exist it will be created implicitly.
    *
+   * Collection namespace validation is performed server-side.
+   *
    * @param name - the collection name we wish to access.
    * @returns return the new Collection instance
    */
@@ -519,19 +527,3 @@ export class Db {
     return new RunCommandCursor(this, command, options);
   }
 }
-
-// TODO(NODE-3484): Refactor into MongoDBNamespace
-// Validate the database name
-function validateDatabaseName(databaseName: string) {
-  if (typeof databaseName !== 'string')
-    throw new MongoInvalidArgumentError('Database name must be a string');
-  if (databaseName.length === 0)
-    throw new MongoInvalidArgumentError('Database name cannot be the empty string');
-  if (databaseName === '$external') return;
-
-  const invalidChars = [' ', '.', '$', '/', '\\'];
-  for (let i = 0; i < invalidChars.length; i++) {
-    if (databaseName.indexOf(invalidChars[i]) !== -1)
-      throw new MongoAPIError(`database names cannot contain the character '${invalidChars[i]}'`);
-  }
-}

src/operations/rename.ts

@@ -2,7 +2,7 @@ import type { Document } from '../bson';
 import { Collection } from '../collection';
 import type { Server } from '../sdam/server';
 import type { ClientSession } from '../sessions';
-import { checkCollectionName, MongoDBNamespace } from '../utils';
+import { MongoDBNamespace } from '../utils';
 import { CommandOperation, type CommandOperationOptions } from './command';
 import { Aspect, defineAspects } from './operation';
 
@@ -21,7 +21,6 @@ export class RenameOperation extends CommandOperation<Document> {
     public newName: string,
     public override options: RenameOptions
   ) {
-    checkCollectionName(newName);
     super(collection, options);
     this.ns = new MongoDBNamespace('admin', '$cmd');
   }

src/utils.ts

@@ -78,39 +78,6 @@ export function hostMatchesWildcards(host: string, wildcards: string[]): boolean
   return false;
 }
 
-/**
- * Throws if collectionName is not a valid mongodb collection namespace.
- * @internal
- */
-export function checkCollectionName(collectionName: string): void {
-  if ('string' !== typeof collectionName) {
-    throw new MongoInvalidArgumentError('Collection name must be a String');
-  }
-
-  if (!collectionName || collectionName.indexOf('..') !== -1) {
-    throw new MongoInvalidArgumentError('Collection names cannot be empty');
-  }
-
-  if (
-    collectionName.indexOf('$') !== -1 &&
-    collectionName.match(/((^\$cmd)|(oplog\.\$main))/) == null
-  ) {
-    // TODO(NODE-3483): Use MongoNamespace static method
-    throw new MongoInvalidArgumentError("Collection names must not contain '$'");
-  }
-
-  if (collectionName.match(/^\.|\.$/) != null) {
-    // TODO(NODE-3483): Use MongoNamespace static method
-    throw new MongoInvalidArgumentError("Collection names must not start or end with '.'");
-  }
-
-  // Validate that we are not passing 0x00 in the collection name
-  if (collectionName.indexOf('\x00') !== -1) {
-    // TODO(NODE-3483): Use MongoNamespace static method
-    throw new MongoInvalidArgumentError('Collection names cannot contain a null character');
-  }
-}
-
 /**
  * Ensure Hint field is in a shape we expect:
  * - object of index names mapping to 1 or -1
@@ -386,11 +353,6 @@ export function maybeCallback<T>(
   return;
 }
 
-/** @internal */
-export function databaseNamespace(ns: string): string {
-  return ns.split('.')[0];
-}
-
 /**
  * Synchronously Generate a UUIDv4
  * @internal

test/integration/collection-management/collection.test.ts

@@ -1,6 +1,6 @@
 import { expect } from 'chai';
 
-import { Collection, type Db, isHello, type MongoClient } from '../../mongodb';
+import { Collection, type Db, isHello, type MongoClient, MongoServerError } from '../../mongodb';
 import * as mock from '../../tools/mongodb-mock/index';
 import { setupDatabase } from '../shared';
 
@@ -95,18 +95,14 @@ describe('Collection', function () {
       ]);
     });
 
-    it('should fail due to illegal listCollections', function (done) {
-      expect(() => db.collection(5)).to.throw('Collection name must be a String');
-      expect(() => db.collection('')).to.throw('Collection names cannot be empty');
-      expect(() => db.collection('te$t')).to.throw("Collection names must not contain '$'");
-      expect(() => db.collection('.test')).to.throw(
-        "Collection names must not start or end with '.'"
-      );
-      expect(() => db.collection('test.')).to.throw(
-        "Collection names must not start or end with '.'"
-      );
-      expect(() => db.collection('test..t')).to.throw('Collection names cannot be empty');
-      done();
+    it('fails on server due to invalid namespace', async function () {
+      const error = await db
+        .collection('a\x00b')
+        .insertOne({ a: 1 })
+        .catch(error => error);
+      expect(error).to.be.instanceOf(MongoServerError);
+      expect(error).to.have.property('code', 73);
+      expect(error).to.have.property('codeName', 'InvalidNamespace');
     });
 
     it('should correctly count on non-existent collection', function (done) {

test/integration/node-specific/db.test.js

@@ -2,36 +2,47 @@
 
 const { setupDatabase, assert: test } = require(`../shared`);
 const { expect } = require('chai');
-const { Db, MongoClient } = require('../../mongodb');
+const { MongoClient, MongoInvalidArgumentError, MongoServerError } = require('../../mongodb');
 
 describe('Db', function () {
   before(function () {
     return setupDatabase(this.configuration);
   });
 
-  it('shouldCorrectlyHandleIllegalDbNames', {
-    metadata: {
-      requires: { topology: ['single', 'replicaset', 'sharded'] }
-    },
+  context('when given illegal db name', function () {
+    let client;
+    let db;
+
+    beforeEach(function () {
+      client = this.configuration.newClient();
+    });
+
+    afterEach(async function () {
+      db = undefined;
+      await client.close();
+    });
+
+    context('of type string, containing no dot characters', function () {
+      it('should throw error on server only', async function () {
+        db = client.db('a\x00b');
+        const error = await db.createCollection('spider').catch(error => error);
+        expect(error).to.be.instanceOf(MongoServerError);
+        expect(error).to.have.property('code', 73);
+        expect(error).to.have.property('codeName', 'InvalidNamespace');
+      });
+    });
 
-    test: done => {
-      const client = { bsonOptions: {} };
-      expect(() => new Db(client, 5)).to.throw('Database name must be a string');
-      expect(() => new Db(client, '')).to.throw('Database name cannot be the empty string');
-      expect(() => new Db(client, 'te$t')).to.throw(
-        "database names cannot contain the character '$'"
-      );
-      expect(() => new Db(client, '.test', function () {})).to.throw(
-        "database names cannot contain the character '.'"
-      );
-      expect(() => new Db(client, '\\test', function () {})).to.throw(
-        "database names cannot contain the character '\\'"
-      );
-      expect(() => new Db(client, 'test test', function () {})).to.throw(
-        "database names cannot contain the character ' '"
-      );
-      done();
-    }
+    context('of type string, containing a dot character', function () {
+      it('should throw MongoInvalidArgumentError', function () {
+        expect(() => client.db('a.b')).to.throw(MongoInvalidArgumentError);
+      });
+    });
+
+    context('of type non-string type', function () {
+      it('should not throw client-side', function () {
+        expect(() => client.db(5)).to.not.throw();
+      });
+    });
   });
 
   it('shouldCorrectlyHandleFailedConnection', {