From 8845fb09ae597dc3eb4cdf85a2c22a9ad31f11f2 Mon Sep 17 00:00:00 2001 From: Lingzhi Deng Date: Thu, 9 May 2019 14:36:12 -0400 Subject: SERVER-40423: Move command validation from session to CommandHelpers (cherry picked from commit 5674fa1f3087f65ea326b33f5b81647d4dcfb8d6 and 52b2cc0886cdb992e2491067bdc029301d5bb6af) --- jstests/core/txns/commands_not_allowed_in_txn.js | 12 ++++-- src/mongo/db/commands.cpp | 49 ++++++++++++++++++++++++ src/mongo/db/commands.h | 5 +++ src/mongo/db/service_entry_point_common.cpp | 4 ++ src/mongo/db/session.cpp | 43 --------------------- 5 files changed, 67 insertions(+), 46 deletions(-) diff --git a/jstests/core/txns/commands_not_allowed_in_txn.js b/jstests/core/txns/commands_not_allowed_in_txn.js index 5b39cdfa5c5..b366f03da9a 100644 --- a/jstests/core/txns/commands_not_allowed_in_txn.js +++ b/jstests/core/txns/commands_not_allowed_in_txn.js @@ -36,6 +36,8 @@ function testCommand(command) { jsTest.log("Testing command: " + tojson(command)); + const errmsgRegExp = new RegExp( + 'Cannot run .* in a multi-document transaction.\|This command is not supported in transactions'); // Check that the command runs successfully outside transactions. setup(); @@ -43,14 +45,16 @@ // Check that the command cannot be used to start a transaction. setup(); - assert.commandFailedWithCode(sessionDb.runCommand(Object.assign({}, command, { + let res = assert.commandFailedWithCode(sessionDb.runCommand(Object.assign({}, command, { readConcern: {level: "snapshot"}, txnNumber: NumberLong(++txnNumber), stmtId: NumberInt(0), startTransaction: true, autocommit: false })), - ErrorCodes.OperationNotSupportedInTransaction); + ErrorCodes.OperationNotSupportedInTransaction); + // Check that the command fails with expected error message. + assert(res.errmsg.match(errmsgRegExp), res); assert.commandFailedWithCode(sessionDb.adminCommand({ commitTransaction: 1, txnNumber: NumberLong(txnNumber), @@ -70,12 +74,14 @@ startTransaction: true, autocommit: false })); - assert.commandFailedWithCode( + res = assert.commandFailedWithCode( sessionDb.runCommand(Object.assign( {}, command, {txnNumber: NumberLong(txnNumber), stmtId: NumberInt(1), autocommit: false})), ErrorCodes.OperationNotSupportedInTransaction); + // Check that the command fails with expected error message. + assert(res.errmsg.match(errmsgRegExp), res); assert.commandWorked(sessionDb.adminCommand({ commitTransaction: 1, txnNumber: NumberLong(txnNumber), diff --git a/src/mongo/db/commands.cpp b/src/mongo/db/commands.cpp index 266c05c907c..b449af1b84a 100644 --- a/src/mongo/db/commands.cpp +++ b/src/mongo/db/commands.cpp @@ -102,6 +102,31 @@ bool checkAuthorizationImplPreParse(OperationContext* opCtx, return false; } +// The command names that are allowed in a multi-document transaction. +const StringMap txnCmdWhitelist = {{"abortTransaction", 1}, + {"aggregate", 1}, + {"commitTransaction", 1}, + {"delete", 1}, + {"distinct", 1}, + {"doTxn", 1}, + {"find", 1}, + {"findandmodify", 1}, + {"findAndModify", 1}, + {"geoSearch", 1}, + {"getMore", 1}, + {"insert", 1}, + {"killCursors", 1}, + {"prepareTransaction", 1}, + {"update", 1}}; + +// The command names that are allowed in a multi-document transaction only when test commands are +// enabled. +const StringMap txnCmdForTestingWhitelist = {{"dbHash", 1}}; + +// The commands that can be run on the 'admin' database in multi-document transactions. +const StringMap txnAdminCommands = { + {"abortTransaction", 1}, {"commitTransaction", 1}, {"doTxn", 1}, {"prepareTransaction", 1}}; + } // namespace @@ -385,6 +410,30 @@ bool CommandHelpers::uassertShouldAttemptParse(OperationContext* opCtx, } } +Status CommandHelpers::canUseTransactions(StringData dbName, StringData cmdName) { + if (cmdName == "count"_sd) { + return {ErrorCodes::OperationNotSupportedInTransaction, + "Cannot run 'count' in a multi-document transaction. Please see " + "http://dochub.mongodb.org/core/transaction-count for a recommended alternative."}; + } + + if (txnCmdWhitelist.find(cmdName) == txnCmdWhitelist.cend() && + !(getTestCommandsEnabled() && + txnCmdForTestingWhitelist.find(cmdName) != txnCmdForTestingWhitelist.cend())) { + return {ErrorCodes::OperationNotSupportedInTransaction, + str::stream() << "Cannot run '" << cmdName << "' in a multi-document transaction."}; + } + + if (dbName == "config"_sd || dbName == "local"_sd || + (dbName == "admin"_sd && txnAdminCommands.find(cmdName) == txnAdminCommands.cend())) { + return {ErrorCodes::OperationNotSupportedInTransaction, + str::stream() << "Cannot run command against the '" << dbName + << "' database in a transaction"}; + } + + return Status::OK(); +} + constexpr StringData CommandHelpers::kHelpFieldName; ////////////////////////////////////////////////////////////// diff --git a/src/mongo/db/commands.h b/src/mongo/db/commands.h index fdc4a24085b..1ecd810911c 100644 --- a/src/mongo/db/commands.h +++ b/src/mongo/db/commands.h @@ -239,6 +239,11 @@ struct CommandHelpers { const Command* command, const OpMsgRequest& request); + /** + * Returns OK if command is allowed to run under a transaction in the given database. + */ + static Status canUseTransactions(StringData dbName, StringData cmdName); + static constexpr StringData kHelpFieldName = "help"_sd; }; diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp index ff715c7f72e..143c62139b5 100644 --- a/src/mongo/db/service_entry_point_common.cpp +++ b/src/mongo/db/service_entry_point_common.cpp @@ -701,6 +701,10 @@ void execCommandDatabase(OperationContext* opCtx, str::stream() << "Invalid database name: '" << dbname << "'", NamespaceString::validDBName(dbname, NamespaceString::DollarInDbNameBehavior::Allow)); + if (sessionOptions.getAutocommit()) { + uassertStatusOK(CommandHelpers::canUseTransactions(dbname, command->getName())); + } + // Session ids are forwarded in requests, so commands that require roundtrips between // servers may result in a deadlock when a server tries to check out a session it is already // using to service an earlier operation in the command's chain. To avoid this, only check diff --git a/src/mongo/db/session.cpp b/src/mongo/db/session.cpp index 03acd548174..337bd40baa5 100644 --- a/src/mongo/db/session.cpp +++ b/src/mongo/db/session.cpp @@ -91,31 +91,6 @@ MONGO_EXPORT_SERVER_PARAMETER(transactionLifetimeLimitSeconds, std::int32_t, 60) namespace { -// The command names that are allowed in a multi-document transaction. -const StringMap txnCmdWhitelist = {{"abortTransaction", 1}, - {"aggregate", 1}, - {"commitTransaction", 1}, - {"delete", 1}, - {"distinct", 1}, - {"doTxn", 1}, - {"find", 1}, - {"findandmodify", 1}, - {"findAndModify", 1}, - {"geoSearch", 1}, - {"getMore", 1}, - {"insert", 1}, - {"killCursors", 1}, - {"prepareTransaction", 1}, - {"update", 1}}; - -// The command names that are allowed in a multi-document transaction only when test commands are -// enabled. -const StringMap txnCmdForTestingWhitelist = {{"dbHash", 1}}; - -// The commands that can be run on the 'admin' database in multi-document transactions. -const StringMap txnAdminCommands = { - {"abortTransaction", 1}, {"commitTransaction", 1}, {"doTxn", 1}, {"prepareTransaction", 1}}; - void fassertOnRepeatedExecution(const LogicalSessionId& lsid, TxnNumber txnNumber, StmtId stmtId, @@ -359,24 +334,6 @@ void Session::beginOrContinueTxn(OperationContext* opCtx, invariant(!opCtx->lockState()->isLocked()); - uassert(ErrorCodes::OperationNotSupportedInTransaction, - "Cannot run 'count' in a multi-document transaction. Please see " - "http://dochub.mongodb.org/core/transaction-count for a recommended alternative.", - !autocommit || cmdName != "count"_sd); - - uassert(ErrorCodes::OperationNotSupportedInTransaction, - str::stream() << "Cannot run '" << cmdName << "' in a multi-document transaction.", - !autocommit || txnCmdWhitelist.find(cmdName) != txnCmdWhitelist.cend() || - (getTestCommandsEnabled() && - txnCmdForTestingWhitelist.find(cmdName) != txnCmdForTestingWhitelist.cend())); - - uassert(ErrorCodes::OperationNotSupportedInTransaction, - str::stream() << "Cannot run command against the '" << dbName - << "' database in a transaction", - !autocommit || (dbName != "config"_sd && dbName != "local"_sd && - (dbName != "admin"_sd || - txnAdminCommands.find(cmdName) != txnAdminCommands.cend()))); - stdx::lock_guard lg(_mutex); _beginOrContinueTxn(lg, txnNumber, autocommit, startTransaction); } -- cgit v1.2.1