diff --git a/lib/query.js b/lib/query.js index 22956fb818f..34af09d552b 100644 --- a/lib/query.js +++ b/lib/query.js @@ -2380,13 +2380,19 @@ Query.prototype.merge = function(source) { } opts.omit = {}; - if (this._conditions && this._conditions.$and && source.$and) { + if (source.$and) { opts.omit['$and'] = true; - this._conditions.$and = this._conditions.$and.concat(source.$and); + if (!this._conditions) { + this._conditions = {}; + } + this._conditions.$and = (this._conditions.$and || []).concat(source.$and); } - if (this._conditions && this._conditions.$or && source.$or) { + if (source.$or) { opts.omit['$or'] = true; - this._conditions.$or = this._conditions.$or.concat(source.$or); + if (!this._conditions) { + this._conditions = {}; + } + this._conditions.$or = (this._conditions.$or || []).concat(source.$or); } // plain object diff --git a/lib/utils.js b/lib/utils.js index e3ed05b111f..9af4b12727e 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -15,6 +15,7 @@ const isObject = require('./helpers/isObject'); const isMongooseArray = require('./types/array/isMongooseArray'); const isMongooseDocumentArray = require('./types/documentArray/isMongooseDocumentArray'); const isBsonType = require('./helpers/isBsonType'); +const isPOJO = require('./helpers/isPOJO'); const getFunctionName = require('./helpers/getFunctionName'); const isMongooseObject = require('./helpers/isMongooseObject'); const promiseOrCallback = require('./helpers/promiseOrCallback'); @@ -264,7 +265,13 @@ exports.merge = function merge(to, from, options, path) { continue; } if (to[key] == null) { - to[key] = from[key]; + if (isPOJO(from[key])) { + to[key] = { ...from[key] }; + } else if (Array.isArray(from[key])) { + to[key] = [...from[key]]; + } else { + to[key] = from[key]; + } } else if (exports.isObject(from[key])) { if (!exports.isObject(to[key])) { to[key] = {}; @@ -501,7 +508,7 @@ exports.populate = function populate(path, select, model, match, options, subPop if (path instanceof PopulateOptions) { // If reusing old populate docs, avoid reusing `_docs` because that may // lead to bugs and memory leaks. See gh-11641 - path._docs = []; + path._docs = {}; path._childDocs = []; return [path]; } diff --git a/test/query.test.js b/test/query.test.js index 21db30e8b3b..1073bb089e6 100644 --- a/test/query.test.js +++ b/test/query.test.js @@ -4054,6 +4054,25 @@ describe('Query', function() { }); }); + it('shallow clones $and, $or if merging with empty filter (gh-14567) (gh-12944)', function() { + const TestModel = db.model( + 'Test', + Schema({ name: String, age: Number, active: Boolean }) + ); + + let originalQuery = { $and: [{ active: true }] }; + let q = TestModel.countDocuments(originalQuery) + .and([{ age: { $gte: 18 } }]); + assert.deepStrictEqual(originalQuery, { $and: [{ active: true }] }); + assert.deepStrictEqual(q.getFilter(), { $and: [{ active: true }, { age: { $gte: 18 } }] }); + + originalQuery = { $or: [{ active: true }] }; + q = TestModel.countDocuments(originalQuery) + .or([{ age: { $gte: 18 } }]); + assert.deepStrictEqual(originalQuery, { $or: [{ active: true }] }); + assert.deepStrictEqual(q.getFilter(), { $or: [{ active: true }, { age: { $gte: 18 } }] }); + }); + it('should avoid sending empty session to MongoDB server (gh-13052)', async function() { const m = new mongoose.Mongoose(); @@ -4236,4 +4255,22 @@ describe('Query', function() { q.sort({}, { override: true }); assert.deepStrictEqual(q.getOptions().sort, {}); }); + + it('avoids mutating user-provided query selectors (gh-14567)', async function() { + const TestModel = db.model( + 'Test', + Schema({ name: String, age: Number, active: Boolean }) + ); + + await TestModel.create({ name: 'John', age: 21 }); + await TestModel.create({ name: 'Bob', age: 35 }); + + const adultQuery = { age: { $gte: 18 } }; + + const docs = await TestModel.find(adultQuery).where('age').lte(25); + assert.equal(docs.length, 1); + assert.equal(docs[0].name, 'John'); + + assert.deepStrictEqual(adultQuery, { age: { $gte: 18 } }); + }); });