Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
21 changes: 1 addition & 20 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 @@ -146,9 +146,6 @@ export class Db {
// 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 @@ -519,19 +516,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]}'`);
}
}
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('should fail on server due to illegal collection name', async function () {
try {
const illegalCollection = await db.createCollection('a\x00b');
await illegalCollection.insertOne({ a: 1 });
expect.fail('a MongoServerError was expected due to illegal collection name');
} catch (error) {
expect(error instanceof MongoServerError);
}
});

it('should correctly count on non-existent collection', function (done) {
Expand Down
49 changes: 25 additions & 24 deletions test/integration/node-specific/db.test.js
Original file line number Diff line number Diff line change
@@ -1,37 +1,38 @@
'use strict';

import { MongoServerError } from 'mongodb-legacy';

const { setupDatabase, assert: test } = require(`../shared`);
const { expect } = require('chai');
const { Db, MongoClient } = require('../../mongodb');
const { MongoClient } = 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 () {
try {
db = client.db('a\x00b');
await db.createCollection('spider');
expect.fail('a MongoServerError was expected due to illegal db name');
} catch (error) {
expect(error instanceof MongoServerError);
}
});
});

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