From e5617db9eed1dd0b75cebbade97c337219aab07f Mon Sep 17 00:00:00 2001 From: Hafez Date: Thu, 25 Jun 2020 23:45:58 +0200 Subject: [PATCH 1/5] remove unused arguments from `applySchemaTypeTransforms` --- lib/document.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/document.js b/lib/document.js index 3e0e42cc1e..1c0ba36585 100644 --- a/lib/document.js +++ b/lib/document.js @@ -3075,7 +3075,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)) { From ea7ecf3f1daf3c9b7286ce97ec0fcdcb080d8f98 Mon Sep 17 00:00:00 2001 From: Hafez Date: Fri, 26 Jun 2020 00:10:51 +0200 Subject: [PATCH 2/5] test: repro #9163 --- test/document.test.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/document.test.js b/test/document.test.js index 9983736d38..bb521b8a46 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -8975,4 +8975,26 @@ describe('document', function() { const axl = new Person({ fullName: 'Axl Rose' }); 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 = mongoose.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`./); + }); }); From a29b9609a0db84011936b27ed095fea2b048c30b Mon Sep 17 00:00:00 2001 From: Hafez Date: Fri, 26 Jun 2020 00:13:39 +0200 Subject: [PATCH 3/5] fix(document): disallow `transform` functions that return promises --- lib/document.js | 15 +++++++++++++-- lib/helpers/isPromise.js | 6 ++++++ 2 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 lib/helpers/isPromise.js diff --git a/lib/document.js b/lib/document.js index 1c0ba36585..b3c41c10c3 100644 --- a/lib/document.js +++ b/lib/document.js @@ -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; @@ -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; @@ -3430,6 +3435,12 @@ function applySchemaTypeTransforms(self, json) { return json; } +function throwErrorIfPromise(path, transformedValue) { + if (isPromise(transformedValue)) { + throw new Error('`transform` option has to be synchronous, but is returning a promise on path `' + path + '`.'); + } +} + /** * The return value of this method is used in calls to JSON.stringify(doc). * diff --git a/lib/helpers/isPromise.js b/lib/helpers/isPromise.js new file mode 100644 index 0000000000..d6db2608cc --- /dev/null +++ b/lib/helpers/isPromise.js @@ -0,0 +1,6 @@ +'use strict'; +function isPromise(val) { + return !!val && (typeof val === 'object' || typeof val === 'function') && typeof val.then === 'function'; +} + +module.exports = isPromise; \ No newline at end of file From 05bbdd6d9af789a8a08b63ae329cd9a3cf4e131d Mon Sep 17 00:00:00 2001 From: Hafez Date: Fri, 26 Jun 2020 00:53:08 +0200 Subject: [PATCH 4/5] fix tests re #9163 --- test/document.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/document.test.js b/test/document.test.js index bb521b8a46..ff875ce1b8 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -8986,7 +8986,7 @@ describe('document', function() { } }); - const User = mongoose.model('User', userSchema); + const User = db.model('User', userSchema); const user = new User({ name: 'Hafez' }); assert.throws(function() { From 9761ced8160397889a992dc0052c93ff6f8560d3 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 1 Jul 2020 10:30:08 -0400 Subject: [PATCH 5/5] chore: quick copy change --- lib/document.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/document.js b/lib/document.js index b3c41c10c3..a8614bbb74 100644 --- a/lib/document.js +++ b/lib/document.js @@ -3437,7 +3437,7 @@ function applySchemaTypeTransforms(self, json) { function throwErrorIfPromise(path, transformedValue) { if (isPromise(transformedValue)) { - throw new Error('`transform` option has to be synchronous, but is returning a promise on path `' + path + '`.'); + throw new Error('`transform` function must be synchronous, but the transform on path `' + path + '` returned a promise.'); } }