diff options
author | Ian Boros <ian.boros@10gen.com> | 2019-01-10 18:42:43 -0500 |
---|---|---|
committer | Ian Boros <ian.boros@10gen.com> | 2019-01-11 13:22:42 -0500 |
commit | d315547544d7146b93a8e6e94cc4b88cd0d19c95 (patch) | |
tree | d6b247d3002a58249afe1934aa2a509a8e274a79 | |
parent | 7fc54303ba911cb94b0f7157710f1047fd78dbe0 (diff) | |
download | mongo-d315547544d7146b93a8e6e94cc4b88cd0d19c95.tar.gz |
SERVER-38275 ban explain with UUID
-rw-r--r-- | jstests/core/explain_uuid.js | 56 | ||||
-rw-r--r-- | src/mongo/db/commands.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/commands/count_cmd.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/commands/distinct.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/commands/find_cmd.cpp | 6 | ||||
-rw-r--r-- | src/mongo/idl/idl_parser.cpp | 6 |
6 files changed, 77 insertions, 9 deletions
diff --git a/jstests/core/explain_uuid.js b/jstests/core/explain_uuid.js new file mode 100644 index 00000000000..fc338016fa8 --- /dev/null +++ b/jstests/core/explain_uuid.js @@ -0,0 +1,56 @@ +/** + * Test that running explain() providing a collection UUID rather than collection name will fail + * cleanly. + */ +(function() { + "use strict"; + + // Use our own database so that we're guaranteed the only collection is this one. + const explainDB = db.getSiblingDB("explain_uuid_db"); + + assert.commandWorked(explainDB.dropDatabase()); + + const coll = explainDB.explain_uuid; + assert.commandWorked(coll.insert({a: 1})); + + const collInfos = explainDB.getCollectionInfos({name: coll.getName()}); + assert.eq(collInfos.length, 1, tojson(collInfos)); + const uuid = collInfos[0].info.uuid; + + // Run a find explain looking up by UUID. + assert.commandFailedWithCode(explainDB.runCommand({explain: {find: uuid}}), + ErrorCodes.InvalidNamespace); + + // Do similar for other commands. + assert.commandFailedWithCode(explainDB.runCommand({explain: {aggregate: uuid, cursor: {}}}), + ErrorCodes.TypeMismatch); + + assert.commandFailedWithCode(explainDB.runCommand({explain: {count: uuid}}), + ErrorCodes.InvalidNamespace); + + assert.commandFailedWithCode(explainDB.runCommand({explain: {distinct: uuid, key: "x"}}), + ErrorCodes.InvalidNamespace); + + // When auth is enabled, running findAndModify with an invalid namespace will produce a special + // error during the auth check, rather than the generic 'InvalidNamespace' error. + const expectedCode = TestData.auth ? 17137 : ErrorCodes.InvalidNamespace; + assert.commandFailedWithCode( + explainDB.runCommand({explain: {findAndModify: uuid, query: {a: 1}, remove: true}}), + expectedCode); + + assert.commandFailedWithCode( + explainDB.runCommand({explain: {delete: uuid, deletes: [{q: {}, limit: 1}]}}), + ErrorCodes.BadValue); + + assert.commandFailedWithCode(explainDB.runCommand({ + explain: { + update: uuid, + updates: [{ + q: {a: 1}, + u: {$set: {b: 1}}, + }] + } + }), + ErrorCodes.BadValue); + +})(); diff --git a/src/mongo/db/commands.cpp b/src/mongo/db/commands.cpp index cbaeb7f9625..266c05c907c 100644 --- a/src/mongo/db/commands.cpp +++ b/src/mongo/db/commands.cpp @@ -186,6 +186,12 @@ NamespaceString CommandHelpers::parseNsCollectionRequired(StringData dbname, // Accepts both BSON String and Symbol for collection name per SERVER-16260 // TODO(kangas) remove Symbol support in MongoDB 3.0 after Ruby driver audit BSONElement first = cmdObj.firstElement(); + const bool isUUID = (first.canonicalType() == canonicalizeBSONType(mongo::BinData) && + first.binDataType() == BinDataType::newUUID); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Collection name must be provided. UUID is not valid in this " + << "context", + !isUUID); uassert(ErrorCodes::InvalidNamespace, str::stream() << "collection name has invalid type " << typeName(first.type()), first.canonicalType() == canonicalizeBSONType(mongo::String)); diff --git a/src/mongo/db/commands/count_cmd.cpp b/src/mongo/db/commands/count_cmd.cpp index 8c08d19091c..cd2891f6266 100644 --- a/src/mongo/db/commands/count_cmd.cpp +++ b/src/mongo/db/commands/count_cmd.cpp @@ -112,11 +112,11 @@ public: BSONObjBuilder* out) const override { std::string dbname = opMsgRequest.getDatabase().toString(); const BSONObj& cmdObj = opMsgRequest.body; - // Acquire locks and resolve possible UUID. The RAII object is optional, because in the case - // of a view, the locks need to be released. + // Acquire locks. The RAII object is optional, because in the case of a view, the locks + // need to be released. boost::optional<AutoGetCollectionForReadCommand> ctx; ctx.emplace(opCtx, - CommandHelpers::parseNsOrUUID(dbname, cmdObj), + CommandHelpers::parseNsCollectionRequired(dbname, cmdObj), AutoGetCollection::ViewMode::kViewsPermitted); const auto nss = ctx->getNss(); diff --git a/src/mongo/db/commands/distinct.cpp b/src/mongo/db/commands/distinct.cpp index 90c98b5f70f..3535120348b 100644 --- a/src/mongo/db/commands/distinct.cpp +++ b/src/mongo/db/commands/distinct.cpp @@ -117,11 +117,11 @@ public: BSONObjBuilder* out) const override { std::string dbname = request.getDatabase().toString(); const BSONObj& cmdObj = request.body; - // Acquire locks and resolve possible UUID. The RAII object is optional, because in the case - // of a view, the locks need to be released. + // Acquire locks. The RAII object is optional, because in the case of a view, the locks + // need to be released. boost::optional<AutoGetCollectionForReadCommand> ctx; ctx.emplace(opCtx, - CommandHelpers::parseNsOrUUID(dbname, cmdObj), + CommandHelpers::parseNsCollectionRequired(dbname, cmdObj), AutoGetCollection::ViewMode::kViewsPermitted); const auto nss = ctx->getNss(); diff --git a/src/mongo/db/commands/find_cmd.cpp b/src/mongo/db/commands/find_cmd.cpp index 63f3c2cd17b..a47959978bc 100644 --- a/src/mongo/db/commands/find_cmd.cpp +++ b/src/mongo/db/commands/find_cmd.cpp @@ -135,11 +135,11 @@ public: BSONObjBuilder* out) const override { std::string dbname = request.getDatabase().toString(); const BSONObj& cmdObj = request.body; - // Acquire locks and resolve possible UUID. The RAII object is optional, because in the case - // of a view, the locks need to be released. + // Acquire locks. The RAII object is optional, because in the case of a view, the locks + // need to be released. boost::optional<AutoGetCollectionForReadCommand> ctx; ctx.emplace(opCtx, - CommandHelpers::parseNsOrUUID(dbname, cmdObj), + CommandHelpers::parseNsCollectionRequired(dbname, cmdObj), AutoGetCollection::ViewMode::kViewsPermitted); const auto nss = ctx->getNss(); diff --git a/src/mongo/idl/idl_parser.cpp b/src/mongo/idl/idl_parser.cpp index 46a0c16916e..bb317567fb3 100644 --- a/src/mongo/idl/idl_parser.cpp +++ b/src/mongo/idl/idl_parser.cpp @@ -227,6 +227,12 @@ void IDLParserErrorContext::throwBadEnumValue(StringData enumValue) const { NamespaceString IDLParserErrorContext::parseNSCollectionRequired(StringData dbName, const BSONElement& element) { + const bool isUUID = (element.canonicalType() == canonicalizeBSONType(mongo::BinData) && + element.binDataType() == BinDataType::newUUID); + uassert(ErrorCodes::BadValue, + str::stream() << "Collection name must be provided. UUID is not valid in this " + << "context", + !isUUID); uassert(ErrorCodes::BadValue, str::stream() << "collection name has invalid type " << typeName(element.type()), element.canonicalType() == canonicalizeBSONType(mongo::String)); |