Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
31 changes: 9 additions & 22 deletions src/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -134,21 +134,20 @@ export class Db {
public static SYSTEM_JS_COLLECTION = CONSTANTS.SYSTEM_JS_COLLECTION;

/**
* Creates a new Db instance
* Creates a new Db instance.
*
* Database name validation occurs at operation time.
*
* @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);

// Internal state of the db object
this.s = {
// Options
Expand Down Expand Up @@ -218,6 +217,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
*/
Expand Down Expand Up @@ -294,6 +295,8 @@ export class Db {
/**
* Returns a reference to a MongoDB Collection. If it does not exist it will be created implicitly.
*
* Collection namespace validation occurs at operation time.
*
* @param name - the collection name we wish to access.
* @returns return the new Collection instance
*/
Expand Down Expand Up @@ -519,19 +522,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]}'`);
}
}
2 changes: 2 additions & 0 deletions src/mongo_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,8 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> {
/**
* Create a new Db instance sharing the current socket connections.
*
* Db name validation occurs at operation time
*
* @param dbName - The name of the database we want to use. If not provided, use database name from connection string.
* @param options - Optional settings for Db construction
*/
Expand Down
3 changes: 1 addition & 2 deletions src/operations/rename.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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');
}
Expand Down
33 changes: 0 additions & 33 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 9 additions & 13 deletions test/integration/collection-management/collection.test.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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) {
Expand Down
45 changes: 21 additions & 24 deletions test/integration/node-specific/db.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,33 @@

const { setupDatabase, assert: test } = require(`../shared`);
const { expect } = require('chai');
const { Db, MongoClient } = require('../../mongodb');
const { MongoClient, MongoServerError } = require('../../mongodb');

describe('Db', function () {
before(function () {
return setupDatabase(this.configuration);
});

it('shouldCorrectlyHandleIllegalDbNames', {
metadata: {
requires: { topology: ['single', 'replicaset', 'sharded'] }
},

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('when given illegal db name', function () {
let client;
let db;

beforeEach(function () {
client = this.configuration.newClient();
});

afterEach(async function () {
db = undefined;
await client.close();
});

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');
});
});

it('shouldCorrectlyHandleFailedConnection', {
Expand Down
92 changes: 0 additions & 92 deletions test/integration/node-specific/operation_examples.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1650,98 +1650,6 @@ describe('Operations', function () {
}
});

/**
* An example of illegal and legal renaming of a collection using a Promise.
*
* example-class Collection
* example-method rename
*/
it('shouldCorrectlyRenameCollectionWithPromises', {
metadata: {
requires: { topology: ['single'] }
},

test: async function () {
const configuration = this.configuration;
const client: MongoClient = configuration.newClient(configuration.writeConcernMax(), {
maxPoolSize: 1
});
// LINE var MongoClient = require('mongodb').MongoClient,
// LINE test = require('assert');
// LINE const client = new MongoClient('mongodb://localhost:27017/test');
// LINE client.connect().then(() => {
// LINE var db = client.db('test);
// REPLACE configuration.writeConcernMax() WITH {w:1}
// REMOVE-LINE done();
// BEGIN
// Open a couple of collections

await client.connect();
this.defer(async () => await client.close());
const db = client.db(configuration.db);
/* es-lint-disable */
let [collection1] = await Promise.all([
db.createCollection('test_rename_collection_with_promise'),
db.createCollection('test_rename_collection2_with_promise')
]);

// Attempt to rename a collection to a number
// @ts-expect-error need to test that it will fail on number inputs
let err = await collection1.rename(5).catch(e => e);
expect(err).to.be.instanceOf(Error);
expect(err).to.have.property('message', 'Collection name must be a String');

// Attempt to rename a collection to an empty string
err = await collection1.rename('').catch(e => e);
expect(err).to.be.instanceOf(Error);
expect(err).to.have.property('message', 'Collection names cannot be empty');

// Attemp to rename a collection to an illegal name including the character $
err = await collection1.rename('te$t').catch(e => e);
expect(err).to.be.instanceOf(Error);
expect(err).to.have.property('message', "Collection names must not contain '$'");

// Attempt to rename a collection to an illegal name starting with the character .
err = await collection1.rename('.test').catch(e => e);
expect(err).to.be.instanceOf(Error);
expect(err).to.have.property('message', "Collection names must not start or end with '.'");
// Attempt to rename a collection to an illegal name ending with the character .
err = await collection1.rename('test.').catch(e => e);
expect(err).to.be.instanceOf(Error);
expect(err).to.have.property('message', "Collection names must not start or end with '.'");

// Attempt to rename a collection to an illegal name with an empty middle name
err = await collection1.rename('tes..t').catch(e => e);
expect(err).to.be.instanceOf(Error);
expect(err).to.have.property('message', 'Collection names cannot be empty');

// Attempt to rename a collection to an illegal name with an empty middle name
err = await collection1.rename('tes..t').catch(e => e);
expect(err).to.be.instanceOf(Error);
expect(err).to.have.property('message', 'Collection names cannot be empty');

// Insert a couple of documents
await collection1.insertMany([{ x: 1 }, { x: 2 }], configuration.writeConcernMax());

// Attempt to rename the first collection to the second one, this will fail
err = await collection1.rename('test_rename_collection2_with_promise').catch(e => e);
expect(err).to.be.instanceOf(Error);
expect(err.message).to.have.length.gt(0);

// Attempt to rename the first collection to a name that does not exist
// this will be successful
collection1 = await collection1.rename('test_rename_collection3_with_promise');

// Ensure that the collection is pointing to the new one
expect(collection1.collectionName).to.equal('test_rename_collection3_with_promise');

expect(await collection1.count()).equals(2);

/* es-lint-enable */
// END
}
});

/**
* Example of a simple document update with safe set to false on an existing document using a Promise.
*
Expand Down