Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(model): deep clone bulkWrite() updateOne arguments to avoid mutating documents in update #14197

Merged
merged 6 commits into from Dec 30, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion lib/drivers/node-mongodb-native/collection.js
Expand Up @@ -119,7 +119,6 @@ function iter(i) {
throw error;
}
}

let _args = args;
let callback = null;
if (this._shouldBufferCommands() && this.buffer) {
Expand Down
14 changes: 8 additions & 6 deletions lib/helpers/model/castBulkWrite.js
Expand Up @@ -6,6 +6,7 @@ const applyTimestampsToChildren = require('../update/applyTimestampsToChildren')
const applyTimestampsToUpdate = require('../update/applyTimestampsToUpdate');
const cast = require('../../cast');
const castUpdate = require('../query/castUpdate');
const clone = require('../clone');
const decorateUpdateWithVersionKey = require('../update/decorateUpdateWithVersionKey');
const { inspect } = require('util');
const setDefaultsOnInsert = require('../setDefaultsOnInsert');
Expand Down Expand Up @@ -64,30 +65,32 @@ module.exports = function castBulkWrite(originalModel, op, options) {
const schema = model.schema;
const strict = options.strict != null ? options.strict : model.schema.options.strict;

const update = clone(op['updateOne']['update']);

_addDiscriminatorToObject(schema, op['updateOne']['filter']);

if (model.schema.$timestamps != null && op['updateOne'].timestamps !== false) {
const createdAt = model.schema.$timestamps.createdAt;
const updatedAt = model.schema.$timestamps.updatedAt;
applyTimestampsToUpdate(now, createdAt, updatedAt, op['updateOne']['update'], {});
applyTimestampsToUpdate(now, createdAt, updatedAt, update, {});
}

if (op['updateOne'].timestamps !== false) {
applyTimestampsToChildren(now, op['updateOne']['update'], model.schema);
applyTimestampsToChildren(now, update, model.schema);
}

const shouldSetDefaultsOnInsert = op['updateOne'].setDefaultsOnInsert == null ?
globalSetDefaultsOnInsert :
op['updateOne'].setDefaultsOnInsert;
if (shouldSetDefaultsOnInsert !== false) {
setDefaultsOnInsert(op['updateOne']['filter'], model.schema, op['updateOne']['update'], {
setDefaultsOnInsert(op['updateOne']['filter'], model.schema, update, {
setDefaultsOnInsert: true,
upsert: op['updateOne'].upsert
});
}

decorateUpdateWithVersionKey(
op['updateOne']['update'],
update,
op['updateOne'],
model.schema.options.versionKey
);
Expand All @@ -96,8 +99,7 @@ module.exports = function castBulkWrite(originalModel, op, options) {
strict: strict,
upsert: op['updateOne'].upsert
});

op['updateOne']['update'] = castUpdate(model.schema, op['updateOne']['update'], {
op['updateOne']['update'] = castUpdate(model.schema, update, {
strict: strict,
upsert: op['updateOne'].upsert
}, model, op['updateOne']['filter']);
Expand Down
7 changes: 5 additions & 2 deletions lib/helpers/query/castUpdate.js
Expand Up @@ -13,6 +13,7 @@ const moveImmutableProperties = require('../update/moveImmutableProperties');
const schemaMixedSymbol = require('../../schema/symbols').schemaMixedSymbol;
const setDottedPath = require('../path/setDottedPath');
const utils = require('../../utils');
const { internalToObjectOptions } = require('../../options');

const mongodbUpdateOperators = new Set([
'$currentDate',
Expand Down Expand Up @@ -99,7 +100,10 @@ module.exports = function castUpdate(schema, obj, options, context, filter) {
const op = ops[i];
val = ret[op];
hasDollarKey = hasDollarKey || op.startsWith('$');

if (val != null && val.$__) {
val = val.toObject(internalToObjectOptions);
ret[op] = val;
}
if (val &&
typeof val === 'object' &&
!Buffer.isBuffer(val) &&
Expand All @@ -110,7 +114,6 @@ module.exports = function castUpdate(schema, obj, options, context, filter) {
+ 'Expected an object, received ' + typeof val;
throw new Error(msg);
}

if (op.startsWith('$') && utils.isEmptyObject(val)) {
delete ret[op];
}
Expand Down
3 changes: 0 additions & 3 deletions lib/model.js
Expand Up @@ -3335,7 +3335,6 @@ Model.bulkWrite = async function bulkWrite(ops, options) {
const ordered = options.ordered == null ? true : options.ordered;

const validations = ops.map(op => castBulkWrite(this, op, options));

return new Promise((resolve, reject) => {
if (ordered) {
each(validations, (fn, cb) => fn(cb), error => {
Expand All @@ -3352,7 +3351,6 @@ Model.bulkWrite = async function bulkWrite(ops, options) {
if (error) {
return reject(error);
}

resolve(res);
});
} catch (err) {
Expand Down Expand Up @@ -3424,7 +3422,6 @@ Model.bulkWrite = async function bulkWrite(ops, options) {
res.mongoose.results = results;
}
}

resolve(res);
});
}
Expand Down
41 changes: 41 additions & 0 deletions test/model.test.js
Expand Up @@ -6392,6 +6392,47 @@ describe('Model', function() {
});
assert.deepEqual(timestampsOptions, [undefined, undefined]);
});
it('should not modify the object in the $set clause and not error when dealing with or without timestamps (gh-14164)', async function() {
const timeSchema = new Schema({
name: String,
properties: { type: Schema.Types.Mixed, default: {} }
}, { timestamps: true });
const timelessSchema = new Schema({
name: String,
properties: { type: Schema.Types.Mixed, default: {} }
});

const Time = db.model('Time', timeSchema);
const Timeless = db.model('Timeless', timelessSchema);

const timeDoc = await Time.create({
name: 'Time Test'
});
timeDoc.properties.color = 'Red';
const beforeSet = {};
Object.assign(beforeSet, timeDoc.toObject());
await Time.bulkWrite([{
updateOne: {
filter: { _id: timeDoc._id },
update: { $set: timeDoc }
}
}]);
assert.deepStrictEqual(beforeSet, timeDoc.toObject());

const timelessDoc = await Timeless.create({
name: 'Timeless Test'
});
timelessDoc.properties.color = 'Red';
const timelessObj = {};
Object.assign(timelessObj, timelessDoc.toObject());
await Timeless.bulkWrite([{
updateOne: {
filter: { _id: timelessDoc._id },
update: { $set: timelessDoc }
}
}]);
assert.deepStrictEqual(timelessObj, timelessDoc.toObject());
});
});

describe('bulkSave() (gh-9673)', function() {
Expand Down
2 changes: 0 additions & 2 deletions test/schema.select.test.js
Expand Up @@ -15,12 +15,10 @@ describe('schema select option', function() {

before(function() {
db = start();
mongoose.set('debug', true);
});

after(async function() {
await db.close();
mongoose.set('debug', false);
});

beforeEach(() => db.deleteModel(/.*/));
Expand Down