From e0e49f6c6e958285f5d7a46d2a55cd3da4b13be0 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 27 Apr 2023 14:01:40 -0400 Subject: [PATCH 1/2] fix(query): set ObjectParameterError if calling `findOneAndX()` with filter as non-object Fix #13264 --- lib/error/objectParameter.js | 2 +- lib/query.js | 46 +++++++++++++++++++++++------------- 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/lib/error/objectParameter.js b/lib/error/objectParameter.js index 5881a481aea..295582e2484 100644 --- a/lib/error/objectParameter.js +++ b/lib/error/objectParameter.js @@ -18,7 +18,7 @@ class ObjectParameterError extends MongooseError { */ constructor(value, paramName, fnName) { super('Parameter "' + paramName + '" to ' + fnName + - '() must be an object, got ' + value.toString()); + '() must be an object, got "' + value.toString() + '" (type ' + typeof value + ')'); } } diff --git a/lib/query.js b/lib/query.js index 418fb3663e4..4d6f3accc86 100644 --- a/lib/query.js +++ b/lib/query.js @@ -3411,7 +3411,7 @@ function prepareDiscriminatorCriteria(query) { * @api public */ -Query.prototype.findOneAndUpdate = function(criteria, doc, options, callback) { +Query.prototype.findOneAndUpdate = function(filter, doc, options, callback) { this.op = 'findOneAndUpdate'; this._validateOp(); this._validate(); @@ -3426,23 +3426,27 @@ Query.prototype.findOneAndUpdate = function(criteria, doc, options, callback) { case 2: if (typeof doc === 'function') { callback = doc; - doc = criteria; - criteria = undefined; + doc = filter; + filter = undefined; } options = undefined; break; case 1: - if (typeof criteria === 'function') { - callback = criteria; - criteria = options = doc = undefined; + if (typeof filter === 'function') { + callback = filter; + filter = options = doc = undefined; } else { - doc = criteria; - criteria = options = undefined; + doc = filter; + filter = options = undefined; } } - if (mquery.canMerge(criteria)) { - this.merge(criteria); + if (mquery.canMerge(filter)) { + this.merge(filter); + } else if (filter != null) { + this.error( + new ObjectParameterError(filter, 'filter', 'findOneAndUpdate') + ); } // apply doc @@ -3620,7 +3624,7 @@ Query.prototype.findOneAndRemove = function(conditions, options, callback) { * * @method findOneAndDelete * @memberOf Query - * @param {Object} [conditions] + * @param {Object} [filter] * @param {Object} [options] * @param {Boolean} [options.rawResult] if true, returns the [raw result from the MongoDB driver](https://mongodb.github.io/node-mongodb-native/4.9/interfaces/ModifyResult.html) * @param {ClientSession} [options.session=null] The session associated with this query. See [transactions docs](/docs/transactions.html). @@ -3631,7 +3635,7 @@ Query.prototype.findOneAndRemove = function(conditions, options, callback) { * @api public */ -Query.prototype.findOneAndDelete = function(conditions, options, callback) { +Query.prototype.findOneAndDelete = function(filter, options, callback) { this.op = 'findOneAndDelete'; this._validateOp(); this._validate(); @@ -3644,16 +3648,20 @@ Query.prototype.findOneAndDelete = function(conditions, options, callback) { } break; case 1: - if (typeof conditions === 'function') { - callback = conditions; - conditions = undefined; + if (typeof filter === 'function') { + callback = filter; + filter = undefined; options = undefined; } break; } - if (mquery.canMerge(conditions)) { - this.merge(conditions); + if (mquery.canMerge(filter)) { + this.merge(filter); + } else if (filter != null) { + this.error( + new ObjectParameterError(filter, 'filter', 'findOneAndDelete') + ); } options && this.setOptions(options); @@ -3789,6 +3797,10 @@ Query.prototype.findOneAndReplace = function(filter, replacement, options, callb if (mquery.canMerge(filter)) { this.merge(filter); + } else if (filter != null) { + this.error( + new ObjectParameterError(filter, 'filter', 'findOneAndReplace') + ); } if (replacement != null) { From a50aa5495448e85df88b4c39032397b699851948 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 27 Apr 2023 14:06:01 -0400 Subject: [PATCH 2/2] test: add test case for #13264 --- test/model.findOneAndUpdate.test.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/model.findOneAndUpdate.test.js b/test/model.findOneAndUpdate.test.js index 3e7b3660bd7..91b6f13ad72 100644 --- a/test/model.findOneAndUpdate.test.js +++ b/test/model.findOneAndUpdate.test.js @@ -2403,4 +2403,14 @@ describe('model: findOneAndUpdate:', function() { assert.ifError(err); }); + + it('throws error if filter is not an object (gh-13264)', async function() { + const schema = new Schema({ name: String }); + const Model = db.model('Test', schema); + + const err = await Model.findOneAndUpdate('foobar', { name: 'foo' }).then(() => null, err => err); + assert.ok(err); + assert.equal(err.name, 'ObjectParameterError'); + }); + });