Skip to content

Commit

Permalink
Merge pull request #9176 from AbdelrahmanHafez/gh-9163
Browse files Browse the repository at this point in the history
fix(document): disallow `transform` functions that return promises
  • Loading branch information
vkarpov15 authored Jul 1, 2020
2 parents c0d56e6 + 734b1ec commit 316f922
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 3 deletions.
17 changes: 14 additions & 3 deletions lib/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const inspect = require('util').inspect;
const internalToObjectOptions = require('./options').internalToObjectOptions;
const mpath = require('mpath');
const utils = require('./utils');
const isPromise = require('./helpers/isPromise');

const clone = utils.clone;
const deepEqual = utils.deepEqual;
Expand Down Expand Up @@ -3075,7 +3076,7 @@ Document.prototype.$toObject = function(options, json) {
// we need to adjust options.transform to be the child schema's transform and
// not the parent schema's
if (transform) {
applySchemaTypeTransforms(this, ret, gettersOptions, options);
applySchemaTypeTransforms(this, ret);
}

if (transform === true || (schemaOptions.toObject && transform)) {
Expand Down Expand Up @@ -3414,13 +3415,17 @@ function applySchemaTypeTransforms(self, json) {
const schematype = schema.paths[path];
if (typeof schematype.options.transform === 'function') {
const val = self.get(path);
json[path] = schematype.options.transform.call(self, val);
const transformedValue = schematype.options.transform.call(self, val);
throwErrorIfPromise(path, transformedValue);
json[path] = transformedValue;
} else if (schematype.$embeddedSchemaType != null &&
typeof schematype.$embeddedSchemaType.options.transform === 'function') {
const vals = [].concat(self.get(path));
const transform = schematype.$embeddedSchemaType.options.transform;
for (let i = 0; i < vals.length; ++i) {
vals[i] = transform.call(self, vals[i]);
const transformedValue = transform.call(self, vals[i]);
vals[i] = transformedValue;
throwErrorIfPromise(path, transformedValue);
}

json[path] = vals;
Expand All @@ -3430,6 +3435,12 @@ function applySchemaTypeTransforms(self, json) {
return json;
}

function throwErrorIfPromise(path, transformedValue) {
if (isPromise(transformedValue)) {
throw new Error('`transform` function must be synchronous, but the transform on path `' + path + '` returned a promise.');
}
}

/**
* The return value of this method is used in calls to JSON.stringify(doc).
*
Expand Down
6 changes: 6 additions & 0 deletions lib/helpers/isPromise.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
'use strict';
function isPromise(val) {
return !!val && (typeof val === 'object' || typeof val === 'function') && typeof val.then === 'function';
}

module.exports = isPromise;
22 changes: 22 additions & 0 deletions test/document.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8976,6 +8976,28 @@ describe('document', function() {
assert.equal(axl.fullName, 'Axl Rose');
});

it('throws an error when `transform` returns a promise (gh-9163)', function() {
const userSchema = new Schema({
name: {
type: String,
transform: function() {
return new Promise(() => {});
}
}
});

const User = db.model('User', userSchema);

const user = new User({ name: 'Hafez' });
assert.throws(function() {
user.toJSON();
}, /`transform` option has to be synchronous, but is returning a promise on path `name`./);

assert.throws(function() {
user.toObject();
}, /`transform` option has to be synchronous, but is returning a promise on path `name`./);
});

it('uses strict equality when checking mixed paths for modifications (gh-9165)', function() {
const schema = Schema({ obj: {} });
const Model = db.model('gh9165', schema);
Expand Down

0 comments on commit 316f922

Please sign in to comment.