From 4867d92ddb8a90c900d1f1db2bf6c1bb1bd39769 Mon Sep 17 00:00:00 2001 From: emadum Date: Wed, 30 Jun 2021 15:14:41 -0400 Subject: [PATCH 1/4] fix(NODE-3290): backport versioned api validation and tests --- lib/core/index.js | 4 ++ lib/mongo_client.js | 25 ++++++++++ test/functional/versioned-api.test.js | 71 +++++++++++++++++++++++++-- 3 files changed, 96 insertions(+), 4 deletions(-) diff --git a/lib/core/index.js b/lib/core/index.js index cf3bbfed7b..0dd05f975c 100644 --- a/lib/core/index.js +++ b/lib/core/index.js @@ -15,6 +15,10 @@ try { } catch (err) {} // eslint-disable-line module.exports = { + // Versioned API + ServerApiVersion: Object.freeze({ + v1: '1' + }), // Errors MongoError: require('./error').MongoError, MongoNetworkError: require('./error').MongoNetworkError, diff --git a/lib/mongo_client.js b/lib/mongo_client.js index 46f8a85428..98f2d0a951 100644 --- a/lib/mongo_client.js +++ b/lib/mongo_client.js @@ -5,6 +5,7 @@ const Db = require('./db'); const EventEmitter = require('events').EventEmitter; const inherits = require('util').inherits; const MongoError = require('./core').MongoError; +const ServerApiVersion = require('./core').ServerApiVersion; const deprecate = require('util').deprecate; const WriteConcern = require('./write_concern'); const MongoDBNamespace = require('./utils').MongoDBNamespace; @@ -14,6 +15,7 @@ const NativeTopology = require('./topologies/native_topology'); const connect = require('./operations/connect').connect; const validOptions = require('./operations/connect').validOptions; + /** * @fileOverview The **MongoClient** class is a class that allows for making Connections to MongoDB. * @@ -183,6 +185,8 @@ const validOptions = require('./operations/connect').validOptions; * @property {function} [callback] The command result callback */ + + /** * Creates a new MongoClient instance * @constructor @@ -197,6 +201,27 @@ function MongoClient(url, options) { if (options && options.autoEncryption) require('./encrypter'); // Does CSFLE lib check + if (options.serverApi) { + const serverApiToValidate = + typeof options.serverApi === 'string' ? { version: options.serverApi } : options.serverApi; + const versionToValidate = serverApiToValidate && serverApiToValidate.version; + if (!versionToValidate) { + throw new MongoError( + `Invalid \`serverApi\` property; must specify a version from the following enum: ["${Object.values( + ServerApiVersion + ).join('", "')}"]` + ); + } + if (!Object.values(ServerApiVersion).some(v => v === versionToValidate)) { + throw new MongoError( + `Invalid server API version=${versionToValidate}; must be in the following enum: ["${Object.values( + ServerApiVersion + ).join('", "')}"]` + ); + } + options.serverApi = serverApiToValidate; + } + // The internal state this.s = { url: url, diff --git a/test/functional/versioned-api.test.js b/test/functional/versioned-api.test.js index 8e73a51bb0..523bded7f2 100644 --- a/test/functional/versioned-api.test.js +++ b/test/functional/versioned-api.test.js @@ -3,17 +3,80 @@ const expect = require('chai').expect; const loadSpecTests = require('../spec/index').loadSpecTests; const runUnifiedTest = require('./unified-spec-runner/runner').runUnifiedTest; +const ServerApiVersion = require('../../lib/core').ServerApiVersion; describe('Versioned API', function() { - it('should throw an error if serverApi version is provided via the uri with new parser', { - metadata: { topology: 'single' }, - test: function(done) { + describe('client option validation', function() { + it('is supported as a client option when it is a valid ServerApiVersion string', function() { + const validVersions = Object.values(ServerApiVersion); + expect(validVersions.length).to.be.at.least(1); + for (const version of validVersions) { + const client = this.configuration.newClient('mongodb://localhost/', { + serverApi: version + }); + expect(client.s.options) + .to.have.property('serverApi') + .deep.equal({ version }); + } + }); + + it('is supported as a client option when it is an object with a valid version property', function() { + const validVersions = Object.values(ServerApiVersion); + expect(validVersions.length).to.be.at.least(1); + for (const version of validVersions) { + const client = this.configuration.newClient('mongodb://localhost/', { + serverApi: { version } + }); + expect(client.s.options) + .to.have.property('serverApi') + .deep.equal({ version }); + } + }); + + it('is not supported as a client option when it is an invalid string', function() { + expect(() => + this.configuration.newClient('mongodb://localhost/', { + serverApi: 'bad' + }) + ).to.throw(/^Invalid server API version=bad;/); + }); + + it('is not supported as a client option when it is a number', function() { + expect(() => + this.configuration.newClient('mongodb://localhost/', { + serverApi: 1 + }) + ).to.throw(/^Invalid `serverApi` property;/); + }); + + it('is not supported as a client option when it is an object without a specified version', function() { + expect(() => + this.configuration.newClient('mongodb://localhost/', { + serverApi: {} + }) + ).to.throw(/^Invalid `serverApi` property;/); + }); + + it('is not supported as a client option when it is an object with an invalid specified version', function() { + expect(() => + this.configuration.newClient('mongodb://localhost/', { + serverApi: { version: 1 } + }) + ).to.throw(/^Invalid server API version=1;/); + expect(() => + this.configuration.newClient('mongodb://localhost/', { + serverApi: { version: 'bad' } + }) + ).to.throw(/^Invalid server API version=bad;/); + }); + + it('is not supported as a URI option even when it is a valid ServerApiVersion string', function(done) { const client = this.configuration.newClient({ serverApi: '1' }, { useNewUrlParser: true }); client.connect(err => { expect(err).to.match(/URI cannot contain `serverApi`, it can only be passed to the client/); client.close(done); }); - } + }); }); for (const versionedApiTest of loadSpecTests('versioned-api')) { From b2e3b3a50d919a66a06ed84a4d064894acf3657d Mon Sep 17 00:00:00 2001 From: emadum Date: Wed, 30 Jun 2021 15:15:31 -0400 Subject: [PATCH 2/4] chore(ci): update cluster setup script from 4.0, add versioned api support --- lib/mongo_client.js | 8 +++----- test/tools/cluster_setup.sh | 24 +++++++++++++++++++----- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/lib/mongo_client.js b/lib/mongo_client.js index 98f2d0a951..9fdce5cd29 100644 --- a/lib/mongo_client.js +++ b/lib/mongo_client.js @@ -15,7 +15,6 @@ const NativeTopology = require('./topologies/native_topology'); const connect = require('./operations/connect').connect; const validOptions = require('./operations/connect').validOptions; - /** * @fileOverview The **MongoClient** class is a class that allows for making Connections to MongoDB. * @@ -185,8 +184,6 @@ const validOptions = require('./operations/connect').validOptions; * @property {function} [callback] The command result callback */ - - /** * Creates a new MongoClient instance * @constructor @@ -195,6 +192,7 @@ const validOptions = require('./operations/connect').validOptions; * @param {MongoClientOptions} [options] Optional settings */ function MongoClient(url, options) { + options = options || {}; if (!(this instanceof MongoClient)) return new MongoClient(url, options); // Set up event emitter EventEmitter.call(this); @@ -224,8 +222,8 @@ function MongoClient(url, options) { // The internal state this.s = { - url: url, - options: options || {}, + url, + options, promiseLibrary: (options && options.promiseLibrary) || Promise, dbCache: new Map(), sessions: new Set(), diff --git a/test/tools/cluster_setup.sh b/test/tools/cluster_setup.sh index 2f12d17f94..36beca5dc9 100755 --- a/test/tools/cluster_setup.sh +++ b/test/tools/cluster_setup.sh @@ -2,17 +2,31 @@ if [ "$#" -ne 1 ]; then echo "usage: cluster_setup " + echo "override env variables to change dbPath" exit fi +DATA_DIR=${DATA_DIR:-data} +SINGLE_DIR=${SINGLE_DIR:-$DATA_DIR/server} +REPLICASET_DIR=${REPLICASET_DIR:-$DATA_DIR/replica_set} +SHARDED_DIR=${SHARDED_DIR:-$DATA_DIR/sharded_cluster} + +if [[ ! -z "$MONGODB_API_VERSION" ]]; then + echo "Requiring versioned API $MONGODB_API_VERSION" + REQUIRE_API="--setParameter requireApiVersion=$MONGODB_API_VERSION" +fi + if [[ $1 == "replica_set" ]]; then - mlaunch init --replicaset --nodes 3 --arbiter --name rs --port 31000 --enableMajorityReadConcern --setParameter enableTestCommands=1 - echo "mongodb://localhost:31000/?replicaSet=rs" + mkdir -p $REPLICASET_DIR + mlaunch init --dir $REPLICASET_DIR --replicaset --nodes 3 --arbiter --name rs --port 31000 --enableMajorityReadConcern --setParameter enableTestCommands=1 + echo "mongodb://localhost:31000,localhost:31001,localhost:31002/?replicaSet=rs" elif [[ $1 == "sharded_cluster" ]]; then - mlaunch init --replicaset --nodes 3 --arbiter --name rs --port 51000 --enableMajorityReadConcern --setParameter enableTestCommands=1 --sharded 1 --mongos 2 - echo "mongodb://localhost:51000,localhost:51001/" + mkdir -p $SHARDED_DIR + mlaunch init --dir $SHARDED_DIR --replicaset --nodes 3 --arbiter --name rs --port 51000 --enableMajorityReadConcern --setParameter enableTestCommands=1 --sharded 1 --mongos 2 + echo "mongodb://localhost:51000,localhost:51001" elif [[ $1 == "server" ]]; then - mlaunch init --single --setParameter enableTestCommands=1 + mkdir -p $SINGLE_DIR + mlaunch init --dir $SINGLE_DIR --single --setParameter enableTestCommands=1 $REQUIRE_API echo "mongodb://localhost:27017" else echo "unsupported topology: $1" From a370bbb9d2bad03a37f6aef0b82f6d15c92bddbc Mon Sep 17 00:00:00 2001 From: emadum Date: Fri, 2 Jul 2021 13:16:06 -0400 Subject: [PATCH 3/4] review feedback --- index.js | 3 +++ lib/core/index.js | 9 ++++++--- lib/mongo_client.js | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index 4e9e6359e8..ec572201bf 100644 --- a/index.js +++ b/index.js @@ -17,6 +17,9 @@ connect.MongoWriteConcernError = core.MongoWriteConcernError; connect.MongoBulkWriteError = require('./lib/bulk/common').BulkWriteError; connect.BulkWriteError = connect.MongoBulkWriteError; +// Expose server versions +connect.ServerApiVersion = core.ServerApiVersion; + // Actual driver classes exported connect.Admin = require('./lib/admin'); connect.MongoClient = require('./lib/mongo_client'); diff --git a/lib/core/index.js b/lib/core/index.js index 0dd05f975c..f1352c2e66 100644 --- a/lib/core/index.js +++ b/lib/core/index.js @@ -14,11 +14,14 @@ try { } } catch (err) {} // eslint-disable-line +/** An enumeration of valid server API versions */ +const ServerApiVersion = Object.freeze({ + v1: '1' +}); + module.exports = { // Versioned API - ServerApiVersion: Object.freeze({ - v1: '1' - }), + ServerApiVersion, // Errors MongoError: require('./error').MongoError, MongoNetworkError: require('./error').MongoNetworkError, diff --git a/lib/mongo_client.js b/lib/mongo_client.js index e746639365..7c65e0d37f 100644 --- a/lib/mongo_client.js +++ b/lib/mongo_client.js @@ -198,7 +198,7 @@ function MongoClient(url, options) { // Set up event emitter EventEmitter.call(this); - if (options && options.autoEncryption) require('./encrypter'); // Does CSFLE lib check + if (options.autoEncryption) require('./encrypter'); // Does CSFLE lib check if (options.serverApi) { const serverApiToValidate = From 4121e11124105b67072b4fcf4b261774ac78b9f3 Mon Sep 17 00:00:00 2001 From: emadum Date: Fri, 2 Jul 2021 13:28:21 -0400 Subject: [PATCH 4/4] fix merge error --- lib/command_utils.js | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/lib/command_utils.js b/lib/command_utils.js index a13a833a4d..a8f52b2d73 100644 --- a/lib/command_utils.js +++ b/lib/command_utils.js @@ -2,10 +2,7 @@ const Msg = require('./core/connection/msg').Msg; const KillCursor = require('./core/connection/commands').KillCursor; const GetMore = require('./core/connection/commands').GetMore; -<<<<<<< HEAD -======= const deepCopy = require('./utils').deepCopy; ->>>>>>> origin/3.6 /** Commands that we want to redact because of the sensitive nature of their contents */ const SENSITIVE_COMMANDS = new Set([ @@ -67,28 +64,17 @@ const extractCommand = command => { let extractedCommand; if (command instanceof GetMore) { extractedCommand = { -<<<<<<< HEAD - getMore: command.cursorId, -======= getMore: deepCopy(command.cursorId), ->>>>>>> origin/3.6 collection: collectionName(command), batchSize: command.numberToReturn }; } else if (command instanceof KillCursor) { extractedCommand = { killCursors: collectionName(command), -<<<<<<< HEAD - cursors: command.cursorIds - }; - } else if (command instanceof Msg) { - extractedCommand = command.command; -======= cursors: deepCopy(command.cursorIds) }; } else if (command instanceof Msg) { extractedCommand = deepCopy(command.command); ->>>>>>> origin/3.6 } else if (command.query && command.query.$query) { let result; if (command.ns === 'admin.$cmd') { @@ -99,21 +85,13 @@ const extractCommand = command => { result = { find: collectionName(command) }; Object.keys(LEGACY_FIND_QUERY_MAP).forEach(key => { if (typeof command.query[key] !== 'undefined') -<<<<<<< HEAD - result[LEGACY_FIND_QUERY_MAP[key]] = command.query[key]; -======= result[LEGACY_FIND_QUERY_MAP[key]] = deepCopy(command.query[key]); ->>>>>>> origin/3.6 }); } Object.keys(LEGACY_FIND_OPTIONS_MAP).forEach(key => { -<<<<<<< HEAD - if (typeof command[key] !== 'undefined') result[LEGACY_FIND_OPTIONS_MAP[key]] = command[key]; -======= if (typeof command[key] !== 'undefined') result[LEGACY_FIND_OPTIONS_MAP[key]] = deepCopy(command[key]); ->>>>>>> origin/3.6 }); OP_QUERY_KEYS.forEach(key => { @@ -130,11 +108,7 @@ const extractCommand = command => { extractedCommand = result; } } else { -<<<<<<< HEAD - extractedCommand = command.query || command; -======= extractedCommand = deepCopy(command.query || command); ->>>>>>> origin/3.6 } const commandName = Object.keys(extractedCommand)[0];