diff options
author | Mathias Stearn <mathias@10gen.com> | 2017-03-31 13:19:52 -0400 |
---|---|---|
committer | Mathias Stearn <mathias@10gen.com> | 2017-04-12 16:00:07 -0400 |
commit | 731790d836e22babbd953c68acbe36cdd97694e8 (patch) | |
tree | 5263fd9fdcb22998b9daf4377b4f49ebcb01a049 /src/mongo | |
parent | 1db2940cbc397b3fd8b5250024b85ffa175cccd8 (diff) | |
download | mongo-731790d836e22babbd953c68acbe36cdd97694e8.tar.gz |
SERVER-28507 Centralize ignored fields in command implementations
Diffstat (limited to 'src/mongo')
28 files changed, 202 insertions, 204 deletions
diff --git a/src/mongo/bson/util/bson_check.h b/src/mongo/bson/util/bson_check.h index e6f2e3c98bc..edfff37b5a0 100644 --- a/src/mongo/bson/util/bson_check.h +++ b/src/mongo/bson/util/bson_check.h @@ -29,42 +29,42 @@ #pragma once #include "mongo/base/status.h" +#include "mongo/db/commands.h" #include "mongo/db/jsobj.h" #include "mongo/util/mongoutils/str.h" +#include "mongo/util/string_map.h" namespace mongo { /** - * Confirms that "o" only contains fields whose names are in "begin".."end", + * Confirms that obj only contains field names where allowed(name) returns true, * and that no field name occurs multiple times. * * On failure, returns BadValue and a message naming the unexpected field or DuplicateKey and a * message naming the repeated field. "objectName" is included in the message, for reporting * purposes. */ -template <typename Iter> -Status bsonCheckOnlyHasFields(StringData objectName, - const BSONObj& o, - const Iter& begin, - const Iter& end) { - std::vector<int> occurrences(std::distance(begin, end), 0); - for (BSONObj::iterator iter(o); iter.more();) { +template <typename Condition> +Status bsonCheckOnlyHasFieldsImpl(StringData objectName, + const BSONObj& obj, + const Condition& allowed) { + StringMap<bool> seenFields; + for (BSONObj::iterator iter(obj); iter.more();) { const BSONElement e = iter.next(); - const Iter found = std::find(begin, end, e.fieldNameStringData()); - if (found != end) { - ++occurrences[std::distance(begin, found)]; - } else { + const auto name = e.fieldNameStringData(); + + if (!allowed(name)) { return Status(ErrorCodes::BadValue, str::stream() << "Unexpected field " << e.fieldName() << " in " << objectName); } - } - int i = 0; - for (Iter curr = begin; curr != end; ++curr, ++i) { - if (occurrences[i] > 1) { + + bool& seenBefore = seenFields[name]; + if (!seenBefore) { + seenBefore = true; + } else { return Status(ErrorCodes::DuplicateKey, - str::stream() << "Field " << *curr << " appears " << occurrences[i] - << " times in " + str::stream() << "Field " << name << " appears multiple times in " << objectName); } } @@ -72,14 +72,30 @@ Status bsonCheckOnlyHasFields(StringData objectName, } /** - * Same as above, but operates over an array of string-ish items, "legals", instead - * of "begin".."end". + * Like above but only allows fields from the passed-in container. */ -template <typename StringType, int N> +template <typename Container> Status bsonCheckOnlyHasFields(StringData objectName, - const BSONObj& o, - const StringType (&legals)[N]) { - return bsonCheckOnlyHasFields(objectName, o, &legals[0], legals + N); + const BSONObj& obj, + const Container& allowedFields) { + return bsonCheckOnlyHasFieldsImpl(objectName, obj, [&](StringData name) { + return std::find(std::begin(allowedFields), std::end(allowedFields), name) != + std::end(allowedFields); + }); +} + +/** + * Like above but only allows fields from the passed-in container or are generic command arguments. + */ +template <typename Container> +Status bsonCheckOnlyHasFieldsForCommand(StringData objectName, + const BSONObj& obj, + const Container& allowedFields) { + return bsonCheckOnlyHasFieldsImpl(objectName, obj, [&](StringData name) { + return Command::isGenericArgument(name) || + (std::find(std::begin(allowedFields), std::end(allowedFields), name) != + std::end(allowedFields)); + }); } /** diff --git a/src/mongo/bson/util/bson_check_test.cpp b/src/mongo/bson/util/bson_check_test.cpp index 18365f9ee62..945fa995f11 100644 --- a/src/mongo/bson/util/bson_check_test.cpp +++ b/src/mongo/bson/util/bson_check_test.cpp @@ -26,6 +26,8 @@ * it in the license file. */ +#include <vector> + #include "mongo/bson/util/bson_check.h" #include "mongo/db/jsobj.h" #include "mongo/unittest/unittest.h" @@ -34,9 +36,9 @@ namespace mongo { namespace { TEST(BsonCheck, CheckNothingLegal) { - const char* const* nada = NULL; - ASSERT_OK(bsonCheckOnlyHasFields("", BSONObj(), nada, nada)); - ASSERT_EQUALS(ErrorCodes::BadValue, bsonCheckOnlyHasFields("", BSON("a" << 1), nada, nada)); + ASSERT_OK(bsonCheckOnlyHasFields("", BSONObj(), std::vector<StringData>())); + ASSERT_EQUALS(ErrorCodes::BadValue, + bsonCheckOnlyHasFields("", BSON("a" << 1), std::vector<StringData>())); } const char* const legals[] = {"aField", "anotherField", "thirdField"}; diff --git a/src/mongo/db/auth/user_management_commands_parser.cpp b/src/mongo/db/auth/user_management_commands_parser.cpp index 35c37020c0e..d480e7bc271 100644 --- a/src/mongo/db/auth/user_management_commands_parser.cpp +++ b/src/mongo/db/auth/user_management_commands_parser.cpp @@ -42,6 +42,7 @@ #include "mongo/db/auth/privilege_parser.h" #include "mongo/db/auth/user_document_parser.h" #include "mongo/db/auth/user_name.h" +#include "mongo/db/commands.h" #include "mongo/db/jsobj.h" #include "mongo/platform/unordered_set.h" #include "mongo/util/mongoutils/str.h" @@ -60,7 +61,8 @@ Status _checkNoExtraFields(const BSONObj& cmdObj, // ones. for (BSONObjIterator iter(cmdObj); iter.more(); iter.next()) { StringData fieldName = (*iter).fieldNameStringData(); - if (!validFieldNames.count(fieldName.toString())) { + if (!Command::isGenericArgument(fieldName) && + !validFieldNames.count(fieldName.toString())) { return Status(ErrorCodes::BadValue, mongoutils::str::stream() << "\"" << fieldName << "\" is not " "a valid argument to " @@ -149,8 +151,6 @@ Status parseRolePossessionManipulationCommands(const BSONObj& cmdObj, unordered_set<std::string> validFieldNames; validFieldNames.insert(cmdName.toString()); validFieldNames.insert("roles"); - validFieldNames.insert("writeConcern"); - validFieldNames.insert("maxTimeMS"); Status status = _checkNoExtraFields(cmdObj, cmdName, validFieldNames); if (!status.isOK()) { @@ -191,8 +191,6 @@ Status parseCreateOrUpdateUserCommands(const BSONObj& cmdObj, validFieldNames.insert("digestPassword"); validFieldNames.insert("pwd"); validFieldNames.insert("roles"); - validFieldNames.insert("writeConcern"); - validFieldNames.insert("maxTimeMS"); Status status = _checkNoExtraFields(cmdObj, cmdName, validFieldNames); if (!status.isOK()) { @@ -273,8 +271,6 @@ Status parseAndValidateDropUserCommand(const BSONObj& cmdObj, UserName* parsedUserName) { unordered_set<std::string> validFieldNames; validFieldNames.insert("dropUser"); - validFieldNames.insert("writeConcern"); - validFieldNames.insert("maxTimeMS"); Status status = _checkNoExtraFields(cmdObj, "dropUser", validFieldNames); if (!status.isOK()) { @@ -296,8 +292,6 @@ Status parseFromDatabaseCommand(const BSONObj& cmdObj, std::string command) { unordered_set<std::string> validFieldNames; validFieldNames.insert(command); - validFieldNames.insert("writeConcern"); - validFieldNames.insert("maxTimeMS"); Status status = _checkNoExtraFields(cmdObj, command, validFieldNames); if (!status.isOK()) { @@ -316,7 +310,6 @@ Status parseUsersInfoCommand(const BSONObj& cmdObj, StringData dbname, UsersInfo validFieldNames.insert("usersInfo"); validFieldNames.insert("showPrivileges"); validFieldNames.insert("showCredentials"); - validFieldNames.insert("maxTimeMS"); Status status = _checkNoExtraFields(cmdObj, "usersInfo", validFieldNames); if (!status.isOK()) { @@ -364,7 +357,6 @@ Status parseRolesInfoCommand(const BSONObj& cmdObj, StringData dbname, RolesInfo validFieldNames.insert("rolesInfo"); validFieldNames.insert("showPrivileges"); validFieldNames.insert("showBuiltinRoles"); - validFieldNames.insert("maxTimeMS"); Status status = _checkNoExtraFields(cmdObj, "rolesInfo", validFieldNames); if (!status.isOK()) { @@ -467,8 +459,6 @@ Status parseCreateOrUpdateRoleCommands(const BSONObj& cmdObj, validFieldNames.insert(cmdName.toString()); validFieldNames.insert("privileges"); validFieldNames.insert("roles"); - validFieldNames.insert("writeConcern"); - validFieldNames.insert("maxTimeMS"); Status status = _checkNoExtraFields(cmdObj, cmdName, validFieldNames); if (!status.isOK()) { @@ -527,8 +517,6 @@ Status parseAndValidateRolePrivilegeManipulationCommands(const BSONObj& cmdObj, unordered_set<std::string> validFieldNames; validFieldNames.insert(cmdName.toString()); validFieldNames.insert("privileges"); - validFieldNames.insert("writeConcern"); - validFieldNames.insert("maxTimeMS"); Status status = _checkNoExtraFields(cmdObj, cmdName, validFieldNames); if (!status.isOK()) { @@ -569,8 +557,6 @@ Status parseDropRoleCommand(const BSONObj& cmdObj, RoleName* parsedRoleName) { unordered_set<std::string> validFieldNames; validFieldNames.insert("dropRole"); - validFieldNames.insert("writeConcern"); - validFieldNames.insert("maxTimeMS"); Status status = _checkNoExtraFields(cmdObj, "dropRole", validFieldNames); if (!status.isOK()) { @@ -599,8 +585,6 @@ Status parseMergeAuthzCollectionsCommand(const BSONObj& cmdObj, validFieldNames.insert("tempRolesCollection"); validFieldNames.insert("db"); validFieldNames.insert("drop"); - validFieldNames.insert("writeConcern"); - validFieldNames.insert("maxTimeMS"); Status status = _checkNoExtraFields(cmdObj, "_mergeAuthzCollections", validFieldNames); if (!status.isOK()) { @@ -648,8 +632,6 @@ Status parseAuthSchemaUpgradeCommand(const BSONObj& cmdObj, validFieldNames.insert("authSchemaUpgrade"); validFieldNames.insert("maxSteps"); validFieldNames.insert("upgradeShards"); - validFieldNames.insert("writeConcern"); - validFieldNames.insert("maxTimeMS"); Status status = _checkNoExtraFields(cmdObj, "authSchemaUpgrade", validFieldNames); if (!status.isOK()) { diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp index 93abe72214c..5e6ec1af9e4 100644 --- a/src/mongo/db/catalog/coll_mod.cpp +++ b/src/mongo/db/catalog/coll_mod.cpp @@ -63,22 +63,20 @@ struct CollModRequest { StatusWith<CollModRequest> parseCollModRequest(OperationContext* opCtx, const NamespaceString& nss, Collection* coll, - const BSONObj& cmdObj) { + const BSONObj& cmdObj, + BSONObjBuilder* oplogEntryBuilder) { bool isView = !coll; CollModRequest cmr; BSONForEach(e, cmdObj) { - if (str::equals("collMod", e.fieldName())) { + const auto fieldName = e.fieldNameStringData(); + if (Command::isGenericArgument(fieldName)) { + continue; // Don't add to oplog builder. + } else if (fieldName == "collMod") { // no-op - } else if (str::startsWith(e.fieldName(), "$")) { - // no-op ignore top-level fields prefixed with $. They are for the command processor - } else if (QueryRequest::cmdOptionMaxTimeMS == e.fieldNameStringData()) { - // no-op - } else if (str::equals("shardVersion", e.fieldName())) { - // no-op - } else if (str::equals("index", e.fieldName()) && !isView) { + } else if (fieldName == "index" && !isView) { BSONObj indexObj = e.Obj(); StringData indexName; BSONObj keyPattern; @@ -158,25 +156,25 @@ StatusWith<CollModRequest> parseCollModRequest(OperationContext* opCtx, "existing expireAfterSeconds field is not a number"); } - } else if (str::equals("validator", e.fieldName()) && !isView) { + } else if (fieldName == "validator" && !isView) { auto statusW = coll->parseValidator(e.Obj()); if (!statusW.isOK()) return statusW.getStatus(); cmr.collValidator = e; - } else if (str::equals("validationLevel", e.fieldName()) && !isView) { + } else if (fieldName == "validationLevel" && !isView) { auto statusW = coll->parseValidationLevel(e.String()); if (!statusW.isOK()) return statusW.getStatus(); cmr.collValidationLevel = e.String(); - } else if (str::equals("validationAction", e.fieldName()) && !isView) { + } else if (fieldName == "validationAction" && !isView) { auto statusW = coll->parseValidationAction(e.String()); if (!statusW.isOK()) statusW.getStatus(); cmr.collValidationAction = e.String(); - } else if (str::equals("pipeline", e.fieldName())) { + } else if (fieldName == "pipeline") { if (!isView) { return Status(ErrorCodes::InvalidOptions, "'pipeline' option only supported on a view"); @@ -185,7 +183,7 @@ StatusWith<CollModRequest> parseCollModRequest(OperationContext* opCtx, return Status(ErrorCodes::InvalidOptions, "not a valid aggregation pipeline"); } cmr.viewPipeLine = e; - } else if (str::equals("viewOn", e.fieldName())) { + } else if (fieldName == "viewOn") { if (!isView) { return Status(ErrorCodes::InvalidOptions, "'viewOn' option only supported on a view"); @@ -195,23 +193,24 @@ StatusWith<CollModRequest> parseCollModRequest(OperationContext* opCtx, } cmr.viewOn = e.str(); } else { - const StringData name = e.fieldNameStringData(); if (isView) { return Status(ErrorCodes::InvalidOptions, - str::stream() << "option not supported on a view: " << name); + str::stream() << "option not supported on a view: " << fieldName); } // As of SERVER-17312 we only support these two options. When SERVER-17320 is // resolved this will need to be enhanced to handle other options. typedef CollectionOptions CO; - if (name == "usePowerOf2Sizes") + if (fieldName == "usePowerOf2Sizes") cmr.usePowerOf2Sizes = e; - else if (name == "noPadding") + else if (fieldName == "noPadding") cmr.noPadding = e; else return Status(ErrorCodes::InvalidOptions, - str::stream() << "unknown option to collMod: " << name); + str::stream() << "unknown option to collMod: " << fieldName); } + + oplogEntryBuilder->append(e); } return {std::move(cmr)}; @@ -256,7 +255,8 @@ Status collMod(OperationContext* opCtx, << nss.ns()); } - auto statusW = parseCollModRequest(opCtx, nss, coll, cmdObj); + BSONObjBuilder oplogEntryBuilder; + auto statusW = parseCollModRequest(opCtx, nss, coll, cmdObj, &oplogEntryBuilder); if (!statusW.isOK()) { return statusW.getStatus(); } @@ -347,7 +347,7 @@ Status collMod(OperationContext* opCtx, // Only observe non-view collMods, as view operations are observed as operations on the // system.views collection. - getGlobalServiceContext()->getOpObserver()->onCollMod(opCtx, nss, cmdObj); + getGlobalServiceContext()->getOpObserver()->onCollMod(opCtx, nss, oplogEntryBuilder.obj()); } wunit.commit(); diff --git a/src/mongo/db/catalog/coll_mod.h b/src/mongo/db/catalog/coll_mod.h index 9442fc7d82e..2ae6e0cb33b 100644 --- a/src/mongo/db/catalog/coll_mod.h +++ b/src/mongo/db/catalog/coll_mod.h @@ -38,11 +38,6 @@ class OperationContext; struct CollModRequest; -StatusWith<CollModRequest> parseCollModRequest(OperationContext* opCtx, - const NamespaceString& nss, - Collection* coll, - const BSONObj& cmdObj); - /** * Performs the collection modification described in "cmdObj" on the collection "ns". */ diff --git a/src/mongo/db/catalog/collection_options.cpp b/src/mongo/db/catalog/collection_options.cpp index 8825bdca62b..72431637574 100644 --- a/src/mongo/db/catalog/collection_options.cpp +++ b/src/mongo/db/catalog/collection_options.cpp @@ -31,6 +31,7 @@ #include "mongo/db/catalog/collection_options.h" #include "mongo/base/string_data.h" +#include "mongo/db/commands.h" #include "mongo/db/server_parameters.h" #include "mongo/util/mongoutils/str.h" @@ -91,13 +92,6 @@ Status checkStorageEngineOptions(const BSONElement& elem) { return Status::OK(); } -// These are collection creation options which are handled elsewhere. If we encounter a field which -// CollectionOptions doesn't know about, parsing the options should fail unless we find the field -// name in this whitelist. -const std::set<StringData> collectionOptionsWhitelist{ - "maxTimeMS"_sd, "writeConcern"_sd, -}; - } // namespace bool CollectionOptions::isView() const { @@ -249,8 +243,7 @@ Status CollectionOptions::parse(const BSONObj& options, ParseKind kind) { } pipeline = e.Obj().getOwned(); - } else if (!createdOn24OrEarlier && - collectionOptionsWhitelist.find(fieldName) == collectionOptionsWhitelist.end()) { + } else if (!createdOn24OrEarlier && !Command::isGenericArgument(fieldName)) { return Status(ErrorCodes::InvalidOptions, str::stream() << "The field '" << fieldName << "' is not a valid collection option. Options: " diff --git a/src/mongo/db/catalog/create_collection.cpp b/src/mongo/db/catalog/create_collection.cpp index f8a39c20350..19e32cb9b96 100644 --- a/src/mongo/db/catalog/create_collection.cpp +++ b/src/mongo/db/catalog/create_collection.cpp @@ -63,7 +63,9 @@ Status createCollection(OperationContext* opCtx, // Build options object from remaining cmdObj elements. BSONObjBuilder optionsBuilder; while (it.more()) { - optionsBuilder.append(it.next()); + const auto elem = it.next(); + if (!Command::isGenericArgument(elem.fieldNameStringData())) + optionsBuilder.append(elem); } BSONObj options = optionsBuilder.obj(); diff --git a/src/mongo/db/commands.h b/src/mongo/db/commands.h index 0c9ffca316b..02e9f125e95 100644 --- a/src/mongo/db/commands.h +++ b/src/mongo/db/commands.h @@ -472,6 +472,22 @@ public: const std::string& dbname, const BSONObj& cmdObj); + /** + * Returns true if the provided argument is one that is handled by the command processing layer + * and should generally be ignored by individual command implementations. In particular, + * commands that fail on unrecognized arguments must not fail for any of these. + */ + static bool isGenericArgument(StringData arg) { + // Not including "help" since we don't pass help requests through to the command parser. + // If that changes, it should be added. + return arg == "maxTimeMS" || // + arg == "$queryOptions" || // + arg == "shardVersion" || // + arg == "writeConcern" || // + arg == "readConcern" || // + false; // These comments tell clang-format to keep this line-oriented. + } + private: /** * Checks if the given client is authorized to run this command on database "dbname" diff --git a/src/mongo/db/commands/create_indexes.cpp b/src/mongo/db/commands/create_indexes.cpp index 189ec8e78f1..8dfca9ed9cc 100644 --- a/src/mongo/db/commands/create_indexes.cpp +++ b/src/mongo/db/commands/create_indexes.cpp @@ -66,9 +66,6 @@ namespace { const StringData kIndexesFieldName = "indexes"_sd; const StringData kCommandName = "createIndexes"_sd; -const StringData kWriteConcern = "writeConcern"_sd; -const StringData kMaxTimeMS = "maxTimeMS"_sd; -const StringData kShardVersion = "shardVersion"_sd; /** * Parses the index specifications from 'cmdObj', validates them, and returns equivalent index @@ -125,8 +122,8 @@ StatusWith<std::vector<BSONObj>> parseAndValidateIndexSpecs( } hasIndexesField = true; - } else if (kCommandName == cmdElemFieldName || kWriteConcern == cmdElemFieldName || - kMaxTimeMS == cmdElemFieldName || kShardVersion == cmdElemFieldName) { + } else if (kCommandName == cmdElemFieldName || + Command::isGenericArgument(cmdElemFieldName)) { continue; } else { return {ErrorCodes::BadValue, diff --git a/src/mongo/db/commands/current_op.cpp b/src/mongo/db/commands/current_op.cpp index c5f05e314ae..ebb4c15d207 100644 --- a/src/mongo/db/commands/current_op.cpp +++ b/src/mongo/db/commands/current_op.cpp @@ -103,9 +103,12 @@ public: i.next(); // skip {currentOp: 1} which is required to be the first element while (i.more()) { BSONElement e = i.next(); - if (str::equals("$all", e.fieldName())) { + const auto fieldName = e.fieldNameStringData(); + if (fieldName == "$all") { continue; - } else if (str::equals("$ownOps", e.fieldName())) { + } else if (fieldName == "$ownOps") { + continue; + } else if (Command::isGenericArgument(fieldName)) { continue; } diff --git a/src/mongo/db/commands/feature_compatibility_version_command_parser.cpp b/src/mongo/db/commands/feature_compatibility_version_command_parser.cpp index e8536e88db2..551ac40a758 100644 --- a/src/mongo/db/commands/feature_compatibility_version_command_parser.cpp +++ b/src/mongo/db/commands/feature_compatibility_version_command_parser.cpp @@ -32,6 +32,7 @@ #include "mongo/base/status_with.h" #include "mongo/bson/bsonobj.h" +#include "mongo/db/commands.h" #include "mongo/db/query/query_request.h" #include "mongo/util/version.h" @@ -62,8 +63,8 @@ StatusWith<std::string> FeatureCompatibilityVersionCommandParser::extractVersion // Ensure that the command does not contain any unrecognized parameters for (const auto& cmdElem : cmdObj) { - if (cmdElem.fieldNameStringData() == commandName || - cmdElem.fieldNameStringData() == QueryRequest::cmdOptionMaxTimeMS) { + const auto fieldName = cmdElem.fieldNameStringData(); + if (fieldName == commandName || Command::isGenericArgument(fieldName)) { continue; } diff --git a/src/mongo/db/commands/get_last_error.cpp b/src/mongo/db/commands/get_last_error.cpp index 0340a01139b..c5d8ff9b519 100644 --- a/src/mongo/db/commands/get_last_error.cpp +++ b/src/mongo/db/commands/get_last_error.cpp @@ -203,9 +203,17 @@ public: } } - BSONObj writeConcernDoc = cmdObj; + BSONObj writeConcernDoc = ([&] { + BSONObjBuilder bob; + for (auto&& elem : cmdObj) { + if (!Command::isGenericArgument(elem.fieldNameStringData())) + bob.append(elem); + } + return bob.obj(); + }()); + // Use the default options if we have no gle options aside from wOpTime/wElectionId - const int nFields = cmdObj.nFields(); + const int nFields = writeConcernDoc.nFields(); bool useDefaultGLEOptions = (nFields == 1) || (nFields == 2 && lastOpTimePresent) || (nFields == 3 && lastOpTimePresent && electionIdPresent); diff --git a/src/mongo/db/commands/parameters.cpp b/src/mongo/db/commands/parameters.cpp index f2e10dac2b5..c908edb3390 100644 --- a/src/mongo/db/commands/parameters.cpp +++ b/src/mongo/db/commands/parameters.cpp @@ -164,6 +164,8 @@ public: while (parameterCheckIterator.more()) { BSONElement parameter = parameterCheckIterator.next(); std::string parameterName = parameter.fieldName(); + if (Command::isGenericArgument(parameterName)) + continue; ServerParameter::Map::const_iterator foundParameter = parameterMap.find(parameterName); diff --git a/src/mongo/db/ops/write_ops_parsers.cpp b/src/mongo/db/ops/write_ops_parsers.cpp index 1cbe9dea328..f45af99d00e 100644 --- a/src/mongo/db/ops/write_ops_parsers.cpp +++ b/src/mongo/db/ops/write_ops_parsers.cpp @@ -33,6 +33,7 @@ #include "mongo/bson/util/bson_check.h" #include "mongo/client/dbclientinterface.h" #include "mongo/db/catalog/document_validation.h" +#include "mongo/db/commands.h" #include "mongo/db/dbmessage.h" #include "mongo/util/assert_util.h" #include "mongo/util/mongoutils/str.h" @@ -106,15 +107,11 @@ void parseWriteCommand(StringData dbName, } else if (fieldName == uniqueFieldName) { haveUniqueField = true; *uniqueField = field; - } else if (fieldName[0] != '$') { - std::initializer_list<StringData> ignoredFields = { - "writeConcern", "maxTimeMS", "shardVersion"}; - uassert(ErrorCodes::FailedToParse, - str::stream() << "Unknown option to " << cmd.firstElementFieldName() - << " command: " - << fieldName, - std::find(ignoredFields.begin(), ignoredFields.end(), fieldName) != - ignoredFields.end()); + } else if (!Command::isGenericArgument(fieldName)) { + uasserted(ErrorCodes::FailedToParse, + str::stream() << "Unknown option to " << cmd.firstElementFieldName() + << " command: " + << fieldName); } } diff --git a/src/mongo/db/pipeline/aggregation_request.cpp b/src/mongo/db/pipeline/aggregation_request.cpp index 43adbb213ad..163c4f288c1 100644 --- a/src/mongo/db/pipeline/aggregation_request.cpp +++ b/src/mongo/db/pipeline/aggregation_request.cpp @@ -83,8 +83,7 @@ StatusWith<AggregationRequest> AggregationRequest::parseFromBSON( AggregationRequest request(std::move(nss), std::move(pipeline)); - const std::initializer_list<StringData> optionsParsedElseWhere = { - WriteConcernOptions::kWriteConcernField, kPipelineName, kCommandName}; + const std::initializer_list<StringData> optionsParsedElseWhere = {kPipelineName, kCommandName}; bool hasCursorElem = false; bool hasExplainElem = false; @@ -93,24 +92,14 @@ StatusWith<AggregationRequest> AggregationRequest::parseFromBSON( for (auto&& elem : cmdObj) { auto fieldName = elem.fieldNameStringData(); - // $queryOptions is the exception to our '$' filter. We expect this field to be validated - // elsewhere. if (QueryRequest::kUnwrappedReadPrefField == fieldName) { + // We expect this field to be validated elsewhere. request.setUnwrappedReadPref(elem.embeddedObject()); - } - - // Ignore top-level fields prefixed with $. They are for the command processor, not us. - if (fieldName[0] == '$') { - continue; - } - - // Ignore options that are parsed elsewhere. - if (std::find(optionsParsedElseWhere.begin(), optionsParsedElseWhere.end(), fieldName) != - optionsParsedElseWhere.end()) { - continue; - } - - if (kCursorName == fieldName) { + } else if (std::find(optionsParsedElseWhere.begin(), + optionsParsedElseWhere.end(), + fieldName) != optionsParsedElseWhere.end()) { + // Ignore options that are parsed elsewhere. + } else if (kCursorName == fieldName) { long long batchSize; auto status = CursorRequest::parseCommandCursorOptions(cmdObj, kDefaultBatchSize, &batchSize); @@ -192,7 +181,7 @@ StatusWith<AggregationRequest> AggregationRequest::parseFromBSON( request.setAllowDiskUse(elem.Bool()); } else if (bypassDocumentValidationCommandOption() == fieldName) { request.setBypassDocumentValidation(elem.trueValue()); - } else { + } else if (!Command::isGenericArgument(fieldName)) { return {ErrorCodes::FailedToParse, str::stream() << "unrecognized field '" << elem.fieldName() << "'"}; } diff --git a/src/mongo/db/pipeline/aggregation_request_test.cpp b/src/mongo/db/pipeline/aggregation_request_test.cpp index ba036c71bf4..cb68a8af79d 100644 --- a/src/mongo/db/pipeline/aggregation_request_test.cpp +++ b/src/mongo/db/pipeline/aggregation_request_test.cpp @@ -368,10 +368,10 @@ TEST(AggregationRequestTest, ShouldRejectExplainExecStatsVerbosityWithWriteConce // Ignore fields parsed elsewhere. // -TEST(AggregationRequestTest, ShouldIgnoreFieldsPrefixedWithDollar) { +TEST(AggregationRequestTest, ShouldIgnoreQueryOptions) { NamespaceString nss("a.collection"); const BSONObj inputBson = - fromjson("{pipeline: [{$match: {a: 'abc'}}], cursor: {}, $unknown: 1}"); + fromjson("{pipeline: [{$match: {a: 'abc'}}], cursor: {}, $queryOptions: {}}"); ASSERT_OK(AggregationRequest::parseFromBSON(nss, inputBson).getStatus()); } diff --git a/src/mongo/db/query/getmore_request.cpp b/src/mongo/db/query/getmore_request.cpp index 48f68d338ee..6c738c4da67 100644 --- a/src/mongo/db/query/getmore_request.cpp +++ b/src/mongo/db/query/getmore_request.cpp @@ -34,6 +34,7 @@ #include <boost/optional.hpp> +#include "mongo/db/commands.h" #include "mongo/db/namespace_string.h" #include "mongo/db/repl/bson_extract_optime.h" #include "mongo/util/assert_util.h" @@ -112,15 +113,15 @@ StatusWith<GetMoreRequest> GetMoreRequest::parseFromBSON(const std::string& dbna boost::optional<repl::OpTime> lastKnownCommittedOpTime; for (BSONElement el : cmdObj) { - const char* fieldName = el.fieldName(); - if (str::equals(fieldName, kGetMoreCommandName)) { + const auto fieldName = el.fieldNameStringData(); + if (fieldName == kGetMoreCommandName) { if (el.type() != BSONType::NumberLong) { return {ErrorCodes::TypeMismatch, str::stream() << "Field 'getMore' must be of type long in: " << cmdObj}; } cursorid = el.Long(); - } else if (str::equals(fieldName, kCollectionField)) { + } else if (fieldName == kCollectionField) { if (el.type() != BSONType::String) { return {ErrorCodes::TypeMismatch, str::stream() << "Field 'collection' must be of type string in: " @@ -128,14 +129,14 @@ StatusWith<GetMoreRequest> GetMoreRequest::parseFromBSON(const std::string& dbna } nss = parseNs(dbname, cmdObj); - } else if (str::equals(fieldName, kBatchSizeField)) { + } else if (fieldName == kBatchSizeField) { if (!el.isNumber()) { return {ErrorCodes::TypeMismatch, str::stream() << "Field 'batchSize' must be a number in: " << cmdObj}; } batchSize = el.numberLong(); - } else if (str::equals(fieldName, kAwaitDataTimeoutField)) { + } else if (fieldName == kAwaitDataTimeoutField) { auto maxAwaitDataTime = QueryRequest::parseMaxTimeMS(el); if (!maxAwaitDataTime.isOK()) { return maxAwaitDataTime.getStatus(); @@ -144,20 +145,20 @@ StatusWith<GetMoreRequest> GetMoreRequest::parseFromBSON(const std::string& dbna if (maxAwaitDataTime.getValue()) { awaitDataTimeout = Milliseconds(maxAwaitDataTime.getValue()); } - } else if (str::equals(fieldName, kTermField)) { + } else if (fieldName == kTermField) { if (el.type() != BSONType::NumberLong) { return {ErrorCodes::TypeMismatch, str::stream() << "Field 'term' must be of type NumberLong in: " << cmdObj}; } term = el.Long(); - } else if (str::equals(fieldName, kLastKnownCommittedOpTimeField)) { + } else if (fieldName == kLastKnownCommittedOpTimeField) { repl::OpTime ot; Status status = bsonExtractOpTimeField(el.wrap(), kLastKnownCommittedOpTimeField, &ot); if (!status.isOK()) { return status; } lastKnownCommittedOpTime = ot; - } else if (!str::startsWith(fieldName, "$")) { + } else if (!Command::isGenericArgument(fieldName)) { return {ErrorCodes::FailedToParse, str::stream() << "Failed to parse: " << cmdObj << ". " << "Unrecognized field '" diff --git a/src/mongo/db/query/getmore_request_test.cpp b/src/mongo/db/query/getmore_request_test.cpp index 828a434658d..f1be6a613bf 100644 --- a/src/mongo/db/query/getmore_request_test.cpp +++ b/src/mongo/db/query/getmore_request_test.cpp @@ -168,12 +168,12 @@ TEST(GetMoreRequestTest, parseFromBSONBatchSizeProvided) { ASSERT_EQUALS(200, *result.getValue().batchSize); } -TEST(GetMoreRequestTest, parseFromBSONIgnoreDollarPrefixedFields) { +TEST(GetMoreRequestTest, parseFromBSONIgnoreQueryOptions) { StatusWith<GetMoreRequest> result = GetMoreRequest::parseFromBSON("db", BSON("getMore" << CursorId(123) << "collection" << "coll" - << "$foo" + << "$queryOptions" << "bar")); ASSERT_OK(result.getStatus()); ASSERT_EQUALS("db.coll", result.getValue().nss.toString()); diff --git a/src/mongo/db/query/query_request.cpp b/src/mongo/db/query/query_request.cpp index 467f57f41f0..0b6d6e4c5c4 100644 --- a/src/mongo/db/query/query_request.cpp +++ b/src/mongo/db/query/query_request.cpp @@ -34,6 +34,7 @@ #include "mongo/base/status_with.h" #include "mongo/bson/simple_bsonobj_comparator.h" #include "mongo/client/dbclientinterface.h" +#include "mongo/db/commands.h" #include "mongo/db/dbmessage.h" #include "mongo/db/namespace_string.h" #include "mongo/db/repl/read_concern_args.h" @@ -121,34 +122,34 @@ StatusWith<unique_ptr<QueryRequest>> QueryRequest::makeFromFindCommand(Namespace BSONObjIterator it(cmdObj); while (it.more()) { BSONElement el = it.next(); - const char* fieldName = el.fieldName(); - if (str::equals(fieldName, kFindCommandName)) { + const auto fieldName = el.fieldNameStringData(); + if (fieldName == kFindCommandName) { Status status = checkFieldType(el, String); if (!status.isOK()) { return status; } - } else if (str::equals(fieldName, kFilterField)) { + } else if (fieldName == kFilterField) { Status status = checkFieldType(el, Object); if (!status.isOK()) { return status; } qr->_filter = el.Obj().getOwned(); - } else if (str::equals(fieldName, kProjectionField)) { + } else if (fieldName == kProjectionField) { Status status = checkFieldType(el, Object); if (!status.isOK()) { return status; } qr->_proj = el.Obj().getOwned(); - } else if (str::equals(fieldName, kSortField)) { + } else if (fieldName == kSortField) { Status status = checkFieldType(el, Object); if (!status.isOK()) { return status; } qr->_sort = el.Obj().getOwned(); - } else if (str::equals(fieldName, kHintField)) { + } else if (fieldName == kHintField) { BSONObj hintObj; if (Object == el.type()) { hintObj = cmdObj["hint"].Obj().getOwned(); @@ -160,7 +161,7 @@ StatusWith<unique_ptr<QueryRequest>> QueryRequest::makeFromFindCommand(Namespace } qr->_hint = hintObj; - } else if (str::equals(fieldName, repl::ReadConcernArgs::kReadConcernFieldName.c_str())) { + } else if (fieldName == repl::ReadConcernArgs::kReadConcernFieldName) { // Read concern parsing is handled elsewhere, but we store a copy here. Status status = checkFieldType(el, Object); if (!status.isOK()) { @@ -168,7 +169,7 @@ StatusWith<unique_ptr<QueryRequest>> QueryRequest::makeFromFindCommand(Namespace } qr->_readConcern = el.Obj().getOwned(); - } else if (str::equals(fieldName, QueryRequest::kUnwrappedReadPrefField.c_str())) { + } else if (fieldName == QueryRequest::kUnwrappedReadPrefField) { // Read preference parsing is handled elsewhere, but we store a copy here. Status status = checkFieldType(el, Object); if (!status.isOK()) { @@ -176,7 +177,7 @@ StatusWith<unique_ptr<QueryRequest>> QueryRequest::makeFromFindCommand(Namespace } qr->setUnwrappedReadPref(el.Obj()); - } else if (str::equals(fieldName, kCollationField)) { + } else if (fieldName == kCollationField) { // Collation parsing is handled elsewhere, but we store a copy here. Status status = checkFieldType(el, Object); if (!status.isOK()) { @@ -184,7 +185,7 @@ StatusWith<unique_ptr<QueryRequest>> QueryRequest::makeFromFindCommand(Namespace } qr->_collation = el.Obj().getOwned(); - } else if (str::equals(fieldName, kSkipField)) { + } else if (fieldName == kSkipField) { if (!el.isNumber()) { str::stream ss; ss << "Failed to parse: " << cmdObj.toString() << ". " @@ -198,7 +199,7 @@ StatusWith<unique_ptr<QueryRequest>> QueryRequest::makeFromFindCommand(Namespace if (skip) { qr->_skip = skip; } - } else if (str::equals(fieldName, kLimitField)) { + } else if (fieldName == kLimitField) { if (!el.isNumber()) { str::stream ss; ss << "Failed to parse: " << cmdObj.toString() << ". " @@ -212,7 +213,7 @@ StatusWith<unique_ptr<QueryRequest>> QueryRequest::makeFromFindCommand(Namespace if (limit) { qr->_limit = limit; } - } else if (str::equals(fieldName, kBatchSizeField)) { + } else if (fieldName == kBatchSizeField) { if (!el.isNumber()) { str::stream ss; ss << "Failed to parse: " << cmdObj.toString() << ". " @@ -221,7 +222,7 @@ StatusWith<unique_ptr<QueryRequest>> QueryRequest::makeFromFindCommand(Namespace } qr->_batchSize = el.numberLong(); - } else if (str::equals(fieldName, kNToReturnField)) { + } else if (fieldName == kNToReturnField) { if (!el.isNumber()) { str::stream ss; ss << "Failed to parse: " << cmdObj.toString() << ". " @@ -230,21 +231,21 @@ StatusWith<unique_ptr<QueryRequest>> QueryRequest::makeFromFindCommand(Namespace } qr->_ntoreturn = el.numberLong(); - } else if (str::equals(fieldName, kSingleBatchField)) { + } else if (fieldName == kSingleBatchField) { Status status = checkFieldType(el, Bool); if (!status.isOK()) { return status; } qr->_wantMore = !el.boolean(); - } else if (str::equals(fieldName, kCommentField)) { + } else if (fieldName == kCommentField) { Status status = checkFieldType(el, String); if (!status.isOK()) { return status; } qr->_comment = el.str(); - } else if (str::equals(fieldName, kMaxScanField)) { + } else if (fieldName == kMaxScanField) { if (!el.isNumber()) { str::stream ss; ss << "Failed to parse: " << cmdObj.toString() << ". " @@ -253,84 +254,84 @@ StatusWith<unique_ptr<QueryRequest>> QueryRequest::makeFromFindCommand(Namespace } qr->_maxScan = el.numberInt(); - } else if (str::equals(fieldName, cmdOptionMaxTimeMS)) { + } else if (fieldName == cmdOptionMaxTimeMS) { StatusWith<int> maxTimeMS = parseMaxTimeMS(el); if (!maxTimeMS.isOK()) { return maxTimeMS.getStatus(); } qr->_maxTimeMS = maxTimeMS.getValue(); - } else if (str::equals(fieldName, kMinField)) { + } else if (fieldName == kMinField) { Status status = checkFieldType(el, Object); if (!status.isOK()) { return status; } qr->_min = el.Obj().getOwned(); - } else if (str::equals(fieldName, kMaxField)) { + } else if (fieldName == kMaxField) { Status status = checkFieldType(el, Object); if (!status.isOK()) { return status; } qr->_max = el.Obj().getOwned(); - } else if (str::equals(fieldName, kReturnKeyField)) { + } else if (fieldName == kReturnKeyField) { Status status = checkFieldType(el, Bool); if (!status.isOK()) { return status; } qr->_returnKey = el.boolean(); - } else if (str::equals(fieldName, kShowRecordIdField)) { + } else if (fieldName == kShowRecordIdField) { Status status = checkFieldType(el, Bool); if (!status.isOK()) { return status; } qr->_showRecordId = el.boolean(); - } else if (str::equals(fieldName, kSnapshotField)) { + } else if (fieldName == kSnapshotField) { Status status = checkFieldType(el, Bool); if (!status.isOK()) { return status; } qr->_snapshot = el.boolean(); - } else if (str::equals(fieldName, kTailableField)) { + } else if (fieldName == kTailableField) { Status status = checkFieldType(el, Bool); if (!status.isOK()) { return status; } qr->_tailable = el.boolean(); - } else if (str::equals(fieldName, kOplogReplayField)) { + } else if (fieldName == kOplogReplayField) { Status status = checkFieldType(el, Bool); if (!status.isOK()) { return status; } qr->_oplogReplay = el.boolean(); - } else if (str::equals(fieldName, kNoCursorTimeoutField)) { + } else if (fieldName == kNoCursorTimeoutField) { Status status = checkFieldType(el, Bool); if (!status.isOK()) { return status; } qr->_noCursorTimeout = el.boolean(); - } else if (str::equals(fieldName, kAwaitDataField)) { + } else if (fieldName == kAwaitDataField) { Status status = checkFieldType(el, Bool); if (!status.isOK()) { return status; } qr->_awaitData = el.boolean(); - } else if (str::equals(fieldName, kPartialResultsField)) { + } else if (fieldName == kPartialResultsField) { Status status = checkFieldType(el, Bool); if (!status.isOK()) { return status; } qr->_allowPartialResults = el.boolean(); - } else if (str::equals(fieldName, kOptionsField)) { + } else if (fieldName == kOptionsField) { // 3.0.x versions of the shell may generate an explain of a find command with an // 'options' field. We accept this only if the 'options' field is empty so that // the shell's explain implementation is forwards compatible. @@ -353,15 +354,15 @@ StatusWith<unique_ptr<QueryRequest>> QueryRequest::makeFromFindCommand(Namespace str::stream() << "Failed to parse options: " << optionsObj.toString() << ". You may need to update your shell or driver."); } - } else if (str::equals(fieldName, kShardVersionField)) { + } else if (fieldName == kShardVersionField) { // Shard version parsing is handled elsewhere. - } else if (str::equals(fieldName, kTermField)) { + } else if (fieldName == kTermField) { Status status = checkFieldType(el, NumberLong); if (!status.isOK()) { return status; } qr->_replicationTerm = el._numberLong(); - } else if (!str::startsWith(fieldName, '$')) { + } else if (!Command::isGenericArgument(fieldName)) { return Status(ErrorCodes::FailedToParse, str::stream() << "Failed to parse: " << cmdObj.toString() << ". " << "Unrecognized field '" diff --git a/src/mongo/db/repl/handshake_args.cpp b/src/mongo/db/repl/handshake_args.cpp index 2ceae3df86e..d8ebf3c0fc7 100644 --- a/src/mongo/db/repl/handshake_args.cpp +++ b/src/mongo/db/repl/handshake_args.cpp @@ -53,7 +53,8 @@ const std::string kLegalHandshakeFieldNames[] = { HandshakeArgs::HandshakeArgs() : _hasRid(false), _hasMemberId(false), _rid(OID()), _memberId(-1) {} Status HandshakeArgs::initialize(const BSONObj& argsObj) { - Status status = bsonCheckOnlyHasFields("HandshakeArgs", argsObj, kLegalHandshakeFieldNames); + Status status = + bsonCheckOnlyHasFieldsForCommand("HandshakeArgs", argsObj, kLegalHandshakeFieldNames); if (!status.isOK()) return status; diff --git a/src/mongo/db/repl/old_update_position_args.cpp b/src/mongo/db/repl/old_update_position_args.cpp index 92ffd74db79..e3c2f295d26 100644 --- a/src/mongo/db/repl/old_update_position_args.cpp +++ b/src/mongo/db/repl/old_update_position_args.cpp @@ -70,8 +70,8 @@ const std::string kLegalUpdateInfoFieldNames[] = { } // namespace Status OldUpdatePositionArgs::initialize(const BSONObj& argsObj) { - Status status = - bsonCheckOnlyHasFields("OldUpdatePositionArgs", argsObj, kLegalUpdatePositionFieldNames); + Status status = bsonCheckOnlyHasFieldsForCommand( + "OldUpdatePositionArgs", argsObj, kLegalUpdatePositionFieldNames); if (!status.isOK()) return status; diff --git a/src/mongo/db/repl/repl_set_heartbeat_args.cpp b/src/mongo/db/repl/repl_set_heartbeat_args.cpp index babca5a0dfa..db88f7e47b6 100644 --- a/src/mongo/db/repl/repl_set_heartbeat_args.cpp +++ b/src/mongo/db/repl/repl_set_heartbeat_args.cpp @@ -70,8 +70,8 @@ ReplSetHeartbeatArgs::ReplSetHeartbeatArgs() _senderHost(HostAndPort()) {} Status ReplSetHeartbeatArgs::initialize(const BSONObj& argsObj) { - Status status = - bsonCheckOnlyHasFields("ReplSetHeartbeatArgs", argsObj, kLegalHeartbeatFieldNames); + Status status = bsonCheckOnlyHasFieldsForCommand( + "ReplSetHeartbeatArgs", argsObj, kLegalHeartbeatFieldNames); if (!status.isOK()) return status; diff --git a/src/mongo/db/repl/repl_set_heartbeat_args_v1.cpp b/src/mongo/db/repl/repl_set_heartbeat_args_v1.cpp index af6b4d69eed..39bab8cdda7 100644 --- a/src/mongo/db/repl/repl_set_heartbeat_args_v1.cpp +++ b/src/mongo/db/repl/repl_set_heartbeat_args_v1.cpp @@ -57,8 +57,8 @@ const std::string kLegalHeartbeatFieldNames[] = {kCheckEmptyFieldName, } // namespace Status ReplSetHeartbeatArgsV1::initialize(const BSONObj& argsObj) { - Status status = - bsonCheckOnlyHasFields("ReplSetHeartbeatArgs", argsObj, kLegalHeartbeatFieldNames); + Status status = bsonCheckOnlyHasFieldsForCommand( + "ReplSetHeartbeatArgs", argsObj, kLegalHeartbeatFieldNames); if (!status.isOK()) return status; diff --git a/src/mongo/db/repl/repl_set_request_votes_args.cpp b/src/mongo/db/repl/repl_set_request_votes_args.cpp index 83ded65f45f..9dbafb9c564 100644 --- a/src/mongo/db/repl/repl_set_request_votes_args.cpp +++ b/src/mongo/db/repl/repl_set_request_votes_args.cpp @@ -70,7 +70,8 @@ const std::string kLegalResponseFieldNames[] = { Status ReplSetRequestVotesArgs::initialize(const BSONObj& argsObj) { - Status status = bsonCheckOnlyHasFields("ReplSetRequestVotes", argsObj, kLegalArgsFieldNames); + Status status = + bsonCheckOnlyHasFieldsForCommand("ReplSetRequestVotes", argsObj, kLegalArgsFieldNames); if (!status.isOK()) return status; diff --git a/src/mongo/s/commands/cluster_map_reduce_cmd.cpp b/src/mongo/s/commands/cluster_map_reduce_cmd.cpp index b155d322b3e..5c1b6445615 100644 --- a/src/mongo/s/commands/cluster_map_reduce_cmd.cpp +++ b/src/mongo/s/commands/cluster_map_reduce_cmd.cpp @@ -82,7 +82,7 @@ BSONObj fixForShards(const BSONObj& orig, BSONObjIterator i(orig); while (i.more()) { BSONElement e = i.next(); - const std::string fn = e.fieldName(); + const auto fn = e.fieldNameStringData(); if (fn == bypassDocumentValidationCommandOption() || fn == "map" || fn == "mapreduce" || fn == "mapReduce" || fn == "mapparams" || fn == "reduce" || fn == "query" || @@ -92,8 +92,8 @@ BSONObj fixForShards(const BSONObj& orig, b.append(e); } else if (fn == "out" || fn == "finalize" || fn == "writeConcern") { // We don't want to copy these - } else { - badShardedField = fn; + } else if (!Command::isGenericArgument(fn)) { + badShardedField = fn.toString(); return BSONObj(); } } diff --git a/src/mongo/s/write_ops/batched_delete_request.cpp b/src/mongo/s/write_ops/batched_delete_request.cpp index 1819423660b..74903579419 100644 --- a/src/mongo/s/write_ops/batched_delete_request.cpp +++ b/src/mongo/s/write_ops/batched_delete_request.cpp @@ -28,6 +28,7 @@ #include "mongo/s/write_ops/batched_delete_request.h" +#include "mongo/db/commands.h" #include "mongo/db/field_parser.h" #include "mongo/util/mongoutils/str.h" @@ -133,13 +134,9 @@ bool BatchedDeleteRequest::parseBSON(StringData dbName, const BSONObj& source, s if (fieldState == FieldParser::FIELD_INVALID) return false; _isOrderedSet = fieldState == FieldParser::FIELD_SET; - } else if (fieldName[0] != '$') { - std::initializer_list<StringData> ignoredFields = {"maxTimeMS", "shardVersion"}; - if (std::find(ignoredFields.begin(), ignoredFields.end(), fieldName) == - ignoredFields.end()) { - *errMsg = str::stream() << "Unknown option to delete command: " << fieldName; - return false; - } + } else if (!Command::isGenericArgument(fieldName)) { + *errMsg = str::stream() << "Unknown option to delete command: " << fieldName; + return false; } } diff --git a/src/mongo/s/write_ops/batched_insert_request.cpp b/src/mongo/s/write_ops/batched_insert_request.cpp index d4ee4c0527a..69aa4927643 100644 --- a/src/mongo/s/write_ops/batched_insert_request.cpp +++ b/src/mongo/s/write_ops/batched_insert_request.cpp @@ -29,6 +29,7 @@ #include "mongo/s/write_ops/batched_insert_request.h" #include "mongo/db/catalog/document_validation.h" +#include "mongo/db/commands.h" #include "mongo/db/field_parser.h" #include "mongo/util/mongoutils/str.h" @@ -112,8 +113,9 @@ bool BatchedInsertRequest::parseBSON(StringData dbName, const BSONObj& source, s while (sourceIt.more()) { BSONElement sourceEl = sourceIt.next(); + const auto fieldName = sourceEl.fieldNameStringData(); - if (collName() == sourceEl.fieldName()) { + if (fieldName == collName()) { std::string temp; FieldParser::FieldState fieldState = FieldParser::extract(sourceEl, collName, &temp, errMsg); @@ -124,7 +126,7 @@ bool BatchedInsertRequest::parseBSON(StringData dbName, const BSONObj& source, s str::stream() << "Invalid namespace: " << _ns.ns(), _ns.isValid()); _isNSSet = fieldState == FieldParser::FIELD_SET; - } else if (documents() == sourceEl.fieldName()) { + } else if (fieldName == documents()) { FieldParser::FieldState fieldState = FieldParser::extract(sourceEl, documents, &_documents, errMsg); if (fieldState == FieldParser::FIELD_INVALID) @@ -132,28 +134,23 @@ bool BatchedInsertRequest::parseBSON(StringData dbName, const BSONObj& source, s _isDocumentsSet = fieldState == FieldParser::FIELD_SET; if (_documents.size() >= 1) extractIndexNSS(_documents.at(0), &_targetNSS); - } else if (writeConcern() == sourceEl.fieldName()) { + } else if (fieldName == writeConcern()) { FieldParser::FieldState fieldState = FieldParser::extract(sourceEl, writeConcern, &_writeConcern, errMsg); if (fieldState == FieldParser::FIELD_INVALID) return false; _isWriteConcernSet = fieldState == FieldParser::FIELD_SET; - } else if (ordered() == sourceEl.fieldName()) { + } else if (fieldName == ordered()) { FieldParser::FieldState fieldState = FieldParser::extract(sourceEl, ordered, &_ordered, errMsg); if (fieldState == FieldParser::FIELD_INVALID) return false; _isOrderedSet = fieldState == FieldParser::FIELD_SET; - } else if (bypassDocumentValidationCommandOption() == sourceEl.fieldNameStringData()) { + } else if (fieldName == bypassDocumentValidationCommandOption()) { _shouldBypassValidation = sourceEl.trueValue(); - } else if (sourceEl.fieldName()[0] != '$') { - std::initializer_list<StringData> ignoredFields = {"maxTimeMS", "shardVersion"}; - if (std::find(ignoredFields.begin(), ignoredFields.end(), sourceEl.fieldName()) == - ignoredFields.end()) { - *errMsg = str::stream() << "Unknown option to insert command: " - << sourceEl.fieldName(); - return false; - } + } else if (!Command::isGenericArgument(fieldName)) { + *errMsg = str::stream() << "Unknown option to insert command: " << sourceEl.fieldName(); + return false; } } diff --git a/src/mongo/s/write_ops/batched_update_request.cpp b/src/mongo/s/write_ops/batched_update_request.cpp index 3c7f4fe9664..648c60938bf 100644 --- a/src/mongo/s/write_ops/batched_update_request.cpp +++ b/src/mongo/s/write_ops/batched_update_request.cpp @@ -29,6 +29,7 @@ #include "mongo/s/write_ops/batched_update_request.h" #include "mongo/db/catalog/document_validation.h" +#include "mongo/db/commands.h" #include "mongo/db/field_parser.h" #include "mongo/util/mongoutils/str.h" @@ -143,13 +144,9 @@ bool BatchedUpdateRequest::parseBSON(StringData dbName, const BSONObj& source, s _isOrderedSet = fieldState == FieldParser::FIELD_SET; } else if (fieldName == bypassDocumentValidationCommandOption()) { _shouldBypassValidation = elem.trueValue(); - } else if (fieldName[0] != '$') { - std::initializer_list<StringData> ignoredFields = {"maxTimeMS", "shardVersion"}; - if (std::find(ignoredFields.begin(), ignoredFields.end(), fieldName) == - ignoredFields.end()) { - *errMsg = str::stream() << "Unknown option to update command: " << fieldName; - return false; - } + } else if (!Command::isGenericArgument(fieldName)) { + *errMsg = str::stream() << "Unknown option to update command: " << fieldName; + return false; } } return true; |