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(document): make array getters avoid unintentionally modifying array, defer getters until index access instead #13774

Merged
merged 4 commits into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
8 changes: 8 additions & 0 deletions lib/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -4091,6 +4091,14 @@ function applyGetters(self, json, options) {
if (ii === last) {
const val = self.$get(path);
branch[part] = clone(val, options);
if (Array.isArray(branch[part]) && schema.paths[path].$embeddedSchemaType) {
for (let i = 0; i < branch[part].length; ++i) {
branch[part][i] = schema.paths[path].$embeddedSchemaType.applyGetters(
branch[part][i],
self
);
}
}
} else if (v == null) {
if (part in cur) {
branch[part] = v;
Expand Down
7 changes: 0 additions & 7 deletions lib/schema/array.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,13 +289,6 @@ SchemaArray.prototype.applyGetters = function(value, scope) {
}

const ret = SchemaType.prototype.applyGetters.call(this, value, scope);
if (Array.isArray(ret)) {
const rawValue = utils.isMongooseArray(ret) ? ret.__array : ret;
const len = rawValue.length;
for (let i = 0; i < len; ++i) {
rawValue[i] = this.caster.applyGetters(rawValue[i], scope);
}
}
return ret;
};

Expand Down
3 changes: 3 additions & 0 deletions lib/types/array/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ function MongooseArray(values, path, doc, schematype) {
if (mongooseArrayMethods.hasOwnProperty(prop)) {
return mongooseArrayMethods[prop];
}
if (typeof prop === 'string' && numberRE.test(prop) && schematype?.$embeddedSchemaType != null) {
return schematype.$embeddedSchemaType.applyGetters(__array[prop], doc);
}

return __array[prop];
},
Expand Down
47 changes: 46 additions & 1 deletion test/document.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5564,6 +5564,7 @@ describe('document', function() {
const Test = db.model('Test', testSchema);

const doc = new Test({ arr: [new mongoose.Types.ObjectId()] });
assert.equal(called, 0);
assert.deepEqual(doc.toObject({ getters: true }).arr, [42]);
assert.equal(called, 1);
});
Expand All @@ -5582,7 +5583,6 @@ describe('document', function() {

const Test = db.model('Test', testSchema);


let doc = await Test.create({ arr: [new mongoose.Types.ObjectId()] });
assert.equal(called, 1);

Expand Down Expand Up @@ -12343,6 +12343,51 @@ describe('document', function() {
assert.strictEqual(Object.polluted, undefined);
});

it('does not modify array when calling getters (gh-13748)', async function() {
// create simple setter to add a sufix
const addSufix = (name) => {
return name + '-sufix';
};

// create simple gettrer to remove last 6 letters (should be "-sufix")
const removeSufix = (name) => {
return ('' + name).slice(0, -6);
};

const userSchema = new mongoose.Schema(
{
name: String,
age: Number,
profession: {
type: String,
get: removeSufix,
set: addSufix
},
hobbies: [{ type: String, get: removeSufix, set: addSufix }]
},
{
toObject: { getters: true },
toJSON: { getters: true }
}
);
const User = db.model('User', userSchema);

const usr = await User.create({
name: 'John',
age: 18,
profession: 'teacher',
hobbies: ['swimming', 'football']
});

const oneUser = await User.findById(usr._id).orFail();
assert.equal(oneUser.profession, 'teacher');
assert.equal(oneUser.profession, 'teacher');
assert.equal(oneUser.profession, 'teacher');
assert.equal(oneUser.hobbies[0], 'swimming');
assert.equal(oneUser.hobbies[0], 'swimming');
assert.equal(oneUser.hobbies[0], 'swimming');
});

it('sets defaults on subdocs with subdoc projection (gh-13720)', async function() {
const subSchema = new mongoose.Schema({
propertyA: { type: String, default: 'A' },
Expand Down