diff options
| author | David Storch <david.storch@10gen.com> | 2016-12-05 11:19:48 -0500 |
|---|---|---|
| committer | David Storch <david.storch@10gen.com> | 2016-12-09 17:59:52 -0500 |
| commit | 7cf929f25638e4ad9525775c8ea0e18f3c86faf5 (patch) | |
| tree | 42328072009a72ce73f1c5bffe60f0c36ee0d492 | |
| parent | 586ac20773ff7dc18cabf329c238bf261e00387d (diff) | |
| download | mongo-7cf929f25638e4ad9525775c8ea0e18f3c86faf5.tar.gz | |
SERVER-24128 reject embedded null bytes in namespace string parsing
53 files changed, 809 insertions, 439 deletions
diff --git a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml index 031b3336ee3..2fa9b0857d6 100644 --- a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml +++ b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml @@ -82,8 +82,6 @@ selector: - jstests/sharding/sharded_limit_batchsize.js # Enable when 3.2.11 becomes last-stable - jstests/sharding/merge_chunks_test.js - # TODO Change in error reporting, enable when 3.4 becomes last-stable. See SERVER-26440. - - jstests/sharding/explain_cmd_invalid_namespace.js # $graphLookup is not supported in versions <= 3.2 - jstests/sharding/read_committed_lookup.js - jstests/sharding/balancer_shell_commands.js diff --git a/jstests/core/commands_namespace_parsing.js b/jstests/core/commands_namespace_parsing.js new file mode 100644 index 00000000000..c5e6cd3a285 --- /dev/null +++ b/jstests/core/commands_namespace_parsing.js @@ -0,0 +1,388 @@ +// This file tests that commands namespace parsing rejects embedded null bytes. +// Note that for each command, a properly formatted command object must be passed to the helper +// function, regardless of the namespace used in the command object. +(function() { + "use strict"; + + const isFullyQualified = true; + const isNotFullyQualified = false; + const isAdminCommand = true; + const isNotAdminCommand = false; + + // If the command expects the namespace to be fully qualified, set `isFullyQualified` to true. + // If the command must be run against the admin database, set `isAdminCommand` to true. + function assertFailsWithInvalidNamespacesForField( + field, command, isFullyQualified, isAdminCommand) { + const invalidNamespaces = []; + invalidNamespaces.push(isFullyQualified ? "mydb." : ""); + invalidNamespaces.push(isFullyQualified ? "mydb.\0" : "\0"); + invalidNamespaces.push(isFullyQualified ? "mydb.a\0b" : "a\0b"); + + const cmds = []; + for (let ns of invalidNamespaces) { + const cmd = Object.extend({}, command, /* deep copy */ true); + + const fieldNames = field.split("."); + const lastFieldNameIndex = fieldNames.length - 1; + let objToUpdate = cmd; + for (let i = 0; i < lastFieldNameIndex; i++) { + objToUpdate = objToUpdate[fieldNames[i]]; + } + objToUpdate[fieldNames[lastFieldNameIndex]] = ns; + + cmds.push(cmd); + } + + const dbCmd = isAdminCommand ? db.adminCommand : db.runCommand; + for (let cmd of cmds) { + assert.commandFailedWithCode(dbCmd.apply(db, [cmd]), ErrorCodes.InvalidNamespace); + } + } + + const isMaster = db.runCommand("ismaster"); + assert.commandWorked(isMaster); + const isMongos = (isMaster.msg === "isdbgrid"); + + const isMMAPv1 = (jsTest.options().storageEngine === "mmapv1"); + + db.commands_namespace_parsing.drop(); + assert.writeOK(db.commands_namespace_parsing.insert({a: 1})); + + // Test aggregate fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "aggregate", {aggregate: "", pipeline: []}, isNotFullyQualified, isNotAdminCommand); + + // Test count fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "count", {count: ""}, isNotFullyQualified, isNotAdminCommand); + + // Test distinct fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "distinct", {distinct: "", key: "a"}, isNotFullyQualified, isNotAdminCommand); + + // Test group fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("group.ns", + {group: {ns: "", $reduce: () => {}, initial: {}}}, + isNotFullyQualified, + isNotAdminCommand); + + // Test mapReduce fails with an invalid input collection name. + assertFailsWithInvalidNamespacesForField("mapreduce", + { + mapreduce: "", + map: function() { + emit(this.a, 1); + }, + reduce: function(key, values) { + return Array.sum(values); + }, + out: "commands_namespace_parsing_out" + }, + isNotFullyQualified, + isNotAdminCommand); + // Test mapReduce fails with an invalid output collection name. + assertFailsWithInvalidNamespacesForField("out", + { + mapreduce: "commands_namespace_parsing", + map: function() { + emit(this.a, 1); + }, + reduce: function(key, values) { + return Array.sum(values); + }, + out: "" + }, + isNotFullyQualified, + isNotAdminCommand); + + // Test geoNear fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "geoNear", {geoNear: "", near: [0.0, 0.0]}, isNotFullyQualified, isNotAdminCommand); + + if (!isMongos) { + // Test geoSearch fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "geoSearch", + {geoSearch: "", search: {}, near: [0.0, 0.0], maxDistance: 10}, + isNotFullyQualified, + isNotAdminCommand); + } + + // Test find fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "find", {find: ""}, isNotFullyQualified, isNotAdminCommand); + + // Test insert fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("insert", + {insert: "", documents: [{q: {a: 1}, u: {a: 2}}]}, + isNotFullyQualified, + isNotAdminCommand); + + // Test update fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("update", + {update: "", updates: [{q: {a: 1}, u: {a: 2}}]}, + isNotFullyQualified, + isNotAdminCommand); + + // Test delete fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("delete", + {delete: "", deletes: [{q: {a: 1}, limit: 1}]}, + isNotFullyQualified, + isNotAdminCommand); + + // Test findAndModify fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("findAndModify", + {findAndModify: "", update: {a: 2}}, + isNotFullyQualified, + isNotAdminCommand); + + // Test getMore fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("collection", + {getMore: NumberLong("123456"), collection: ""}, + isNotFullyQualified, + isNotAdminCommand); + + if (!isMongos) { + // Test parallelCollectionScan fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("parallelCollectionScan", + {parallelCollectionScan: "", numCursors: 10}, + isNotFullyQualified, + isNotAdminCommand); + + // Test godinsert fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "godinsert", {godinsert: "", obj: {_id: 1}}, isNotFullyQualified, isNotAdminCommand); + } + + // Test planCacheListFilters fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "planCacheListFilters", {planCacheListFilters: ""}, isNotFullyQualified, isNotAdminCommand); + + // Test planCacheSetFilter fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("planCacheSetFilter", + {planCacheSetFilter: "", query: {}, indexes: [{a: 1}]}, + isNotFullyQualified, + isNotAdminCommand); + + // Test planCacheClearFilters fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("planCacheClearFilters", + {planCacheClearFilters: ""}, + isNotFullyQualified, + isNotAdminCommand); + + // Test planCacheListQueryShapes fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("planCacheListQueryShapes", + {planCacheListQueryShapes: ""}, + isNotFullyQualified, + isNotAdminCommand); + + // Test planCacheListPlans fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("planCacheListPlans", + {planCacheListPlans: "", query: {}}, + isNotFullyQualified, + isNotAdminCommand); + + // Test planCacheClear fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "planCacheClear", {planCacheClear: ""}, isNotFullyQualified, isNotAdminCommand); + + if (!isMongos) { + // Test cleanupOrphaned fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "cleanupOrphaned", {cleanupOrphaned: ""}, isFullyQualified, isAdminCommand); + } + + if (isMongos) { + // Test enableSharding fails with an invalid database name. + assertFailsWithInvalidNamespacesForField( + "enableSharding", {enableSharding: ""}, isNotFullyQualified, isAdminCommand); + + // Test mergeChunks fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "mergeChunks", + {mergeChunks: "", bounds: [{_id: MinKey()}, {_id: MaxKey()}]}, + isFullyQualified, + isAdminCommand); + + // Test shardCollection fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("shardCollection", + {shardCollection: "", key: {_id: 1}}, + isFullyQualified, + isAdminCommand); + + // Test split fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "split", {split: "", find: {}}, isFullyQualified, isAdminCommand); + + // Test moveChunk fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "moveChunk", + {moveChunk: "", find: {}, to: "commands_namespace_parsing_out"}, + isNotFullyQualified, + isAdminCommand); + + // Test movePrimary fails with an invalid database name. + assertFailsWithInvalidNamespacesForField( + "movePrimary", {movePrimary: ""}, isNotFullyQualified, isAdminCommand); + + // Test updateZoneKeyRange fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "updateZoneKeyRange", + {updateZoneKeyRange: "", min: {_id: MinKey()}, max: {_id: MaxKey()}, zone: "3"}, + isNotFullyQualified, + isAdminCommand); + } + + // Test renameCollection fails with an invalid source collection name. + assertFailsWithInvalidNamespacesForField( + "renameCollection", {renameCollection: "", to: "test.b"}, isFullyQualified, isAdminCommand); + // Test renameCollection fails with an invalid target collection name. + assertFailsWithInvalidNamespacesForField( + "to", {renameCollection: "test.b", to: ""}, isFullyQualified, isAdminCommand); + + // Test copydb fails with an invalid fromdb name. + assertFailsWithInvalidNamespacesForField( + "fromdb", {copydb: 1, fromdb: "", todb: "b"}, isNotFullyQualified, isAdminCommand); + // Test copydb fails with an invalid todb name. + assertFailsWithInvalidNamespacesForField( + "todb", {copydb: 1, fromdb: "a", todb: ""}, isNotFullyQualified, isAdminCommand); + + // Test drop fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "drop", {drop: ""}, isNotFullyQualified, isNotAdminCommand); + + // Test create fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "create", {create: ""}, isNotFullyQualified, isNotAdminCommand); + + if (!isMongos) { + // Test cloneCollection fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("cloneCollection", + {cloneCollection: "", from: "fakehost"}, + isNotFullyQualified, + isNotAdminCommand); + + // Test cloneCollectionAsCapped fails with an invalid source collection name. + assertFailsWithInvalidNamespacesForField( + "cloneCollectionAsCapped", + {cloneCollectionAsCapped: "", toCollection: "b", size: 1024}, + isNotFullyQualified, + isNotAdminCommand); + // Test cloneCollectionAsCapped fails with an invalid target collection name. + assertFailsWithInvalidNamespacesForField( + "toCollection", + {cloneCollectionAsCapped: "commands_namespace_parsing", toCollection: "", size: 1024}, + isNotFullyQualified, + isNotAdminCommand); + + // Test convertToCapped fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("convertToCapped", + {convertToCapped: "", size: 1024}, + isNotFullyQualified, + isNotAdminCommand); + } + + // Test filemd5 fails with an invalid collection name. + // Note: for this command, it is OK to pass 'root: ""', so do not use the helper function. + assert.commandFailedWithCode(db.runCommand({filemd5: ObjectId(), root: "\0"}), + ErrorCodes.InvalidNamespace); + assert.commandFailedWithCode(db.runCommand({filemd5: ObjectId(), root: "a\0b"}), + ErrorCodes.InvalidNamespace); + + // Test createIndexes fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "createIndexes", + {createIndexes: "", indexes: [{key: {a: 1}, name: "a1"}]}, + isNotFullyQualified, + isNotAdminCommand); + + // Test listIndexes fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "listIndexes", {listIndexes: ""}, isNotFullyQualified, isNotAdminCommand); + + // Test dropIndexes fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "dropIndexes", {dropIndexes: "", index: "*"}, isNotFullyQualified, isNotAdminCommand); + + if (!isMongos) { + // Test compact fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "compact", {compact: ""}, isNotFullyQualified, isNotAdminCommand); + } + + // Test collMod fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "collMod", + {collMod: "", index: {keyPattern: {a: 1}, expireAfterSeconds: 60}}, + isNotFullyQualified, + isNotAdminCommand); + + // Test reIndex fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "reIndex", {reIndex: ""}, isNotFullyQualified, isNotAdminCommand); + + if (isMMAPv1 && !isMongos) { + // Test touch fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "touch", {touch: "", data: true, index: true}, isNotFullyQualified, isNotAdminCommand); + } + + // Test collStats fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "collStats", {collStats: ""}, isNotFullyQualified, isNotAdminCommand); + + // Test dataSize fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "dataSize", {dataSize: ""}, isFullyQualified, isNotAdminCommand); + + // Test explain of aggregate fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("aggregate", + {aggregate: "", pipeline: [], explain: true}, + isNotFullyQualified, + isNotAdminCommand); + + // Test explain of count fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "explain.count", {explain: {count: ""}}, isNotFullyQualified, isNotAdminCommand); + + // Test explain of distinct fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("explain.distinct", + {explain: {distinct: "", key: "a"}}, + isNotFullyQualified, + isNotAdminCommand); + + // Test explain of group fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "explain.group.ns", + {explain: {group: {ns: "", $reduce: () => {}, initial: {}}}}, + isNotFullyQualified, + isNotAdminCommand); + + // Test explain of find fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "explain.find", {explain: {find: ""}}, isNotFullyQualified, isNotAdminCommand); + + // Test explain of findAndModify fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField("explain.findAndModify", + {explain: {findAndModify: "", update: {a: 2}}}, + isNotFullyQualified, + isNotAdminCommand); + + // Test explain of delete fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "explain.delete", + {explain: {delete: "", deletes: [{q: {a: 1}, limit: 1}]}}, + isNotFullyQualified, + isNotAdminCommand); + + // Test explain of update fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "explain.update", + {explain: {update: "", updates: [{q: {a: 1}, u: {a: 2}}]}}, + isNotFullyQualified, + isNotAdminCommand); + + // Test validate fails with an invalid collection name. + assertFailsWithInvalidNamespacesForField( + "validate", {validate: ""}, isNotFullyQualified, isNotAdminCommand); +})(); diff --git a/jstests/core/views/views_creation.js b/jstests/core/views/views_creation.js index 2f8bcf52142..c4ade5e728e 100644 --- a/jstests/core/views/views_creation.js +++ b/jstests/core/views/views_creation.js @@ -29,7 +29,7 @@ // Collections that start with 'system.' that are not special to MongoDB fail with a different // error code. assert.commandFailedWithCode(viewsDB.runCommand({create: "system.foo", viewOn: "collection"}), - ErrorCodes.BadValue, + ErrorCodes.InvalidNamespace, "Created an illegal view named 'system.foo'"); // Create a collection for test purposes. @@ -79,8 +79,8 @@ assert.commandFailed(viewsDB.runCommand({create: "", viewOn: "collection", pipeline: pipe})); assert.commandFailedWithCode( viewsDB.runCommand({create: "system.local.new", viewOn: "collection", pipeline: pipe}), - ErrorCodes.BadValue); + ErrorCodes.InvalidNamespace); assert.commandFailedWithCode( viewsDB.runCommand({create: "dollar$", viewOn: "collection", pipeline: pipe}), - ErrorCodes.BadValue); + ErrorCodes.InvalidNamespace); }()); diff --git a/jstests/noPassthrough/copydb_illegal_collections.js b/jstests/noPassthrough/copydb_illegal_collections.js index 61f396631d3..cbce5abecaf 100644 --- a/jstests/noPassthrough/copydb_illegal_collections.js +++ b/jstests/noPassthrough/copydb_illegal_collections.js @@ -15,8 +15,7 @@ var db2 = rst.getPrimary().getDB("db2"); var res = db1.adminCommand({copydb: 1, fromdb: db1._name, todb: db2._name}); - var badValueCode = 2; - assert.commandFailedWithCode(res, badValueCode); + assert.commandFailedWithCode(res, ErrorCodes.InvalidNamespace); assert.gt(res["errmsg"].indexOf("cannot write to 'db2.system.replset'"), -1); rst.awaitReplication(); })(); diff --git a/jstests/sharding/explain_cmd_invalid_namespace.js b/jstests/sharding/explain_cmd_invalid_namespace.js deleted file mode 100644 index 989edec7660..00000000000 --- a/jstests/sharding/explain_cmd_invalid_namespace.js +++ /dev/null @@ -1,185 +0,0 @@ -// This test addresses the invalid namespace issue encountered in SERVER-26440, and checks the other -// explainable commands to ensure none are affected by the same bug. - -(function() { - "use strict"; - - // Start the sharding test. - let st = new ShardingTest({mongos: 1, shards: 1}); - - // Enable sharding on the "test" database. - assert.commandWorked(st.s.adminCommand({enableSharding: "test"})); - - // Connect to the "test" database. - let testDB = st.s.getDB("test"); - - // Create a non-empty test collection. - assert.writeOK(testDB.server26440.insert({a: 1})); - - // Test explain of aggregate fails with an invalid collection name. - // TODO SERVER-24128: The following two commands fail as defined in non-auth configurations, - // but fail differently in auth configurations. We should fail identically in both - // configurations. - // assert.commandFailedWithCode(testDB.runCommand({aggregate: "", pipeline:[], explain:true}), - // ErrorCodes.UnknownError); - // assert.commandFailedWithCode(testDB.runCommand({aggregate: "\0", pipeline:[], explain:true}), - // ErrorCodes.UnknownError); - // TODO SERVER-24128: Make namespace parsing for group reject names with embedded null bytes. - // TODO SERVER-24128: The following command runs successfully in non-auth configurations, but - // fails in auth configurations. We should fail in both configurations. - // assert.commandFailedWithCode(testDB.runCommand({aggregate: "a\0b", pipeline:[], - // explain:true}), ErrorCodes.InvalidNamespace); - - // Test explain of count fails with an invalid collection name. - assert.commandFailedWithCode(testDB.runCommand({explain: {count: ""}}), - ErrorCodes.InvalidNamespace); - assert.commandFailedWithCode(testDB.runCommand({explain: {count: "\0"}}), - ErrorCodes.InvalidNamespace); - assert.commandFailedWithCode(testDB.runCommand({explain: {count: "a\0b"}}), 17295); - - // Test explain of distinct fails with an invalid collection name. - assert.commandFailedWithCode(testDB.runCommand({explain: {distinct: "", key: "a"}}), - ErrorCodes.InvalidNamespace); - assert.commandFailedWithCode(testDB.runCommand({explain: {distinct: "\0", key: "a"}}), 17295); - assert.commandFailedWithCode(testDB.runCommand({explain: {distinct: "a\0b", key: "a"}}), 17295); - - // Test explain of group fails with an invalid collection name. - // TODO SERVER-24128: Currently, we massert() and print a stack trace. Instead, we should fail - // gracefully with a user assertion. - // TODO SERVER-24128: The following command fails in non-auth configurations, but fails - // differently in auth configurations. We should fail identically in both configurations. - // assert.commandFailedWithCode( - // testDB.runCommand({explain: {group: {ns: "", $reduce: () => {}, initial: {}}}}), - // ErrorCodes.InvalidNamespace); - // TODO SERVER-24128: Make namespace parsing for explain of group reject names with embedded - // null bytes. - // TODO SERVER-24128: The following two commands run successfully in non-auth configurations, - // but fail in auth configurations. We should fail in both configurations. - // assert.commandFailedWithCode( - // testDB.runCommand({explain: {group: {ns: "\0", $reduce: () => {}, initial: {}}}}), - // ErrorCodes.InvalidNamespace); - // TODO SERVER-24128: Make namespace parsing for explain of group reject names with embedded - // null bytes. - // assert.commandFailedWithCode( - // testDB.runCommand({explain: {group: {ns: "a\0b", $reduce: () => {}, initial: {}}}}), - // ErrorCodes.InvalidNamespace); - - // Test explain of find fails with an invalid collection name. - assert.commandFailedWithCode(testDB.runCommand({explain: {find: ""}}), - ErrorCodes.InvalidNamespace); - assert.commandFailedWithCode(testDB.runCommand({explain: {find: "\0"}}), - ErrorCodes.InvalidNamespace); - // TODO SERVER-24128: Make namespace parsing for explain of find reject names with embedded null - // bytes. - // assert.commandFailedWithCode(testDB.runCommand({explain: {find: "a\0b"}}), - // ErrorCodes.InvalidNamespace); - - // Test explain of findAndModify fails with an invalid collection name. - assert.commandFailedWithCode(testDB.runCommand({explain: {findAndModify: "", update: {a: 2}}}), - ErrorCodes.InvalidNamespace); - assert.commandFailedWithCode( - testDB.runCommand({explain: {findAndModify: "\0", update: {a: 2}}}), 17295); - assert.commandFailedWithCode( - testDB.runCommand({explain: {findAndModify: "a\0b", update: {a: 2}}}), 17295); - - // Test explain of delete fails with an invalid collection name. - // TODO SERVER-24128: Currently, we massert() and print a stack trace. Instead, we should fail - // gracefully with a user assertion. - assert.commandFailedWithCode( - testDB.runCommand({explain: {delete: "", deletes: [{q: {a: 1}, limit: 1}]}}), 28538); - assert.commandFailedWithCode( - testDB.runCommand({explain: {delete: "\0", deletes: [{q: {a: 1}, limit: 1}]}}), 17295); - assert.commandFailedWithCode( - testDB.runCommand({explain: {delete: "a\0b", deletes: [{q: {a: 1}, limit: 1}]}}), 17295); - - // Test explain of update fails with an invalid collection name. - // TODO SERVER-24128: Currently, we massert() and print a stack trace. Instead, we should fail - // gracefully with a user assertion. - assert.commandFailedWithCode( - testDB.runCommand({explain: {update: "", updates: [{q: {a: 1}, u: {a: 2}}]}}), 28538); - assert.commandFailedWithCode( - testDB.runCommand({explain: {update: "\0", updates: [{q: {a: 1}, u: {a: 2}}]}}), 17295); - assert.commandFailedWithCode( - testDB.runCommand({explain: {update: "a\0b", updates: [{q: {a: 1}, u: {a: 2}}]}}), 17295); - - // Test aggregate fails with an invalid collection name. - // TODO SERVER-24128: The following two commands fail as defined in non-auth configurations, - // but fail differently in auth configurations. We should fail identically in both - // configurations. - // assert.commandFailedWithCode(testDB.runCommand({aggregate: "", pipeline:[]}), - // ErrorCodes.UnknownError); - // assert.commandFailedWithCode(testDB.runCommand({aggregate: "\0", pipeline:[]}), - // ErrorCodes.UnknownError); - // TODO SERVER-24128: Make namespace parsing for group reject names with embedded null bytes. - // TODO SERVER-24128: The following command runs successfully in non-auth configurations, but - // fails in auth configurations. We should fail in both configurations. - // assert.commandFailedWithCode(testDB.runCommand({aggregate: "a\0b", pipeline:[]}), - // ErrorCodes.InvalidNamespace); - - // Test count fails with an invalid collection name. - assert.commandFailedWithCode(testDB.runCommand({count: ""}), ErrorCodes.InvalidNamespace); - assert.commandFailedWithCode(testDB.runCommand({count: "\0"}), ErrorCodes.InvalidNamespace); - // TODO SERVER-24128: Make namespace parsing for count reject names with embedded null bytes. - // assert.commandFailedWithCode(testDB.runCommand({count: "a\0b"}), - // ErrorCodes.InvalidNamespace); - - // Test distinct fails with an invalid collection name. - // TODO SERVER-24128: Currently, we massert() and print a stack trace. Instead, we should fail - // gracefully with a user assertion. - assert.commandFailedWithCode(testDB.runCommand({distinct: "", key: "a"}), 28538); - // TODO SERVER-24128: Currently, we massert() and print a stack trace. Instead, we should fail - // gracefully with a user assertion. - assert.commandFailedWithCode(testDB.runCommand({distinct: "\0", key: "a"}), 28538); - // TODO SERVER-24128: Make namespace parsing for distinct reject names with embedded null bytes. - // assert.commandFailedWithCode(testDB.runCommand({distinct: "a\0b", key: "a"}), - // ErrorCodes.InvalidNamespace); - - // Test group fails with an invalid collection name. - // TODO SERVER-24128: Currently, we massert() and print a stack trace. Instead, we should fail - // gracefully with a user assertion. - // TODO SERVER-24128: The following command fails as defined below in non-auth configurations, - // but fails differently in auth configurations. We should fail identically in both - // configurations. - // assert.commandFailedWithCode( - // testDB.runCommand({group: {ns: "", $reduce: () => {}, initial: {}}}), 28538); - // TODO SERVER-24128: Make namespace parsing for group reject names with embedded null bytes. - // TODO SERVER-24128: The following two commands run successfully in non-auth configurations, - // but fail in auth configurations. We should fail in both configurations. - // assert.commandFailedWithCode(testDB.runCommand({group: {ns: "\0", $reduce: () => {}, initial: - // {}}}), ErrorCodes.InvalidNamespace); - // TODO SERVER-24128: Make namespace parsing for group reject names with embedded null bytes. - // assert.commandFailedWithCode(testDB.runCommand({group: {ns: "a\0b", $reduce: () => {}, - // initial: - // {}}}), ErrorCodes.InvalidNamespace); - - // Test find fails with an invalid collection name. - assert.commandFailedWithCode(testDB.runCommand({find: ""}), ErrorCodes.InvalidNamespace); - assert.commandFailedWithCode(testDB.runCommand({find: "\0"}), ErrorCodes.InvalidNamespace); - // TODO SERVER-24128: Make namespace parsing for find reject names with embedded null bytes. - // assert.commandFailedWithCode(testDB.runCommand({find: "a\0b"}), ErrorCodes.InvalidNamespace); - - // Test findAndModify fails with an invalid collection name. - assert.commandFailedWithCode(testDB.runCommand({findAndModify: "", update: {a: 2}}), - ErrorCodes.InvalidNamespace); - assert.commandFailedWithCode(testDB.runCommand({findAndModify: "\0", update: {a: 2}}), 17295); - assert.commandFailedWithCode(testDB.runCommand({findAndModify: "a\0b", update: {a: 2}}), 17295); - - // Test delete fails with an invalid collection name. - assert.commandFailedWithCode(testDB.runCommand({delete: "", deletes: [{q: {a: 1}, limit: 1}]}), - ErrorCodes.InvalidNamespace); - assert.commandFailedWithCode( - testDB.runCommand({delete: "\0", deletes: [{q: {a: 1}, limit: 1}]}), 17295); - assert.commandFailedWithCode( - testDB.runCommand({delete: "a\0b", deletes: [{q: {a: 1}, limit: 1}]}), 17295); - - // Test update fails with an invalid collection name. - assert.commandFailedWithCode(testDB.runCommand({update: "", updates: [{q: {a: 1}, u: {a: 2}}]}), - ErrorCodes.InvalidNamespace); - assert.commandFailedWithCode( - testDB.runCommand({update: "\0", updates: [{q: {a: 1}, u: {a: 2}}]}), 17295); - assert.commandFailedWithCode( - testDB.runCommand({update: "a\0b", updates: [{q: {a: 1}, u: {a: 2}}]}), 17295); - - // Stop the sharding test. - st.stop(); -})(); diff --git a/src/mongo/client/fetcher.cpp b/src/mongo/client/fetcher.cpp index 741efe9afab..9c73f127809 100644 --- a/src/mongo/client/fetcher.cpp +++ b/src/mongo/client/fetcher.cpp @@ -118,7 +118,7 @@ Status parseCursorResponse(const BSONObj& obj, << "' field must be a string: " << obj); } - NamespaceString tempNss(namespaceElement.valuestrsafe()); + const NamespaceString tempNss(namespaceElement.valueStringData()); if (!tempNss.isValid()) { return Status(ErrorCodes::BadValue, str::stream() << "'" << kCursorFieldName << "." << kNamespaceFieldName diff --git a/src/mongo/db/catalog/create_collection.cpp b/src/mongo/db/catalog/create_collection.cpp index 743c1481213..227481c4104 100644 --- a/src/mongo/db/catalog/create_collection.cpp +++ b/src/mongo/db/catalog/create_collection.cpp @@ -48,14 +48,17 @@ Status createCollection(OperationContext* txn, // Extract ns from first cmdObj element. BSONElement firstElt = it.next(); - uassert(15888, "must pass name of collection to create", firstElt.valuestrsafe()[0] != '\0'); + uassert(ErrorCodes::TypeMismatch, + str::stream() << "Expected first element to be of type String in: " << cmdObj, + firstElt.type() == BSONType::String); + uassert(15888, "must pass name of collection to create", !firstElt.valueStringData().empty()); - Status status = userAllowedCreateNS(dbName, firstElt.valuestr()); + Status status = userAllowedCreateNS(dbName, firstElt.valueStringData()); if (!status.isOK()) { return status; } - NamespaceString nss(dbName, firstElt.valuestrsafe()); + const NamespaceString nss(dbName, firstElt.valueStringData()); // Build options object from remaining cmdObj elements. BSONObjBuilder optionsBuilder; diff --git a/src/mongo/db/cloner.cpp b/src/mongo/db/cloner.cpp index 6b3184eae4b..fbba5e00365 100644 --- a/src/mongo/db/cloner.cpp +++ b/src/mongo/db/cloner.cpp @@ -461,7 +461,7 @@ bool Cloner::copyCollection(OperationContext* txn, uassert(ErrorCodes::CommandNotSupportedOnView, str::stream() << "copyCollection not supported for views. ns: " - << col["name"].valuestrsafe(), + << col["name"].valueStringData(), !(status.isOK() && namespaceType == "view")); } diff --git a/src/mongo/db/commands.cpp b/src/mongo/db/commands.cpp index 42bad714800..8bb1892afe5 100644 --- a/src/mongo/db/commands.cpp +++ b/src/mongo/db/commands.cpp @@ -106,13 +106,13 @@ string Command::parseNs(const string& dbname, const BSONObj& cmdObj) const { if (first.type() != mongo::String) return dbname; - return str::stream() << dbname << '.' << cmdObj.firstElement().valuestr(); + return str::stream() << dbname << '.' << cmdObj.firstElement().valueStringData(); } ResourcePattern Command::parseResourcePattern(const std::string& dbname, const BSONObj& cmdObj) const { - std::string ns = parseNs(dbname, cmdObj); - if (ns.find('.') == std::string::npos) { + const std::string ns = parseNs(dbname, cmdObj); + if (!NamespaceString::validCollectionComponent(ns)) { return ResourcePattern::forDatabaseName(ns); } return ResourcePattern::forExactNamespace(NamespaceString(ns)); diff --git a/src/mongo/db/commands/collection_to_capped.cpp b/src/mongo/db/commands/collection_to_capped.cpp index 5484395cd6e..fda2e9b687e 100644 --- a/src/mongo/db/commands/collection_to_capped.cpp +++ b/src/mongo/db/commands/collection_to_capped.cpp @@ -72,12 +72,17 @@ public: targetActions.addAction(ActionType::insert); targetActions.addAction(ActionType::createIndex); targetActions.addAction(ActionType::convertToCapped); - std::string collection = cmdObj.getStringField("toCollection"); - uassert(16708, "bad 'toCollection' value", !collection.empty()); - out->push_back( - Privilege(ResourcePattern::forExactNamespace(NamespaceString(dbname, collection)), - targetActions)); + const auto nssElt = cmdObj["toCollection"]; + uassert(ErrorCodes::TypeMismatch, + "'toCollection' must be of type String", + nssElt.type() == BSONType::String); + const NamespaceString nss(dbname, nssElt.valueStringData()); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid target namespace: " << nss.ns(), + nss.isValid()); + + out->push_back(Privilege(ResourcePattern::forExactNamespace(nss), targetActions)); } bool run(OperationContext* txn, const string& dbname, @@ -85,12 +90,30 @@ public: int, string& errmsg, BSONObjBuilder& result) { - string from = jsobj.getStringField("cloneCollectionAsCapped"); - string to = jsobj.getStringField("toCollection"); + const auto fromElt = jsobj["cloneCollectionAsCapped"]; + const auto toElt = jsobj["toCollection"]; + + uassert(ErrorCodes::TypeMismatch, + "'cloneCollectionAsCapped' must be of type String", + fromElt.type() == BSONType::String); + uassert(ErrorCodes::TypeMismatch, + "'toCollection' must be of type String", + toElt.type() == BSONType::String); + + const StringData from(fromElt.valueStringData()); + const StringData to(toElt.valueStringData()); + + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid source collection name: " << from, + NamespaceString::validCollectionName(from)); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid target collection name: " << to, + NamespaceString::validCollectionName(to)); + double size = jsobj.getField("size").number(); bool temp = jsobj.getField("temp").trueValue(); - if (from.empty() || to.empty() || size == 0) { + if (size == 0) { errmsg = "invalid command spec"; return false; } @@ -116,7 +139,8 @@ public: str::stream() << "database " << dbname << " not found")); } - Status status = cloneCollectionAsCapped(txn, db, from, to, size, temp); + Status status = + cloneCollectionAsCapped(txn, db, from.toString(), to.toString(), size, temp); return appendCommandStatus(result, status); } } cmdCloneCollectionAsCapped; @@ -152,16 +176,15 @@ public: int, string& errmsg, BSONObjBuilder& result) { - string shortSource = jsobj.getStringField("convertToCapped"); + const NamespaceString nss(parseNsCollectionRequired(dbname, jsobj)); double size = jsobj.getField("size").number(); - if (shortSource.empty() || size == 0) { + if (size == 0) { errmsg = "invalid command spec"; return false; } - return appendCommandStatus( - result, convertToCapped(txn, NamespaceString(dbname, shortSource), size)); + return appendCommandStatus(result, convertToCapped(txn, nss, size)); } } cmdConvertToCapped; diff --git a/src/mongo/db/commands/copydb.cpp b/src/mongo/db/commands/copydb.cpp index 2e80deab625..00f26f8bd4d 100644 --- a/src/mongo/db/commands/copydb.cpp +++ b/src/mongo/db/commands/copydb.cpp @@ -134,24 +134,36 @@ public: } CloneOptions cloneOptions; - cloneOptions.fromDB = cmdObj.getStringField("fromdb"); + const auto fromdbElt = cmdObj["fromdb"]; + uassert(ErrorCodes::TypeMismatch, + "'fromdb' must be of type String", + fromdbElt.type() == BSONType::String); + cloneOptions.fromDB = fromdbElt.str(); cloneOptions.slaveOk = cmdObj["slaveOk"].trueValue(); cloneOptions.useReplAuth = false; cloneOptions.snapshot = true; - string todb = cmdObj.getStringField("todb"); - if (fromhost.empty() || todb.empty() || cloneOptions.fromDB.empty()) { + const auto todbElt = cmdObj["todb"]; + uassert(ErrorCodes::TypeMismatch, + "'todb' must be of type String", + todbElt.type() == BSONType::String); + const std::string todb = todbElt.str(); + + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid 'todb' name: " << todb, + NamespaceString::validDBName(todb, NamespaceString::DollarInDbNameBehavior::Allow)); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid 'fromdb' name: " << cloneOptions.fromDB, + NamespaceString::validDBName(cloneOptions.fromDB, + NamespaceString::DollarInDbNameBehavior::Allow)); + + if (fromhost.empty()) { errmsg = "params missing - {copydb: 1, fromhost: <connection string>, " "fromdb: <db>, todb: <db>}"; return false; } - if (!NamespaceString::validDBName(todb, NamespaceString::DollarInDbNameBehavior::Allow)) { - errmsg = "invalid todb name: " + todb; - return false; - } - Cloner cloner; // Get MONGODB-CR parameters diff --git a/src/mongo/db/commands/copydb_common.cpp b/src/mongo/db/commands/copydb_common.cpp index ab6f5078429..583c4054cfc 100644 --- a/src/mongo/db/commands/copydb_common.cpp +++ b/src/mongo/db/commands/copydb_common.cpp @@ -44,9 +44,18 @@ namespace mongo { namespace copydb { Status checkAuthForCopydbCommand(Client* client, const std::string& dbname, const BSONObj& cmdObj) { + const auto fromdbElt = cmdObj["fromdb"]; + if (fromdbElt.type() != BSONType::String) { + return Status(ErrorCodes::TypeMismatch, "'fromdb' must be of type String"); + } + const auto todbElt = cmdObj["todb"]; + if (todbElt.type() != BSONType::String) { + return Status(ErrorCodes::TypeMismatch, "'todb' must be of type String"); + } + bool fromSelf = StringData(cmdObj.getStringField("fromhost")).empty(); - StringData fromdb = cmdObj.getStringField("fromdb"); - StringData todb = cmdObj.getStringField("todb"); + const StringData fromdb = fromdbElt.valueStringData(); + const StringData todb = todbElt.valueStringData(); // get system collections std::vector<std::string> legalClientSystemCollections; diff --git a/src/mongo/db/commands/copydb_start_commands.cpp b/src/mongo/db/commands/copydb_start_commands.cpp index 3e7f0ee7e60..3dc9769024a 100644 --- a/src/mongo/db/commands/copydb_start_commands.cpp +++ b/src/mongo/db/commands/copydb_start_commands.cpp @@ -176,7 +176,15 @@ public: int, string& errmsg, BSONObjBuilder& result) { - const string fromDb = cmdObj.getStringField("fromdb"); + const auto fromdbElt = cmdObj["fromdb"]; + uassert(ErrorCodes::TypeMismatch, + "'renameCollection' must be of type String", + fromdbElt.type() == BSONType::String); + const string fromDb = fromdbElt.str(); + uassert( + ErrorCodes::InvalidNamespace, + str::stream() << "Invalid 'fromdb' name: " << fromDb, + NamespaceString::validDBName(fromDb, NamespaceString::DollarInDbNameBehavior::Allow)); string fromHost = cmdObj.getStringField("fromhost"); if (fromHost.empty()) { diff --git a/src/mongo/db/commands/create_indexes.cpp b/src/mongo/db/commands/create_indexes.cpp index 570d95a7bdb..4a1707b9124 100644 --- a/src/mongo/db/commands/create_indexes.cpp +++ b/src/mongo/db/commands/create_indexes.cpp @@ -231,7 +231,7 @@ public: int options, string& errmsg, BSONObjBuilder& result) { - const NamespaceString ns(parseNs(dbname, cmdObj)); + const NamespaceString ns(parseNsCollectionRequired(dbname, cmdObj)); Status status = userAllowedWriteNS(ns); if (!status.isOK()) diff --git a/src/mongo/db/commands/dbcommands.cpp b/src/mongo/db/commands/dbcommands.cpp index 4333c271f7a..5eea6f7e39f 100644 --- a/src/mongo/db/commands/dbcommands.cpp +++ b/src/mongo/db/commands/dbcommands.cpp @@ -539,7 +539,7 @@ public: virtual Status checkAuthForCommand(Client* client, const std::string& dbname, const BSONObj& cmdObj) { - NamespaceString nss(parseNs(dbname, cmdObj)); + const NamespaceString nss(parseNs(dbname, cmdObj)); return AuthorizationSession::get(client)->checkAuthForCreate(nss, cmdObj); } @@ -549,7 +549,7 @@ public: int, string& errmsg, BSONObjBuilder& result) { - const NamespaceString ns(parseNs(dbname, cmdObj)); + const NamespaceString ns(parseNsCollectionRequired(dbname, cmdObj)); if (cmdObj.hasField("autoIndexId")) { const char* deprecationWarning = @@ -666,7 +666,13 @@ public: } virtual std::string parseNs(const std::string& dbname, const BSONObj& cmdObj) const { - std::string collectionName = cmdObj.getStringField("root"); + std::string collectionName; + if (const auto rootElt = cmdObj["root"]) { + uassert(ErrorCodes::InvalidNamespace, + "'root' must be of type String", + rootElt.type() == BSONType::String); + collectionName = rootElt.str(); + } if (collectionName.empty()) collectionName = "fs"; collectionName += ".chunks"; @@ -685,7 +691,7 @@ public: int, string& errmsg, BSONObjBuilder& result) { - const std::string ns = parseNs(dbname, jsobj); + const NamespaceString nss(parseNs(dbname, jsobj)); md5digest d; md5_state_t st; @@ -713,7 +719,7 @@ public: BSONObj sort = BSON("files_id" << 1 << "n" << 1); MONGO_WRITE_CONFLICT_RETRY_LOOP_BEGIN { - auto qr = stdx::make_unique<QueryRequest>(NamespaceString(ns)); + auto qr = stdx::make_unique<QueryRequest>(nss); qr->setFilter(query); qr->setSort(sort); @@ -728,7 +734,7 @@ public: // Check shard version at startup. // This will throw before we've done any work if shard version is outdated // We drop and re-acquire these locks every document because md5'ing is expensive - unique_ptr<AutoGetCollectionForRead> ctx(new AutoGetCollectionForRead(txn, ns)); + unique_ptr<AutoGetCollectionForRead> ctx(new AutoGetCollectionForRead(txn, nss)); Collection* coll = ctx->getCollection(); auto statusWithPlanExecutor = getExecutor(txn, @@ -756,7 +762,7 @@ public: break; // skipped chunk is probably on another shard } log() << "should have chunk: " << n << " have:" << myn; - dumpChunks(txn, ns, query, sort); + dumpChunks(txn, nss.ns(), query, sort); uassert(10040, "chunks out of order", n == myn); } @@ -774,7 +780,7 @@ public: try { // RELOCKED - ctx.reset(new AutoGetCollectionForRead(txn, ns)); + ctx.reset(new AutoGetCollectionForRead(txn, nss)); } catch (const SendStaleConfigException& ex) { LOG(1) << "chunk metadata changed during filemd5, will retarget and continue"; break; @@ -1009,7 +1015,7 @@ public: int, string& errmsg, BSONObjBuilder& result) { - const NamespaceString nss(parseNs(dbname, jsobj)); + const NamespaceString nss(parseNsCollectionRequired(dbname, jsobj)); if (nss.coll().empty()) { errmsg = "No collection name specified"; @@ -1048,7 +1054,7 @@ public: virtual Status checkAuthForCommand(Client* client, const std::string& dbname, const BSONObj& cmdObj) { - NamespaceString nss(parseNs(dbname, cmdObj)); + const NamespaceString nss(parseNs(dbname, cmdObj)); return AuthorizationSession::get(client)->checkAuthForCollMod(nss, cmdObj); } @@ -1058,7 +1064,7 @@ public: int, string& errmsg, BSONObjBuilder& result) { - const NamespaceString nss = parseNsCollectionRequired(dbname, jsobj); + const NamespaceString nss(parseNsCollectionRequired(dbname, jsobj)); return appendCommandStatus(result, collMod(txn, nss, jsobj, &result)); } @@ -1107,6 +1113,9 @@ public: } const string ns = parseNs(dbname, jsobj); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid db name: " << ns, + NamespaceString::validDBName(ns, NamespaceString::DollarInDbNameBehavior::Allow)); // TODO (Kal): OldClientContext legacy, needs to be removed { diff --git a/src/mongo/db/commands/dbhash.cpp b/src/mongo/db/commands/dbhash.cpp index acfa7435c72..6d68cd93f00 100644 --- a/src/mongo/db/commands/dbhash.cpp +++ b/src/mongo/db/commands/dbhash.cpp @@ -102,7 +102,10 @@ public: } list<string> colls; - const string ns = parseNs(dbname, cmdObj); + const std::string ns = parseNs(dbname, cmdObj); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid db name: " << ns, + NamespaceString::validDBName(ns, NamespaceString::DollarInDbNameBehavior::Allow)); // We lock the entire database in S-mode in order to ensure that the contents will not // change for the snapshot. diff --git a/src/mongo/db/commands/distinct.cpp b/src/mongo/db/commands/distinct.cpp index e15c0e4c497..2543e4879bc 100644 --- a/src/mongo/db/commands/distinct.cpp +++ b/src/mongo/db/commands/distinct.cpp @@ -117,8 +117,7 @@ public: ExplainCommon::Verbosity verbosity, const rpc::ServerSelectionMetadata&, BSONObjBuilder* out) const { - const string ns = parseNs(dbname, cmdObj); - const NamespaceString nss(ns); + const NamespaceString nss(parseNsCollectionRequired(dbname, cmdObj)); const ExtensionsCallbackReal extensionsCallback(txn, &nss); auto parsedDistinct = ParsedDistinct::parse(txn, nss, cmdObj, extensionsCallback, true); @@ -134,7 +133,7 @@ public: "http://dochub.mongodb.org/core/3.4-feature-compatibility."); } - AutoGetCollectionOrViewForRead ctx(txn, ns); + AutoGetCollectionOrViewForRead ctx(txn, nss); Collection* collection = ctx.getCollection(); if (ctx.getView()) { @@ -151,7 +150,7 @@ public: } auto executor = getExecutorDistinct( - txn, collection, ns, &parsedDistinct.getValue(), PlanExecutor::YIELD_AUTO); + txn, collection, nss.ns(), &parsedDistinct.getValue(), PlanExecutor::YIELD_AUTO); if (!executor.isOK()) { return executor.getStatus(); } @@ -166,8 +165,7 @@ public: int options, string& errmsg, BSONObjBuilder& result) { - const string ns = parseNs(dbname, cmdObj); - const NamespaceString nss(ns); + const NamespaceString nss(parseNsCollectionRequired(dbname, cmdObj)); const ExtensionsCallbackReal extensionsCallback(txn, &nss); auto parsedDistinct = ParsedDistinct::parse(txn, nss, cmdObj, extensionsCallback, false); @@ -185,7 +183,7 @@ public: "http://dochub.mongodb.org/core/3.4-feature-compatibility.")); } - AutoGetCollectionOrViewForRead ctx(txn, ns); + AutoGetCollectionOrViewForRead ctx(txn, nss); Collection* collection = ctx.getCollection(); if (ctx.getView()) { @@ -214,7 +212,7 @@ public: } auto executor = getExecutorDistinct( - txn, collection, ns, &parsedDistinct.getValue(), PlanExecutor::YIELD_AUTO); + txn, collection, nss.ns(), &parsedDistinct.getValue(), PlanExecutor::YIELD_AUTO); if (!executor.isOK()) { return appendCommandStatus(result, executor.getStatus()); } diff --git a/src/mongo/db/commands/find_cmd.cpp b/src/mongo/db/commands/find_cmd.cpp index dc9e5e977cd..bdb0ea33fd6 100644 --- a/src/mongo/db/commands/find_cmd.cpp +++ b/src/mongo/db/commands/find_cmd.cpp @@ -124,7 +124,7 @@ public: Status checkAuthForCommand(Client* client, const std::string& dbname, const BSONObj& cmdObj) override { - NamespaceString nss(parseNs(dbname, cmdObj)); + const NamespaceString nss(parseNs(dbname, cmdObj)); auto hasTerm = cmdObj.hasField(kTermField); return AuthorizationSession::get(client)->checkAuthForFind(nss, hasTerm); } diff --git a/src/mongo/db/commands/geo_near_cmd.cpp b/src/mongo/db/commands/geo_near_cmd.cpp index d3f450b3855..eae4d16be7e 100644 --- a/src/mongo/db/commands/geo_near_cmd.cpp +++ b/src/mongo/db/commands/geo_near_cmd.cpp @@ -110,7 +110,7 @@ public: return false; } - const NamespaceString nss(parseNs(dbname, cmdObj)); + const NamespaceString nss(parseNsCollectionRequired(dbname, cmdObj)); AutoGetCollectionForRead ctx(txn, nss); Collection* collection = ctx.getCollection(); diff --git a/src/mongo/db/commands/getmore_cmd.cpp b/src/mongo/db/commands/getmore_cmd.cpp index cb93562e5df..6c956180070 100644 --- a/src/mongo/db/commands/getmore_cmd.cpp +++ b/src/mongo/db/commands/getmore_cmd.cpp @@ -129,7 +129,7 @@ public: } std::string parseNs(const std::string& dbname, const BSONObj& cmdObj) const override { - return GetMoreRequest::parseNs(dbname, cmdObj); + return GetMoreRequest::parseNs(dbname, cmdObj).ns(); } Status checkAuthForCommand(Client* client, diff --git a/src/mongo/db/commands/group_cmd.cpp b/src/mongo/db/commands/group_cmd.cpp index 88d03549317..0df1c64d113 100644 --- a/src/mongo/db/commands/group_cmd.cpp +++ b/src/mongo/db/commands/group_cmd.cpp @@ -98,18 +98,25 @@ private: virtual Status checkAuthForCommand(Client* client, const std::string& dbname, const BSONObj& cmdObj) { - std::string ns = parseNs(dbname, cmdObj); + const NamespaceString nss(parseNs(dbname, cmdObj)); + if (!AuthorizationSession::get(client)->isAuthorizedForActionsOnNamespace( - NamespaceString(ns), ActionType::find)) { + nss, ActionType::find)) { return Status(ErrorCodes::Unauthorized, "unauthorized"); } return Status::OK(); } virtual std::string parseNs(const std::string& dbname, const BSONObj& cmdObj) const { - const BSONObj& p = cmdObj.firstElement().embeddedObjectUserCheck(); - uassert(17211, "ns has to be set", p["ns"].type() == String); - return dbname + "." + p["ns"].String(); + const auto nsElt = cmdObj.firstElement().embeddedObjectUserCheck()["ns"]; + uassert(ErrorCodes::InvalidNamespace, + "'ns' must be of type String", + nsElt.type() == BSONType::String); + const NamespaceString nss(dbname, nsElt.valueStringData()); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid namespace: " << nss.ns(), + nss.isValid()); + return nss.ns(); } virtual Status explain(OperationContext* txn, @@ -230,7 +237,7 @@ private: Status _parseRequest(const std::string& dbname, const BSONObj& cmdObj, GroupRequest* request) const { - request->ns = parseNs(dbname, cmdObj); + request->ns = NamespaceString(parseNs(dbname, cmdObj)); // By default, group requests are regular group not explain of group. request->explain = false; diff --git a/src/mongo/db/commands/index_filter_commands.cpp b/src/mongo/db/commands/index_filter_commands.cpp index d9a5c5d900c..a591d48491c 100644 --- a/src/mongo/db/commands/index_filter_commands.cpp +++ b/src/mongo/db/commands/index_filter_commands.cpp @@ -120,8 +120,8 @@ bool IndexFilterCommand::run(OperationContext* txn, int options, string& errmsg, BSONObjBuilder& result) { - string ns = parseNs(dbname, cmdObj); - Status status = runIndexFilterCommand(txn, ns, cmdObj, &result); + const NamespaceString nss(parseNsCollectionRequired(dbname, cmdObj)); + Status status = runIndexFilterCommand(txn, nss.ns(), cmdObj, &result); return appendCommandStatus(result, status); } diff --git a/src/mongo/db/commands/list_indexes.cpp b/src/mongo/db/commands/list_indexes.cpp index 2fc5e07545b..59528d8bf11 100644 --- a/src/mongo/db/commands/list_indexes.cpp +++ b/src/mongo/db/commands/list_indexes.cpp @@ -100,7 +100,7 @@ public: // Check for the listIndexes ActionType on the database, or find on system.indexes for pre // 3.0 systems. - NamespaceString ns(parseNs(dbname, cmdObj)); + const NamespaceString ns(parseNsCollectionRequired(dbname, cmdObj)); if (authzSession->isAuthorizedForActionsOnResource(ResourcePattern::forExactNamespace(ns), ActionType::listIndexes) || authzSession->isAuthorizedForActionsOnResource( @@ -122,7 +122,7 @@ public: int, string& errmsg, BSONObjBuilder& result) { - const NamespaceString ns(parseNs(dbname, cmdObj)); + const NamespaceString ns(parseNsCollectionRequired(dbname, cmdObj)); const long long defaultBatchSize = std::numeric_limits<long long>::max(); long long batchSize; diff --git a/src/mongo/db/commands/mr.cpp b/src/mongo/db/commands/mr.cpp index 5e5306f5452..06a0fc43878 100644 --- a/src/mongo/db/commands/mr.cpp +++ b/src/mongo/db/commands/mr.cpp @@ -279,7 +279,14 @@ void JSReducer::_reduce(const BSONList& tuples, BSONObj& key, int& endSizeEstima Config::Config(const string& _dbname, const BSONObj& cmdObj) { dbname = _dbname; - ns = dbname + "." + cmdObj.firstElement().valuestrsafe(); + uassert(ErrorCodes::TypeMismatch, + str::stream() << "'mapReduce' must be of type String", + cmdObj.firstElement().type() == BSONType::String); + const NamespaceString nss(dbname, cmdObj.firstElement().valueStringData()); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid namespace: " << nss.ns(), + nss.isValid()); + ns = nss.ns(); verbose = cmdObj["verbose"].trueValue(); jsMode = cmdObj["jsMode"].trueValue(); @@ -306,9 +313,12 @@ Config::Config(const string& _dbname, const BSONObj& cmdObj) { } if (outputOptions.outType != INMEMORY) { // setup temp collection name - tempNamespace = str::stream() - << (outputOptions.outDB.empty() ? dbname : outputOptions.outDB) << ".tmp.mr." - << cmdObj.firstElement().String() << "_" << JOB_NUMBER.fetchAndAdd(1); + tempNamespace = + NamespaceString(outputOptions.outDB.empty() ? dbname : outputOptions.outDB, + str::stream() << "tmp.mr." << cmdObj.firstElement().valueStringData() + << "_" + << JOB_NUMBER.fetchAndAdd(1)) + .ns(); incLong = tempNamespace + "_inc"; } @@ -1723,13 +1733,19 @@ public: ShardedConnectionInfo::addHook(); // legacy name - string shardedOutputCollection = cmdObj["shardedOutputCollection"].valuestrsafe(); + const auto shardedOutputCollectionElt = cmdObj["shardedOutputCollection"]; + uassert(ErrorCodes::InvalidNamespace, + "'shardedOutputCollection' must be of type String", + shardedOutputCollectionElt.type() == BSONType::String); + const std::string shardedOutputCollection = shardedOutputCollectionElt.str(); verify(shardedOutputCollection.size() > 0); - string inputNS; + + std::string inputNS; if (cmdObj["inputDB"].type() == String) { - inputNS = cmdObj["inputDB"].String() + "." + shardedOutputCollection; + inputNS = + NamespaceString(cmdObj["inputDB"].valueStringData(), shardedOutputCollection).ns(); } else { - inputNS = dbname + "." + shardedOutputCollection; + inputNS = NamespaceString(dbname, shardedOutputCollection).ns(); } CurOp* curOp = CurOp::get(txn); diff --git a/src/mongo/db/commands/mr_common.cpp b/src/mongo/db/commands/mr_common.cpp index d180f276249..1356a6f6fb2 100644 --- a/src/mongo/db/commands/mr_common.cpp +++ b/src/mongo/db/commands/mr_common.cpp @@ -88,9 +88,12 @@ Config::OutputOptions Config::parseOutputOptions(const std::string& dbname, cons } if (outputOptions.outType != INMEMORY) { - outputOptions.finalNamespace = mongoutils::str::stream() - << (outputOptions.outDB.empty() ? dbname : outputOptions.outDB) << "." - << outputOptions.collectionName; + const StringData outDb(outputOptions.outDB.empty() ? dbname : outputOptions.outDB); + const NamespaceString nss(outDb, outputOptions.collectionName); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid 'out' namespace: " << nss.ns(), + nss.isValid()); + outputOptions.finalNamespace = nss.ns(); } return outputOptions; @@ -103,7 +106,7 @@ void addPrivilegesRequiredForMapReduce(Command* commandTemplate, Config::OutputOptions outputOptions = Config::parseOutputOptions(dbname, cmdObj); ResourcePattern inputResource(commandTemplate->parseResourcePattern(dbname, cmdObj)); - uassert(17142, + uassert(ErrorCodes::InvalidNamespace, mongoutils::str::stream() << "Invalid input resource " << inputResource.toString(), inputResource.isExactNamespacePattern()); out->push_back(Privilege(inputResource, ActionType::find)); @@ -123,7 +126,7 @@ void addPrivilegesRequiredForMapReduce(Command* commandTemplate, ResourcePattern outputResource( ResourcePattern::forExactNamespace(NamespaceString(outputOptions.finalNamespace))); - uassert(17143, + uassert(ErrorCodes::InvalidNamespace, mongoutils::str::stream() << "Invalid target namespace " << outputResource.ns().ns(), outputResource.ns().isValid()); diff --git a/src/mongo/db/commands/parallel_collection_scan.cpp b/src/mongo/db/commands/parallel_collection_scan.cpp index 6fee00a712f..75b0e59512b 100644 --- a/src/mongo/db/commands/parallel_collection_scan.cpp +++ b/src/mongo/db/commands/parallel_collection_scan.cpp @@ -90,7 +90,7 @@ public: int options, string& errmsg, BSONObjBuilder& result) { - const NamespaceString ns(parseNs(dbname, cmdObj)); + const NamespaceString ns(parseNsCollectionRequired(dbname, cmdObj)); AutoGetCollectionForRead ctx(txn, ns.ns()); diff --git a/src/mongo/db/commands/pipeline_command.cpp b/src/mongo/db/commands/pipeline_command.cpp index 2eb1c49649d..7fe8d54992d 100644 --- a/src/mongo/db/commands/pipeline_command.cpp +++ b/src/mongo/db/commands/pipeline_command.cpp @@ -337,7 +337,7 @@ public: Status checkAuthForCommand(Client* client, const std::string& dbname, const BSONObj& cmdObj) final { - NamespaceString nss(parseNs(dbname, cmdObj)); + const NamespaceString nss(parseNsCollectionRequired(dbname, cmdObj)); return AuthorizationSession::get(client)->checkAuthForAggregate(nss, cmdObj); } @@ -606,12 +606,7 @@ public: int options, string& errmsg, BSONObjBuilder& result) { - const std::string ns = parseNs(db, cmdObj); - if (nsToCollectionSubstring(ns).empty()) { - errmsg = "missing collection name"; - return false; - } - NamespaceString nss(ns); + const NamespaceString nss(parseNsCollectionRequired(db, cmdObj)); // Parse the options for this request. auto request = AggregationRequest::parseFromBSON(nss, cmdObj); diff --git a/src/mongo/db/commands/plan_cache_commands.cpp b/src/mongo/db/commands/plan_cache_commands.cpp index 25b76b6fbe9..fe713a0667f 100644 --- a/src/mongo/db/commands/plan_cache_commands.cpp +++ b/src/mongo/db/commands/plan_cache_commands.cpp @@ -115,8 +115,8 @@ bool PlanCacheCommand::run(OperationContext* txn, int options, string& errmsg, BSONObjBuilder& result) { - string ns = parseNs(dbname, cmdObj); - Status status = runPlanCacheCommand(txn, ns, cmdObj, &result); + const NamespaceString nss(parseNsCollectionRequired(dbname, cmdObj)); + Status status = runPlanCacheCommand(txn, nss.ns(), cmdObj, &result); return appendCommandStatus(result, status); } diff --git a/src/mongo/db/commands/rename_collection_cmd.cpp b/src/mongo/db/commands/rename_collection_cmd.cpp index 1b213898805..99f5617cf94 100644 --- a/src/mongo/db/commands/rename_collection_cmd.cpp +++ b/src/mongo/db/commands/rename_collection_cmd.cpp @@ -91,31 +91,39 @@ public: int, string& errmsg, BSONObjBuilder& result) { - string source = cmdObj.getStringField(getName()); - string target = cmdObj.getStringField("to"); - - if (!NamespaceString::validCollectionComponent(target.c_str())) { - errmsg = "invalid collection name: " + target; - return false; - } - if (source.empty() || target.empty()) { - errmsg = "invalid command syntax"; - return false; - } + const auto sourceNsElt = cmdObj[getName()]; + const auto targetNsElt = cmdObj["to"]; + + uassert(ErrorCodes::TypeMismatch, + "'renameCollection' must be of type String", + sourceNsElt.type() == BSONType::String); + uassert(ErrorCodes::TypeMismatch, + "'to' must be of type String", + targetNsElt.type() == BSONType::String); + + const NamespaceString source(sourceNsElt.valueStringData()); + const NamespaceString target(targetNsElt.valueStringData()); + + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid source namespace: " << source.ns(), + source.isValid()); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid target namespace: " << target.ns(), + target.isValid()); if ((repl::getGlobalReplicationCoordinator()->getReplicationMode() != repl::ReplicationCoordinator::modeNone)) { - if (NamespaceString(source).isOplog()) { + if (source.isOplog()) { errmsg = "can't rename live oplog while replicating"; return false; } - if (NamespaceString(target).isOplog()) { + if (target.isOplog()) { errmsg = "can't rename to live oplog while replicating"; return false; } } - if (NamespaceString::oplog(source) != NamespaceString::oplog(target)) { + if (source.isOplog() != target.isOplog()) { errmsg = "If either the source or target of a rename is an oplog name, both must be"; return false; } @@ -132,16 +140,15 @@ public: return false; } - if (NamespaceString(source).coll() == "system.indexes" || - NamespaceString(target).coll() == "system.indexes") { + if (source.isSystemDotIndexes() || target.isSystemDotIndexes()) { errmsg = "renaming system.indexes is not allowed"; return false; } return appendCommandStatus(result, renameCollection(txn, - NamespaceString(source), - NamespaceString(target), + source, + target, cmdObj["dropTarget"].trueValue(), cmdObj["stayTemp"].trueValue())); } diff --git a/src/mongo/db/commands/rename_collection_common.cpp b/src/mongo/db/commands/rename_collection_common.cpp index 7bfadbf779f..99b0002521b 100644 --- a/src/mongo/db/commands/rename_collection_common.cpp +++ b/src/mongo/db/commands/rename_collection_common.cpp @@ -45,8 +45,18 @@ namespace rename_collection { Status checkAuthForRenameCollectionCommand(Client* client, const std::string& dbname, const BSONObj& cmdObj) { - NamespaceString sourceNS = NamespaceString(cmdObj.getStringField("renameCollection")); - NamespaceString targetNS = NamespaceString(cmdObj.getStringField("to")); + const auto sourceNsElt = cmdObj["renameCollection"]; + const auto targetNsElt = cmdObj["to"]; + + uassert(ErrorCodes::TypeMismatch, + "'renameCollection' must be of type String", + sourceNsElt.type() == BSONType::String); + uassert(ErrorCodes::TypeMismatch, + "'to' must be of type String", + targetNsElt.type() == BSONType::String); + + const NamespaceString sourceNS(sourceNsElt.valueStringData()); + const NamespaceString targetNS(targetNsElt.valueStringData()); bool dropTarget = cmdObj["dropTarget"].trueValue(); if (sourceNS.db() == targetNS.db() && !sourceNS.isSystem() && !targetNS.isSystem()) { diff --git a/src/mongo/db/commands/test_commands.cpp b/src/mongo/db/commands/test_commands.cpp index 359e97292c5..ccba3bfb41a 100644 --- a/src/mongo/db/commands/test_commands.cpp +++ b/src/mongo/db/commands/test_commands.cpp @@ -81,22 +81,20 @@ public: int, string& errmsg, BSONObjBuilder& result) { - string coll = cmdObj["godinsert"].valuestrsafe(); - log() << "test only command godinsert invoked coll:" << coll; - uassert(13049, "godinsert must specify a collection", !coll.empty()); - string ns = dbname + "." + coll; + const NamespaceString nss(parseNsCollectionRequired(dbname, cmdObj)); + log() << "test only command godinsert invoked coll:" << nss.coll(); BSONObj obj = cmdObj["obj"].embeddedObjectUserCheck(); ScopedTransaction transaction(txn, MODE_IX); Lock::DBLock lk(txn->lockState(), dbname, MODE_X); - OldClientContext ctx(txn, ns); + OldClientContext ctx(txn, nss.ns()); Database* db = ctx.db(); WriteUnitOfWork wunit(txn); UnreplicatedWritesBlock unreplicatedWritesBlock(txn); - Collection* collection = db->getCollection(ns); + Collection* collection = db->getCollection(nss); if (!collection) { - collection = db->createCollection(txn, ns); + collection = db->createCollection(txn, nss.ns()); if (!collection) { errmsg = "could not create collection"; return false; diff --git a/src/mongo/db/commands/validate.cpp b/src/mongo/db/commands/validate.cpp index f1d5ff4a349..9622947e59f 100644 --- a/src/mongo/db/commands/validate.cpp +++ b/src/mongo/db/commands/validate.cpp @@ -87,9 +87,8 @@ public: return true; } - string ns = dbname + "." + cmdObj.firstElement().valuestrsafe(); + const NamespaceString nss(parseNsCollectionRequired(dbname, cmdObj)); - NamespaceString ns_string(ns); const bool full = cmdObj["full"].trueValue(); const bool scanData = cmdObj["scandata"].trueValue(); @@ -101,20 +100,20 @@ public: level = kValidateRecordStore; } - if (!ns_string.isNormal() && full) { + if (!nss.isNormal() && full) { errmsg = "Can only run full validate on a regular collection"; return false; } if (!serverGlobalParams.quiet) { - LOG(0) << "CMD: validate " << ns; + LOG(0) << "CMD: validate " << nss.ns(); } - AutoGetDb ctx(txn, ns_string.db(), MODE_IX); - Lock::CollectionLock collLk(txn->lockState(), ns_string.ns(), MODE_X); - Collection* collection = ctx.getDb() ? ctx.getDb()->getCollection(ns_string) : NULL; + AutoGetDb ctx(txn, nss.db(), MODE_IX); + Lock::CollectionLock collLk(txn->lockState(), nss.ns(), MODE_X); + Collection* collection = ctx.getDb() ? ctx.getDb()->getCollection(nss) : NULL; if (!collection) { - if (ctx.getDb() && ctx.getDb()->getViewCatalog()->lookup(txn, ns_string.ns())) { + if (ctx.getDb() && ctx.getDb()->getViewCatalog()->lookup(txn, nss.ns())) { errmsg = "Cannot validate a view"; return appendCommandStatus(result, {ErrorCodes::CommandNotSupportedOnView, errmsg}); } @@ -123,7 +122,7 @@ public: return false; } - result.append("ns", ns); + result.append("ns", nss.ns()); ValidateResults results; Status status = collection->validate(txn, level, &results, &result); diff --git a/src/mongo/db/exec/group.cpp b/src/mongo/db/exec/group.cpp index 5ec8226c431..37ce67ad9fc 100644 --- a/src/mongo/db/exec/group.cpp +++ b/src/mongo/db/exec/group.cpp @@ -96,9 +96,8 @@ Status GroupStage::initGroupScripting() { const std::string userToken = AuthorizationSession::get(Client::getCurrent())->getAuthenticatedUserNamesToken(); - const NamespaceString nss(_request.ns); _scope = getGlobalScriptEngine()->getPooledScope( - getOpCtx(), nss.db().toString(), "group" + userToken); + getOpCtx(), _request.ns.db().toString(), "group" + userToken); if (!_request.reduceScope.isEmpty()) { _scope->init(&_request.reduceScope); } diff --git a/src/mongo/db/exec/group.h b/src/mongo/db/exec/group.h index 7ec5efe647f..95b8a788a1d 100644 --- a/src/mongo/db/exec/group.h +++ b/src/mongo/db/exec/group.h @@ -30,6 +30,7 @@ #include "mongo/bson/simple_bsonobj_comparator.h" #include "mongo/db/exec/plan_stage.h" +#include "mongo/db/namespace_string.h" #include "mongo/scripting/engine.h" namespace mongo { @@ -41,7 +42,7 @@ class Collection; */ struct GroupRequest { // Namespace to operate on (e.g. "foo.bar"). - std::string ns; + NamespaceString ns; // A predicate describing the set of documents to group. BSONObj query; diff --git a/src/mongo/db/namespace_string-inl.h b/src/mongo/db/namespace_string-inl.h index 0ff45fca39e..1377d38ce41 100644 --- a/src/mongo/db/namespace_string-inl.h +++ b/src/mongo/db/namespace_string-inl.h @@ -122,14 +122,17 @@ inline NamespaceString::NamespaceString() : _ns(), _dotIndex(0) {} inline NamespaceString::NamespaceString(StringData nsIn) { _ns = nsIn.toString(); // copy to our buffer _dotIndex = _ns.find('.'); + uassert(ErrorCodes::InvalidNamespace, + "namespaces cannot have embedded null characters", + _ns.find('\0') == std::string::npos); } inline NamespaceString::NamespaceString(StringData dbName, StringData collectionName) : _ns(dbName.size() + collectionName.size() + 1, '\0') { - uassert(17235, + uassert(ErrorCodes::InvalidNamespace, "'.' is an invalid character in a database name", dbName.find('.') == std::string::npos); - uassert(17246, + uassert(ErrorCodes::InvalidNamespace, "Collection names cannot start with '.'", collectionName.empty() || collectionName[0] != '.'); std::string::iterator it = std::copy(dbName.begin(), dbName.end(), _ns.begin()); @@ -139,7 +142,7 @@ inline NamespaceString::NamespaceString(StringData dbName, StringData collection _dotIndex = dbName.size(); dassert(it == _ns.end()); dassert(_ns[_dotIndex] == '.'); - uassert(17295, + uassert(ErrorCodes::InvalidNamespace, "namespaces cannot have embedded null characters", _ns.find('\0') == std::string::npos); } diff --git a/src/mongo/db/ops/insert.cpp b/src/mongo/db/ops/insert.cpp index db4a490392a..cfc9b10f35f 100644 --- a/src/mongo/db/ops/insert.cpp +++ b/src/mongo/db/ops/insert.cpp @@ -142,7 +142,7 @@ Status userAllowedWriteNS(const NamespaceString& ns) { Status userAllowedWriteNS(StringData db, StringData coll) { if (coll == "system.profile") { - return Status(ErrorCodes::BadValue, + return Status(ErrorCodes::InvalidNamespace, str::stream() << "cannot write to '" << db << ".system.profile'"); } return userAllowedCreateNS(db, coll); @@ -152,19 +152,19 @@ Status userAllowedCreateNS(StringData db, StringData coll) { // validity checking if (db.size() == 0) - return Status(ErrorCodes::BadValue, "db cannot be blank"); + return Status(ErrorCodes::InvalidNamespace, "db cannot be blank"); if (!NamespaceString::validDBName(db, NamespaceString::DollarInDbNameBehavior::Allow)) - return Status(ErrorCodes::BadValue, "invalid db name"); + return Status(ErrorCodes::InvalidNamespace, "invalid db name"); if (coll.size() == 0) - return Status(ErrorCodes::BadValue, "collection cannot be blank"); + return Status(ErrorCodes::InvalidNamespace, "collection cannot be blank"); if (!NamespaceString::validCollectionName(coll)) - return Status(ErrorCodes::BadValue, "invalid collection name"); + return Status(ErrorCodes::InvalidNamespace, "invalid collection name"); if (db.size() + 1 /* dot */ + coll.size() > NamespaceString::MaxNsCollectionLen) - return Status(ErrorCodes::BadValue, + return Status(ErrorCodes::InvalidNamespace, str::stream() << "fully qualified namespace " << db << '.' << coll << " is too long " << "(max is " @@ -174,7 +174,7 @@ Status userAllowedCreateNS(StringData db, StringData coll) { // check spceial areas if (db == "system") - return Status(ErrorCodes::BadValue, "cannot use 'system' database"); + return Status(ErrorCodes::InvalidNamespace, "cannot use 'system' database"); if (coll.startsWith("system.")) { @@ -202,7 +202,7 @@ Status userAllowedCreateNS(StringData db, StringData coll) { if (coll == "system.replset") return Status::OK(); } - return Status(ErrorCodes::BadValue, + return Status(ErrorCodes::InvalidNamespace, str::stream() << "cannot write to '" << db << "." << coll << "'"); } diff --git a/src/mongo/db/ops/write_ops_parsers.cpp b/src/mongo/db/ops/write_ops_parsers.cpp index 3e7281fcdca..e67de9c2e7a 100644 --- a/src/mongo/db/ops/write_ops_parsers.cpp +++ b/src/mongo/db/ops/write_ops_parsers.cpp @@ -90,6 +90,9 @@ void parseWriteCommand(StringData dbName, // The key is the command name and the value is the collection name checkBSONType(String, field); op->ns = NamespaceString(dbName, field.valueStringData()); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid namespace: " << op->ns.ns(), + op->ns.isValid()); firstElement = false; continue; } diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index 3cc981a03a8..3ba0d9bfcb6 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -999,7 +999,8 @@ StatusWith<unique_ptr<PlanExecutor>> getExecutorGroup(OperationContext* txn, unique_ptr<PlanStage> root = make_unique<GroupStage>(txn, request, ws.get(), new EOFStage(txn)); - return PlanExecutor::make(txn, std::move(ws), std::move(root), request.ns, yieldPolicy); + return PlanExecutor::make( + txn, std::move(ws), std::move(root), request.ns.ns(), yieldPolicy); } const NamespaceString nss(request.ns); diff --git a/src/mongo/db/query/getmore_request.cpp b/src/mongo/db/query/getmore_request.cpp index 6244d77ff56..48f68d338ee 100644 --- a/src/mongo/db/query/getmore_request.cpp +++ b/src/mongo/db/query/getmore_request.cpp @@ -70,7 +70,7 @@ GetMoreRequest::GetMoreRequest(NamespaceString namespaceString, Status GetMoreRequest::isValid() const { if (!nss.isValid()) { - return Status(ErrorCodes::BadValue, + return Status(ErrorCodes::InvalidNamespace, str::stream() << "Invalid namespace for getMore: " << nss.ns()); } @@ -89,11 +89,11 @@ Status GetMoreRequest::isValid() const { } // static -std::string GetMoreRequest::parseNs(const std::string& dbname, const BSONObj& cmdObj) { +NamespaceString GetMoreRequest::parseNs(const std::string& dbname, const BSONObj& cmdObj) { BSONElement collElt = cmdObj["collection"]; const std::string coll = (collElt.type() == BSONType::String) ? collElt.String() : ""; - return str::stream() << dbname << "." << coll; + return NamespaceString(dbname, coll); } // static @@ -103,7 +103,7 @@ StatusWith<GetMoreRequest> GetMoreRequest::parseFromBSON(const std::string& dbna // Required fields. boost::optional<CursorId> cursorid; - boost::optional<std::string> fullns; + boost::optional<NamespaceString> nss; // Optional fields. boost::optional<long long> batchSize; @@ -127,7 +127,7 @@ StatusWith<GetMoreRequest> GetMoreRequest::parseFromBSON(const std::string& dbna << cmdObj}; } - fullns = parseNs(dbname, cmdObj); + nss = parseNs(dbname, cmdObj); } else if (str::equals(fieldName, kBatchSizeField)) { if (!el.isNumber()) { return {ErrorCodes::TypeMismatch, @@ -171,17 +171,13 @@ StatusWith<GetMoreRequest> GetMoreRequest::parseFromBSON(const std::string& dbna str::stream() << "Field 'getMore' missing in: " << cmdObj}; } - if (!fullns) { + if (!nss) { return {ErrorCodes::FailedToParse, str::stream() << "Field 'collection' missing in: " << cmdObj}; } - GetMoreRequest request(NamespaceString(*fullns), - *cursorid, - batchSize, - awaitDataTimeout, - term, - lastKnownCommittedOpTime); + GetMoreRequest request( + std::move(*nss), *cursorid, batchSize, awaitDataTimeout, term, lastKnownCommittedOpTime); Status validStatus = request.isValid(); if (!validStatus.isOK()) { return validStatus; diff --git a/src/mongo/db/query/getmore_request.h b/src/mongo/db/query/getmore_request.h index 16455bfb055..8fa2d0fc5dd 100644 --- a/src/mongo/db/query/getmore_request.h +++ b/src/mongo/db/query/getmore_request.h @@ -69,7 +69,7 @@ struct GetMoreRequest { */ BSONObj toBSON() const; - static std::string parseNs(const std::string& dbname, const BSONObj& cmdObj); + static NamespaceString parseNs(const std::string& dbname, const BSONObj& cmdObj); const NamespaceString nss; const CursorId cursorid; diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp index a9c29c2791d..8c58b6d383e 100644 --- a/src/mongo/db/repl/oplog.cpp +++ b/src/mongo/db/repl/oplog.cpp @@ -639,9 +639,17 @@ std::map<std::string, ApplyOpMetadata> opsMap = { {ErrorCodes::NamespaceNotFound, ErrorCodes::IndexNotFound}}}, {"renameCollection", {[](OperationContext* txn, const char* ns, BSONObj& cmd) -> Status { + const auto sourceNsElt = cmd.firstElement(); + const auto targetNsElt = cmd["to"]; + uassert(ErrorCodes::TypeMismatch, + "'renameCollection' must be of type String", + sourceNsElt.type() == BSONType::String); + uassert(ErrorCodes::TypeMismatch, + "'to' must be of type String", + targetNsElt.type() == BSONType::String); return renameCollection(txn, - NamespaceString(cmd.firstElement().valuestrsafe()), - NamespaceString(cmd["to"].valuestrsafe()), + NamespaceString(sourceNsElt.valueStringData()), + NamespaceString(targetNsElt.valueStringData()), cmd["dropTarget"].trueValue(), cmd["stayTemp"].trueValue()); }, @@ -686,6 +694,9 @@ Status applyOperation_inlock(OperationContext* txn, if (fieldO.isABSONObj()) o = fieldO.embeddedObject(); + uassert(ErrorCodes::InvalidNamespace, + "'ns' must be of type String", + fieldNs.type() == BSONType::String); const StringData ns = fieldNs.valueStringData(); BSONObj o2; @@ -1003,7 +1014,10 @@ Status applyCommand_inlock(OperationContext* txn, BSONObj o = fieldO.embeddedObject(); - const NamespaceString nss(fieldNs.valuestrsafe()); + uassert(ErrorCodes::InvalidNamespace, + "'ns' must be of type String", + fieldNs.type() == BSONType::String); + const NamespaceString nss(fieldNs.valueStringData()); if (!nss.isValid()) { return {ErrorCodes::InvalidNamespace, "invalid ns: " + std::string(nss.ns())}; } diff --git a/src/mongo/db/s/cleanup_orphaned_cmd.cpp b/src/mongo/db/s/cleanup_orphaned_cmd.cpp index a9b361027bb..038ecbfbdeb 100644 --- a/src/mongo/db/s/cleanup_orphaned_cmd.cpp +++ b/src/mongo/db/s/cleanup_orphaned_cmd.cpp @@ -214,15 +214,10 @@ public: return false; } - if (ns == "") { - errmsg = "no collection name specified"; - return false; - } - - if (!NamespaceString(ns).isValid()) { - errmsg = "invalid namespace"; - return false; - } + const NamespaceString nss(ns); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid namespace: " << nss.ns(), + nss.isValid()); BSONObj startingFromKey; if (!FieldParser::extract(cmdObj, startingFromKeyField, &startingFromKey, &errmsg)) { @@ -243,7 +238,7 @@ public: } ChunkVersion shardVersion; - Status status = shardingState->refreshMetadataNow(txn, NamespaceString(ns), &shardVersion); + Status status = shardingState->refreshMetadataNow(txn, nss, &shardVersion); if (!status.isOK()) { if (status.code() == ErrorCodes::RemoteChangeDetected) { warning() << "Shard version in transition detected while refreshing " @@ -255,8 +250,8 @@ public: } BSONObj stoppedAtKey; - CleanupResult cleanupResult = cleanupOrphanedData( - txn, NamespaceString(ns), startingFromKey, writeConcern, &stoppedAtKey, &errmsg); + CleanupResult cleanupResult = + cleanupOrphanedData(txn, nss, startingFromKey, writeConcern, &stoppedAtKey, &errmsg); if (cleanupResult == CleanupResult_Error) { return false; diff --git a/src/mongo/s/commands/cluster_enable_sharding_cmd.cpp b/src/mongo/s/commands/cluster_enable_sharding_cmd.cpp index e6352a95fa1..1254fca23b5 100644 --- a/src/mongo/s/commands/cluster_enable_sharding_cmd.cpp +++ b/src/mongo/s/commands/cluster_enable_sharding_cmd.cpp @@ -95,10 +95,10 @@ public: BSONObjBuilder& result) { const std::string dbname = parseNs("", cmdObj); - if (dbname.empty() || !nsIsDbOnly(dbname)) { - errmsg = "invalid db name specified: " + dbname; - return false; - } + uassert( + ErrorCodes::InvalidNamespace, + str::stream() << "invalid db name specified: " << dbname, + NamespaceString::validDBName(dbname, NamespaceString::DollarInDbNameBehavior::Allow)); if (dbname == "admin" || dbname == "config" || dbname == "local") { errmsg = "can't shard " + dbname + " database"; diff --git a/src/mongo/s/commands/cluster_find_cmd.cpp b/src/mongo/s/commands/cluster_find_cmd.cpp index 9ef1eab35fe..52edfe5e20e 100644 --- a/src/mongo/s/commands/cluster_find_cmd.cpp +++ b/src/mongo/s/commands/cluster_find_cmd.cpp @@ -96,7 +96,7 @@ public: Status checkAuthForCommand(Client* client, const std::string& dbname, const BSONObj& cmdObj) final { - NamespaceString nss(parseNs(dbname, cmdObj)); + const NamespaceString nss(parseNs(dbname, cmdObj)); auto hasTerm = cmdObj.hasField(kTermField); return AuthorizationSession::get(client)->checkAuthForFind(nss, hasTerm); } @@ -107,13 +107,7 @@ public: ExplainCommon::Verbosity verbosity, const rpc::ServerSelectionMetadata& serverSelectionMetadata, BSONObjBuilder* out) const final { - const string fullns = parseNs(dbname, cmdObj); - const NamespaceString nss(fullns); - if (!nss.isValid()) { - return {ErrorCodes::InvalidNamespace, - str::stream() << "Invalid collection name: " << nss.ns()}; - } - + const NamespaceString nss(parseNsCollectionRequired(dbname, cmdObj)); // Parse the command BSON to a QueryRequest. bool isExplain = true; auto qr = QueryRequest::makeFromFindCommand(std::move(nss), cmdObj, isExplain); @@ -160,12 +154,7 @@ public: // We count find command as a query op. globalOpCounters.gotQuery(); - const NamespaceString nss(parseNs(dbname, cmdObj)); - if (!nss.isValid()) { - return appendCommandStatus(result, - {ErrorCodes::InvalidNamespace, - str::stream() << "Invalid collection name: " << nss.ns()}); - } + const NamespaceString nss(parseNsCollectionRequired(dbname, cmdObj)); const bool isExplain = false; auto qr = QueryRequest::makeFromFindCommand(nss, cmdObj, isExplain); diff --git a/src/mongo/s/commands/cluster_move_chunk_cmd.cpp b/src/mongo/s/commands/cluster_move_chunk_cmd.cpp index 4404d9663a0..81a258f4051 100644 --- a/src/mongo/s/commands/cluster_move_chunk_cmd.cpp +++ b/src/mongo/s/commands/cluster_move_chunk_cmd.cpp @@ -109,7 +109,11 @@ public: auto scopedCM = uassertStatusOK(ScopedChunkManager::refreshAndGet(txn, nss)); - const string toString = cmdObj["to"].valuestrsafe(); + const auto toElt = cmdObj["to"]; + uassert(ErrorCodes::TypeMismatch, + "'to' must be of type String", + toElt.type() == BSONType::String); + const std::string toString = toElt.str(); if (!toString.size()) { errmsg = "you have to specify where you want to move the chunk"; return false; diff --git a/src/mongo/s/commands/cluster_move_primary_cmd.cpp b/src/mongo/s/commands/cluster_move_primary_cmd.cpp index 63d94120343..b908137ceda 100644 --- a/src/mongo/s/commands/cluster_move_primary_cmd.cpp +++ b/src/mongo/s/commands/cluster_move_primary_cmd.cpp @@ -93,7 +93,11 @@ public: } virtual std::string parseNs(const std::string& dbname, const BSONObj& cmdObj) const { - return cmdObj.firstElement().str(); + const auto nsElt = cmdObj.firstElement(); + uassert(ErrorCodes::InvalidNamespace, + "'movePrimary' must be of type String", + nsElt.type() == BSONType::String); + return nsElt.str(); } virtual bool run(OperationContext* txn, @@ -104,10 +108,10 @@ public: BSONObjBuilder& result) { const string dbname = parseNs("", cmdObj); - if (dbname.empty() || !nsIsDbOnly(dbname)) { - errmsg = "invalid db name specified: " + dbname; - return false; - } + uassert( + ErrorCodes::InvalidNamespace, + str::stream() << "invalid db name specified: " << dbname, + NamespaceString::validDBName(dbname, NamespaceString::DollarInDbNameBehavior::Allow)); if (dbname == "admin" || dbname == "config" || dbname == "local") { errmsg = "can't move primary for " + dbname + " database"; @@ -124,7 +128,11 @@ public: shared_ptr<DBConfig> config = status.getValue(); - const string to = cmdObj["to"].valuestrsafe(); + const auto toElt = cmdObj["to"]; + uassert(ErrorCodes::TypeMismatch, + "'to' must be of type String", + toElt.type() == BSONType::String); + const std::string to = toElt.str(); if (!to.size()) { errmsg = "you have to specify where you want to move it"; return false; diff --git a/src/mongo/s/commands/cluster_pipeline_cmd.cpp b/src/mongo/s/commands/cluster_pipeline_cmd.cpp index 11564077cb0..46ab48b39a2 100644 --- a/src/mongo/s/commands/cluster_pipeline_cmd.cpp +++ b/src/mongo/s/commands/cluster_pipeline_cmd.cpp @@ -68,7 +68,7 @@ public: Status checkAuthForCommand(Client* client, const std::string& dbname, const BSONObj& cmdObj) final { - NamespaceString nss(parseNs(dbname, cmdObj)); + const NamespaceString nss(parseNsCollectionRequired(dbname, cmdObj)); return AuthorizationSession::get(client)->checkAuthForAggregate(nss, cmdObj); } @@ -78,8 +78,7 @@ public: int options, std::string& errmsg, BSONObjBuilder& result) { - const std::string fullns = parseNs(dbname, cmdObj); - const NamespaceString nss(fullns); + const NamespaceString nss(parseNsCollectionRequired(dbname, cmdObj)); ClusterAggregate::Namespaces nsStruct; nsStruct.requestedNss = nss; diff --git a/src/mongo/s/commands/cluster_remove_shard_cmd.cpp b/src/mongo/s/commands/cluster_remove_shard_cmd.cpp index 42acb26fd11..6aeee9bd7e7 100644 --- a/src/mongo/s/commands/cluster_remove_shard_cmd.cpp +++ b/src/mongo/s/commands/cluster_remove_shard_cmd.cpp @@ -85,7 +85,11 @@ public: int options, std::string& errmsg, BSONObjBuilder& result) { - const string target = cmdObj.firstElement().valuestrsafe(); + uassert(ErrorCodes::TypeMismatch, + str::stream() << "Field '" << cmdObj.firstElement().fieldName() + << "' must be of type String", + cmdObj.firstElement().type() == BSONType::String); + const string target = cmdObj.firstElement().str(); const auto shardStatus = grid.shardRegistry()->getShard(txn, ShardId(target)); if (!shardStatus.isOK()) { diff --git a/src/mongo/s/commands/cluster_write_cmd.cpp b/src/mongo/s/commands/cluster_write_cmd.cpp index c7784629993..4751322a0d9 100644 --- a/src/mongo/s/commands/cluster_write_cmd.cpp +++ b/src/mongo/s/commands/cluster_write_cmd.cpp @@ -150,7 +150,6 @@ public: // Disable the last error object for the duration of the write LastError::Disabled disableLastError(cmdLastError); - // TODO: if we do namespace parsing, push this to the type if (!request.parseBSON(dbname, cmdObj, &errmsg) || !request.isValid(&errmsg)) { // Batch parse failure response.setOk(false); diff --git a/src/mongo/s/commands/commands_public.cpp b/src/mongo/s/commands/commands_public.cpp index 4c2506e19ab..74e94ffc96e 100644 --- a/src/mongo/s/commands/commands_public.cpp +++ b/src/mongo/s/commands/commands_public.cpp @@ -234,14 +234,14 @@ public: const string& dbName, BSONObj& cmdObj, vector<ShardId>& shardIds) { - const string fullns = dbName + '.' + cmdObj.firstElement().valuestrsafe(); + const NamespaceString nss(parseNsCollectionRequired(dbName, cmdObj)); auto status = Grid::get(txn)->catalogCache()->getDatabase(txn, dbName); uassertStatusOK(status.getStatus()); shared_ptr<DBConfig> conf = status.getValue(); - if (!conf->isShardingEnabled() || !conf->isSharded(fullns)) { + if (!conf->isShardingEnabled() || !conf->isSharded(nss.ns())) { shardIds.push_back(conf->getPrimaryId()); } else { Grid::get(txn)->shardRegistry()->getAllShardIds(&shardIds); @@ -259,10 +259,10 @@ public: int options, string& errmsg, BSONObjBuilder& result) { - const string fullns = parseNs(dbName, cmdObj); + const NamespaceString nss(parseNs(dbName, cmdObj)); auto conf = uassertStatusOK(Grid::get(txn)->catalogCache()->getDatabase(txn, dbName)); - if (!conf->isSharded(fullns)) { + if (!conf->isSharded(nss.ns())) { return passthrough(txn, conf.get(), cmdObj, options, result); } @@ -427,7 +427,7 @@ public: virtual Status checkAuthForCommand(Client* client, const std::string& dbname, const BSONObj& cmdObj) { - NamespaceString nss(parseNs(dbname, cmdObj)); + const NamespaceString nss(parseNsCollectionRequired(dbname, cmdObj)); return AuthorizationSession::get(client)->checkAuthForCollMod(nss, cmdObj); } @@ -458,7 +458,7 @@ public: int options, string& errmsg, BSONObjBuilder& output) { - const NamespaceString nss = parseNsCollectionRequired(dbName, cmdObj); + const NamespaceString nss(parseNsCollectionRequired(dbName, cmdObj)); auto conf = uassertStatusOK(Grid::get(txn)->catalogCache()->getDatabase(txn, dbName)); if (!conf->isShardingEnabled() || !conf->isSharded(nss.ns())) { @@ -514,7 +514,7 @@ public: virtual Status checkAuthForCommand(Client* client, const std::string& dbname, const BSONObj& cmdObj) { - NamespaceString nss(parseNs(dbname, cmdObj)); + const NamespaceString nss(parseNsCollectionRequired(dbname, cmdObj)); return AuthorizationSession::get(client)->checkAuthForCreate(nss, cmdObj); } virtual bool supportsWriteConcern(const BSONObj& cmd) const override { @@ -567,20 +567,20 @@ public: return appendCommandStatus(result, status.getStatus()); } - const NamespaceString fullns = parseNsCollectionRequired(dbName, cmdObj); + const NamespaceString nss(parseNsCollectionRequired(dbName, cmdObj)); - log() << "DROP: " << fullns; + log() << "DROP: " << nss.ns(); const auto& db = status.getValue(); - if (!db->isShardingEnabled() || !db->isSharded(fullns.ns())) { + if (!db->isShardingEnabled() || !db->isSharded(nss.ns())) { log() << "\tdrop going to do passthrough"; return passthrough(txn, db.get(), cmdObj, result); } - uassertStatusOK(Grid::get(txn)->catalogClient(txn)->dropCollection(txn, fullns)); + uassertStatusOK(Grid::get(txn)->catalogClient(txn)->dropCollection(txn, nss)); // Force a full reload next time the just dropped namespace is accessed - db->invalidateNs(fullns.ns()); + db->invalidateNs(nss.ns()); return true; } @@ -606,17 +606,34 @@ public: int, string& errmsg, BSONObjBuilder& result) { - const string fullnsFrom = cmdObj.firstElement().valuestrsafe(); - const string dbNameFrom = nsToDatabase(fullnsFrom); + const auto fullNsFromElt = cmdObj.firstElement(); + uassert(ErrorCodes::InvalidNamespace, + "'renameCollection' must be of type String", + fullNsFromElt.type() == BSONType::String); + const NamespaceString fullnsFrom(fullNsFromElt.valueStringData()); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid source namespace: " << fullnsFrom.ns(), + fullnsFrom.isValid()); + const string dbNameFrom = fullnsFrom.db().toString(); + auto confFrom = uassertStatusOK(Grid::get(txn)->catalogCache()->getDatabase(txn, dbNameFrom)); - const string fullnsTo = cmdObj["to"].valuestrsafe(); - const string dbNameTo = nsToDatabase(fullnsTo); + const auto fullnsToElt = cmdObj["to"]; + uassert(ErrorCodes::InvalidNamespace, + "'to' must be of type String", + fullnsToElt.type() == BSONType::String); + const NamespaceString fullnsTo(fullnsToElt.valueStringData()); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid target namespace: " << fullnsTo.ns(), + fullnsTo.isValid()); + const string dbNameTo = fullnsTo.db().toString(); auto confTo = uassertStatusOK(Grid::get(txn)->catalogCache()->getDatabase(txn, dbNameTo)); - uassert(13138, "You can't rename a sharded collection", !confFrom->isSharded(fullnsFrom)); - uassert(13139, "You can't rename to a sharded collection", !confTo->isSharded(fullnsTo)); + uassert( + 13138, "You can't rename a sharded collection", !confFrom->isSharded(fullnsFrom.ns())); + uassert( + 13139, "You can't rename to a sharded collection", !confTo->isSharded(fullnsTo.ns())); auto shardTo = confTo->getPrimaryId(); auto shardFrom = confFrom->getPrimaryId(); @@ -650,10 +667,13 @@ public: int, string& errmsg, BSONObjBuilder& result) { - const string todb = cmdObj.getStringField("todb"); - uassert(ErrorCodes::EmptyFieldName, "missing todb argument", !todb.empty()); + const auto todbElt = cmdObj["todb"]; uassert(ErrorCodes::InvalidNamespace, - "invalid todb argument", + "'todb' must be of type String", + todbElt.type() == BSONType::String); + const std::string todb = todbElt.str(); + uassert(ErrorCodes::InvalidNamespace, + "Invalid todb argument", NamespaceString::validDBName(todb, NamespaceString::DollarInDbNameBehavior::Allow)); auto scopedToDb = uassertStatusOK(ScopedShardDatabase::getOrCreate(txn, todb)); @@ -665,8 +685,15 @@ public: if (!fromhost.empty()) { return adminPassthrough(txn, scopedToDb.db(), cmdObj, result); } else { - const string fromdb = cmdObj.getStringField("fromdb"); - uassert(13399, "need a fromdb argument", !fromdb.empty()); + const auto fromDbElt = cmdObj["fromdb"]; + uassert(ErrorCodes::InvalidNamespace, + "'fromdb' must be of type String", + fromDbElt.type() == BSONType::String); + const std::string fromdb = fromDbElt.str(); + uassert(ErrorCodes::InvalidNamespace, + "invalid fromdb argument", + NamespaceString::validDBName(fromdb, + NamespaceString::DollarInDbNameBehavior::Allow)); shared_ptr<DBConfig> confFrom = uassertStatusOK(Grid::get(txn)->catalogCache()->getDatabase(txn, fromdb)); @@ -714,10 +741,10 @@ public: int, string& errmsg, BSONObjBuilder& result) { - const string fullns = parseNs(dbName, cmdObj); + const NamespaceString nss(parseNsCollectionRequired(dbName, cmdObj)); auto conf = uassertStatusOK(Grid::get(txn)->catalogCache()->getDatabase(txn, dbName)); - if (!conf->isShardingEnabled() || !conf->isSharded(fullns)) { + if (!conf->isShardingEnabled() || !conf->isSharded(nss.ns())) { result.appendBool("sharded", false); result.append("primary", conf->getPrimaryId().toString()); @@ -726,7 +753,7 @@ public: result.appendBool("sharded", true); - shared_ptr<ChunkManager> cm = conf->getChunkManager(txn, fullns); + shared_ptr<ChunkManager> cm = conf->getChunkManager(txn, nss.ns()); massert(12594, "how could chunk manager be null!", cm); BSONObjBuilder shardStats; @@ -840,7 +867,7 @@ public: unscaledCollSize += shardAvgObjSize * shardObjCount; } - result.append("ns", fullns); + result.append("ns", nss.ns()); for (map<string, long long>::iterator i = counts.begin(); i != counts.end(); ++i) result.appendNumber(i->first, i->second); @@ -973,6 +1000,10 @@ public: return true; } + virtual std::string parseNs(const std::string& dbname, const BSONObj& cmdObj) const { + return parseNsCollectionRequired(dbname, cmdObj).ns(); + } + } convertToCappedCmd; class GroupCmd : public NotAllowedOnShardedCollectionCmd { @@ -994,8 +1025,16 @@ public: return true; } - virtual std::string parseNs(const std::string& dbName, const BSONObj& cmdObj) const { - return dbName + "." + cmdObj.firstElement().embeddedObjectUserCheck()["ns"].valuestrsafe(); + virtual std::string parseNs(const std::string& dbname, const BSONObj& cmdObj) const { + const auto nsElt = cmdObj.firstElement().embeddedObjectUserCheck()["ns"]; + uassert(ErrorCodes::InvalidNamespace, + "'ns' must be of type String", + nsElt.type() == BSONType::String); + const NamespaceString nss(dbname, nsElt.valueStringData()); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid namespace: " << nss.ns(), + nss.isValid()); + return nss.ns(); } Status explain(OperationContext* txn, @@ -1141,15 +1180,15 @@ public: int options, string& errmsg, BSONObjBuilder& result) { - const string fullns = parseNs(dbName, cmdObj); + const NamespaceString nss(parseNsCollectionRequired(dbName, cmdObj)); auto status = Grid::get(txn)->catalogCache()->getDatabase(txn, dbName); if (!status.isOK()) { - return appendEmptyResultSet(result, status.getStatus(), fullns); + return appendEmptyResultSet(result, status.getStatus(), nss.ns()); } shared_ptr<DBConfig> conf = status.getValue(); - if (!conf->isShardingEnabled() || !conf->isSharded(fullns)) { + if (!conf->isShardingEnabled() || !conf->isSharded(nss.ns())) { if (passthrough(txn, conf.get(), cmdObj, options, result)) { return true; @@ -1191,13 +1230,13 @@ public: return false; } - shared_ptr<ChunkManager> cm = conf->getChunkManager(txn, fullns); + shared_ptr<ChunkManager> cm = conf->getChunkManager(txn, nss.ns()); massert(10420, "how could chunk manager be null!", cm); BSONObj query = getQuery(cmdObj); auto queryCollation = getCollation(cmdObj); if (!queryCollation.isOK()) { - return appendEmptyResultSet(result, queryCollation.getStatus(), fullns); + return appendEmptyResultSet(result, queryCollation.getStatus(), nss.ns()); } // Construct collator for deduping. @@ -1206,7 +1245,7 @@ public: auto statusWithCollator = CollatorFactoryInterface::get(txn->getServiceContext()) ->makeFromBSON(queryCollation.getValue()); if (!statusWithCollator.isOK()) { - return appendEmptyResultSet(result, statusWithCollator.getStatus(), fullns); + return appendEmptyResultSet(result, statusWithCollator.getStatus(), nss.ns()); } collator = std::move(statusWithCollator.getValue()); } @@ -1227,7 +1266,7 @@ public: continue; } - ShardConnection conn(shardStatus.getValue()->getConnString(), fullns); + ShardConnection conn(shardStatus.getValue()->getConnString(), nss.ns()); BSONObj res; bool ok = conn->runCommand(conf->name(), cmdObj, res, options); conn.done(); @@ -1262,7 +1301,7 @@ public: ExplainCommon::Verbosity verbosity, const rpc::ServerSelectionMetadata& serverSelectionMetadata, BSONObjBuilder* out) const { - const NamespaceString nss = parseNsCollectionRequired(dbname, cmdObj); + const NamespaceString nss(parseNsCollectionRequired(dbname, cmdObj)); // Extract the targeting query. BSONObj targetingQuery; @@ -1349,7 +1388,13 @@ public: } virtual std::string parseNs(const std::string& dbname, const BSONObj& cmdObj) const { - std::string collectionName = cmdObj.getStringField("root"); + std::string collectionName; + if (const auto rootElt = cmdObj["root"]) { + uassert(ErrorCodes::InvalidNamespace, + "'root' must be of type String", + rootElt.type() == BSONType::String); + collectionName = rootElt.str(); + } if (collectionName.empty()) collectionName = "fs"; collectionName += ".chunks"; @@ -1372,14 +1417,14 @@ public: int, string& errmsg, BSONObjBuilder& result) { - const string fullns = parseNs(dbName, cmdObj); + const NamespaceString nss(parseNs(dbName, cmdObj)); auto conf = uassertStatusOK(Grid::get(txn)->catalogCache()->getDatabase(txn, dbName)); - if (!conf->isShardingEnabled() || !conf->isSharded(fullns)) { + if (!conf->isShardingEnabled() || !conf->isSharded(nss.ns())) { return passthrough(txn, conf.get(), cmdObj, result); } - shared_ptr<ChunkManager> cm = conf->getChunkManager(txn, fullns); + shared_ptr<ChunkManager> cm = conf->getChunkManager(txn, nss.ns()); massert(13091, "how could chunk manager be null!", cm); if (SimpleBSONObjComparator::kInstance.evaluate(cm->getShardKeyPattern().toBSON() == BSON("files_id" << 1))) { @@ -1387,7 +1432,7 @@ public: vector<Strategy::CommandResult> results; Strategy::commandOp( - txn, dbName, cmdObj, 0, fullns, finder, CollationSpec::kSimpleSpec, &results); + txn, dbName, cmdObj, 0, nss.ns(), finder, CollationSpec::kSimpleSpec, &results); verify(results.size() == 1); // querying on shard key so should only talk to one shard BSONObj res = results.begin()->result; @@ -1424,7 +1469,7 @@ public: dbName, shardCmd, 0, - fullns, + nss.ns(), finder, CollationSpec::kSimpleSpec, &results); @@ -1513,20 +1558,20 @@ public: int options, string& errmsg, BSONObjBuilder& result) { - const string fullns = parseNs(dbName, cmdObj); + const NamespaceString nss(parseNsCollectionRequired(dbName, cmdObj)); auto conf = uassertStatusOK(Grid::get(txn)->catalogCache()->getDatabase(txn, dbName)); - if (!conf->isShardingEnabled() || !conf->isSharded(fullns)) { + if (!conf->isShardingEnabled() || !conf->isSharded(nss.ns())) { return passthrough(txn, conf.get(), cmdObj, options, result); } - shared_ptr<ChunkManager> cm = conf->getChunkManager(txn, fullns); + shared_ptr<ChunkManager> cm = conf->getChunkManager(txn, nss.ns()); massert(13500, "how could chunk manager be null!", cm); BSONObj query = getQuery(cmdObj); auto collation = getCollation(cmdObj); if (!collation.isOK()) { - return appendEmptyResultSet(result, collation.getStatus(), fullns); + return appendEmptyResultSet(result, collation.getStatus(), nss.ns()); } set<ShardId> shardIds; cm->getShardIdsForQuery(txn, query, collation.getValue(), &shardIds); @@ -1588,7 +1633,7 @@ public: // TODO: maybe shrink results if size() > limit } - result.append("ns", fullns); + result.append("ns", nss.ns()); result.append("near", nearStr); int outCount = 0; @@ -1758,7 +1803,8 @@ public: // Check for the listIndexes ActionType on the database, or find on system.indexes for pre // 3.0 systems. - NamespaceString ns(parseNs(dbname, cmdObj)); + const NamespaceString ns(parseNsCollectionRequired(dbname, cmdObj)); + if (authzSession->isAuthorizedForActionsOnResource(ResourcePattern::forExactNamespace(ns), ActionType::listIndexes) || authzSession->isAuthorizedForActionsOnResource( diff --git a/src/mongo/s/write_ops/batched_delete_request.cpp b/src/mongo/s/write_ops/batched_delete_request.cpp index c11717adbe8..1819423660b 100644 --- a/src/mongo/s/write_ops/batched_delete_request.cpp +++ b/src/mongo/s/write_ops/batched_delete_request.cpp @@ -114,6 +114,9 @@ bool BatchedDeleteRequest::parseBSON(StringData dbName, const BSONObj& source, s if (fieldState == FieldParser::FIELD_INVALID) return false; _ns = NamespaceString(dbName, collNameTemp); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid namespace: " << _ns.ns(), + _ns.isValid()); _isNSSet = fieldState == FieldParser::FIELD_SET; } else if (fieldName == deletes.name()) { fieldState = FieldParser::extract(field, deletes, &_deletes, errMsg); diff --git a/src/mongo/s/write_ops/batched_insert_request.cpp b/src/mongo/s/write_ops/batched_insert_request.cpp index 6d17bfd5b1e..d4ee4c0527a 100644 --- a/src/mongo/s/write_ops/batched_insert_request.cpp +++ b/src/mongo/s/write_ops/batched_insert_request.cpp @@ -120,6 +120,9 @@ bool BatchedInsertRequest::parseBSON(StringData dbName, const BSONObj& source, s if (fieldState == FieldParser::FIELD_INVALID) return false; _ns = NamespaceString(dbName, temp); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid namespace: " << _ns.ns(), + _ns.isValid()); _isNSSet = fieldState == FieldParser::FIELD_SET; } else if (documents() == sourceEl.fieldName()) { FieldParser::FieldState fieldState = diff --git a/src/mongo/s/write_ops/batched_update_request.cpp b/src/mongo/s/write_ops/batched_update_request.cpp index e35bd3678d1..3c7f4fe9664 100644 --- a/src/mongo/s/write_ops/batched_update_request.cpp +++ b/src/mongo/s/write_ops/batched_update_request.cpp @@ -122,6 +122,9 @@ bool BatchedUpdateRequest::parseBSON(StringData dbName, const BSONObj& source, s if (fieldState == FieldParser::FIELD_INVALID) return false; _ns = NamespaceString(dbName, collNameTemp); + uassert(ErrorCodes::InvalidNamespace, + str::stream() << "Invalid namespace: " << _ns.ns(), + _ns.isValid()); _isNSSet = fieldState == FieldParser::FIELD_SET; } else if (fieldName == updates.name()) { fieldState = FieldParser::extract(elem, updates, &_updates, errMsg); |