Merge pull request #14580 from Automattic/vkarpov15/gh-14567

fix(query): shallow clone $or, $and if merging onto empty query filter

Author: vkarpov15

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

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];
     }

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 } });
+  });
 });