diff options
| -rw-r--r-- | jstests/core/find_and_modify_invalid_query_params.js | 93 | ||||
| -rw-r--r-- | src/mongo/db/query/find_and_modify_request.cpp | 34 | ||||
| -rw-r--r-- | src/mongo/db/query/find_and_modify_request_test.cpp | 32 | ||||
| -rw-r--r-- | src/mongo/shell/crud_api.js | 6 |
4 files changed, 159 insertions, 6 deletions
diff --git a/jstests/core/find_and_modify_invalid_query_params.js b/jstests/core/find_and_modify_invalid_query_params.js new file mode 100644 index 00000000000..a54d9217b24 --- /dev/null +++ b/jstests/core/find_and_modify_invalid_query_params.js @@ -0,0 +1,93 @@ +/** + * Test that the 'findAndModify' command throws the expected errors for invalid query, sort and + * projection parameters. This test exercises the fix for SERVER-41829. + * @tags: [assumes_unsharded_collection] + */ +(function() { + "use strict"; + + const coll = db.find_and_modify_invalid_inputs; + coll.drop(); + coll.insert({_id: 0}); + coll.insert({_id: 1}); + + function assertFailedWithCode(cmd, errorCode) { + const err = assert.throws(() => coll.findAndModify(cmd)); + assert.eq(err.code, errorCode); + } + + function assertWorked(cmd, expectedValue) { + const out = assert.doesNotThrow(() => coll.findAndModify(cmd)); + assert.eq(out.value, expectedValue); + } + + // Verify that the findAndModify command works when we supply a valid query. + let out = coll.findAndModify({query: {_id: 1}, update: {$set: {value: "basic"}}, new: true}); + assert.eq(out, {_id: 1, value: "basic"}); + + // Verify that invalid 'query' object fails. + assertFailedWithCode({query: null, update: {value: 2}}, 31160); + assertFailedWithCode({query: 1, update: {value: 2}}, 31160); + assertFailedWithCode({query: "{_id: 1}", update: {value: 2}}, 31160); + assertFailedWithCode({query: false, update: {value: 2}}, 31160); + + // Verify that missing and empty query object is allowed. + assertWorked({update: {$set: {value: "missingQuery"}}, new: true}, "missingQuery"); + assertWorked({query: {}, update: {$set: {value: "emptyQuery"}}, new: true}, "emptyQuery"); + + // Verify that command works when we supply a valid sort specification. + assertWorked({sort: {_id: -1}, update: {$set: {value: "sort"}}, new: true}, "sort"); + + // Verify that invaid 'sort' object fails. + assertFailedWithCode({sort: null, update: {value: 2}}, 31174); + assertFailedWithCode({sort: 1, update: {value: 2}}, 31174); + assertFailedWithCode({sort: "{_id: 1}", update: {value: 2}}, 31174); + assertFailedWithCode({sort: false, update: {value: 2}}, 31174); + + // Verify that missing and empty 'sort' object is allowed. + assertWorked({update: {$set: {value: "missingSort"}}, new: true}, "missingSort"); + assertWorked({sort: {}, update: {$set: {value: "emptySort"}}, new: true}, "emptySort"); + + // Verify that the 'fields' projection works. + assertWorked({fields: {_id: 0}, update: {$set: {value: "project"}}, new: true}, "project"); + + // Verify that invaid 'fields' object fails. + assertFailedWithCode({fields: null, update: {value: 2}}, 31175); + assertFailedWithCode({fields: 1, update: {value: 2}}, 31175); + assertFailedWithCode({fields: "{_id: 1}", update: {value: 2}}, 31175); + assertFailedWithCode({fields: false, update: {value: 2}}, 31175); + + // Verify that missing and empty 'fields' object is allowed. Also verify that the command + // projects all the fields. + assertWorked({update: {$set: {value: "missingFields"}}, new: true}, "missingFields"); + assertWorked({fields: {}, update: {$set: {value: "emptyFields"}}, new: true}, "emptyFields"); + + // Verify that findOneAndDelete() shell helper throws the same errors as findAndModify(). + let err = assert.throws(() => coll.findOneAndDelete("{_id: 1}")); + assert.eq(err.code, 31160); + err = assert.throws(() => coll.findOneAndDelete(null, {sort: 1})); + assert.eq(err.code, 31174); + + // Verify that findOneAndReplace() shell helper throws the same errors as findAndModify(). + err = assert.throws(() => coll.findOneAndReplace("{_id: 1}", {})); + assert.eq(err.code, 31160); + err = assert.throws(() => coll.findOneAndReplace(null, {}, {sort: 1})); + assert.eq(err.code, 31174); + + // Verify that findOneAndUpdate() shell helper throws the same errors as findAndModify(). + err = assert.throws(() => coll.findOneAndUpdate("{_id: 1}", {$set: {value: "new"}})); + assert.eq(err.code, 31160); + err = assert.throws(() => coll.findOneAndUpdate(null, {$set: {value: "new"}}, {sort: 1})); + assert.eq(err.code, 31174); + + // Verify that find and modify shell helpers allow null query object. + out = + coll.findOneAndUpdate(null, {$set: {value: "findOneAndUpdate"}}, {returnNewDocument: true}); + assert.eq(out.value, "findOneAndUpdate"); + + out = coll.findOneAndReplace(null, {value: "findOneAndReplace"}, {returnNewDocument: true}); + assert.eq(out.value, "findOneAndReplace"); + + out = coll.findOneAndDelete(null); + assert.eq(out.value, "findOneAndReplace"); +})(); diff --git a/src/mongo/db/query/find_and_modify_request.cpp b/src/mongo/db/query/find_and_modify_request.cpp index df3db508e3f..1f5b8eda868 100644 --- a/src/mongo/db/query/find_and_modify_request.cpp +++ b/src/mongo/db/query/find_and_modify_request.cpp @@ -122,10 +122,38 @@ BSONObj FindAndModifyRequest::toBSON() const { StatusWith<FindAndModifyRequest> FindAndModifyRequest::parseFromBSON(NamespaceString fullNs, const BSONObj& cmdObj) { - BSONObj query = cmdObj.getObjectField(kQueryField); - BSONObj fields = cmdObj.getObjectField(kFieldProjectionField); + BSONObj query; + BSONObj sort; + BSONObj fields; + if (auto queryElement = cmdObj[kQueryField]) { + if (queryElement.type() != Object) { + return {ErrorCodes::Error(31160), + str::stream() << "'" << kQueryField << "' parameter must be an object, found " + << queryElement.type()}; + } + query = queryElement.embeddedObject(); + } + + if (auto sortElement = cmdObj[kSortField]) { + if (sortElement.type() != Object) { + return {ErrorCodes::Error(31174), + str::stream() << "'" << kSortField << "' parameter must be an object, found " + << sortElement.type()}; + } + sort = sortElement.embeddedObject(); + } + + if (auto projectionElement = cmdObj[kFieldProjectionField]) { + if (projectionElement.type() != Object) { + return {ErrorCodes::Error(31175), + str::stream() << "'" << kFieldProjectionField + << "' parameter must be an object, found " + << projectionElement.type()}; + } + fields = projectionElement.embeddedObject(); + } + BSONObj updateObj = cmdObj.getObjectField(kUpdateField); - BSONObj sort = cmdObj.getObjectField(kSortField); BSONObj collation; { diff --git a/src/mongo/db/query/find_and_modify_request_test.cpp b/src/mongo/db/query/find_and_modify_request_test.cpp index 6c3a8bdd17e..2454195a95d 100644 --- a/src/mongo/db/query/find_and_modify_request_test.cpp +++ b/src/mongo/db/query/find_and_modify_request_test.cpp @@ -522,5 +522,37 @@ TEST(FindAndModifyRequest, ParseWithCollationTypeMismatch) { ASSERT_EQUALS(parseStatus.getStatus(), ErrorCodes::TypeMismatch); } +TEST(FindAndModifyRequest, InvalidQueryParameter) { + BSONObj cmdObj(fromjson(R"json({ + findAndModify: 'user', + query: '{ x: 1 }', + remove: true + })json")); + + auto parseStatus = FindAndModifyRequest::parseFromBSON(NamespaceString("a.b"), cmdObj); + ASSERT_EQ(31160, parseStatus.getStatus().code()); +} + +TEST(FindAndModifyRequest, InvalidSortParameter) { + BSONObj cmdObj(fromjson(R"json({ + findAndModify: 'user', + sort: 1, + remove: true + })json")); + + auto parseStatus = FindAndModifyRequest::parseFromBSON(NamespaceString("a.b"), cmdObj); + ASSERT_EQ(31174, parseStatus.getStatus().code()); +} + +TEST(FindAndModifyRequest, InvalidFieldParameter) { + BSONObj cmdObj(fromjson(R"json({ + findAndModify: 'user', + fields: null, + remove: true + })json")); + + auto parseStatus = FindAndModifyRequest::parseFromBSON(NamespaceString("a.b"), cmdObj); + ASSERT_EQ(31175, parseStatus.getStatus().code()); +} } // unnamed namespace } // namespace mongo diff --git a/src/mongo/shell/crud_api.js b/src/mongo/shell/crud_api.js index 32ad1240b44..fd1aa7be5ef 100644 --- a/src/mongo/shell/crud_api.js +++ b/src/mongo/shell/crud_api.js @@ -697,7 +697,7 @@ DBCollection.prototype.updateMany = function(filter, update, options) { DBCollection.prototype.findOneAndDelete = function(filter, options) { var opts = Object.extend({}, options || {}); // Set up the command - var cmd = {query: filter, remove: true}; + var cmd = {query: filter || {}, remove: true}; if (opts.sort) { cmd.sort = opts.sort; @@ -755,7 +755,7 @@ DBCollection.prototype.findOneAndReplace = function(filter, replacement, options } // Set up the command - var cmd = {query: filter, update: replacement}; + var cmd = {query: filter || {}, update: replacement}; if (opts.sort) { cmd.sort = opts.sort; } @@ -820,7 +820,7 @@ DBCollection.prototype.findOneAndUpdate = function(filter, update, options) { } // Set up the command - var cmd = {query: filter, update: update}; + var cmd = {query: filter || {}, update: update}; if (opts.sort) { cmd.sort = opts.sort; } |